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

solution #356

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

solution #356

wants to merge 2 commits into from

Conversation

Baranotik15
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Great job on implementing the solution! 🎉 The code meets all the task requirements and constraints effectively. While there are some minor improvements suggested regarding type hinting, these do not impact the functionality of your code. Keep up the good work and consider refining the type hints for even clearer code in the future. 😊

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

)


def get_single_page_quotes(soup: Tag) -> [Quote]:

Choose a reason for hiding this comment

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

The return type hint [Quote] should be List[Quote] from the typing module for better clarity and to avoid confusion. This requires importing List from typing.

return [parse_single_quote(quote) for quote in quotes]


def get_all_quotes() -> [Quote]:

Choose a reason for hiding this comment

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

Similar to the previous comment, the return type hint [Quote] should be List[Quote] from the typing module. Ensure to import List from typing.

return all_quotes


def write_quotes_to_csv(quotes: [Quote], path: str) -> None:

Choose a reason for hiding this comment

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

The type hint for the quotes parameter should be List[Quote] instead of [Quote] for consistency and clarity. Import List from typing if not already done.

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