Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mining progress #7

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Mining progress #7

wants to merge 17 commits into from

Conversation

pcanelas
Copy link
Member

@pcanelas pcanelas commented Nov 8, 2022

Mining of a Question in ROS Answers:

  • Updated the models: added attributes to the User, Answer, Comment and Question class.
  • Changes to the parser
    • Completed scrapping the user, answer and question.
    • Formatted the code.

@@ -1,25 +1,23 @@
# -*- coding: utf-8 -*-
import argparse

from .parser import url_to_question
from ros_answers_miner.parser import url_to_question
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

relative imports are better here; I'd switch back to the original version of the line unless there's a strong reason for the change?


def main() -> None:
parser = argparse.ArgumentParser('ros-answers-miner')
subparsers = parser.add_subparsers()

p = subparsers.add_parser(
'scrape',
help='extracts information from a given ROS Answers question URL')
'scrap',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo?


import attr
class User:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use attrs or a dataclass instead.

}


class Comment:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, prefer attrs or a data class



@attr.s(auto_attribs=True)
class Answer:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was attrs dropped?

comments: OrderedSet[Comment]
answers: OrderedSet[Answer]

def __init__(self, url, date, votes, views, user, title, content, tags, comments, answers):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this is why attrs exists! Don't roll your own init, hash, cmp, eq, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants