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

Added scraper for IrishCentral #1416

Merged
merged 54 commits into from
Dec 6, 2024
Merged

Added scraper for IrishCentral #1416

merged 54 commits into from
Dec 6, 2024

Conversation

bencondemi
Copy link
Contributor

@bencondemi bencondemi commented Dec 3, 2024

Resolves #1167

With ingredients in "ul" structure and no yields ~ https://www.irishcentral.com/culture/food-drink/shepherds-pie-recipe
With ingredients in "p" structure and with yields ~ https://www.irishcentral.com/culture/food-drink/apple-jameson-tart-recipe

bencondemi and others added 30 commits November 26, 2024 20:23
Co-authored-by: Joey <[email protected]>
Co-authored-by: Joey <[email protected]>
Copy link
Collaborator

@jknndy jknndy left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Made some comments throughout, some of these changes will require test updates. Also once you're done with these be sure to run our test suite locally and the formatter as well. Let me know if you have any questions!

black .\recipe_scrapers\irishcentral.py && python -m unittest

The exception FieldNotProvidedByWebsiteException should only be used when the site never provides the field and should only be used for mandatory fields (not an issue here just mentioning). So there should only be one implementation for total_time in this instance.

recipe_scrapers/irishcentral.py Outdated Show resolved Hide resolved
recipe_scrapers/irishcentral.py Outdated Show resolved Hide resolved
recipe_scrapers/irishcentral.py Outdated Show resolved Hide resolved
recipe_scrapers/irishcentral.py Outdated Show resolved Hide resolved
recipe_scrapers/irishcentral.py Outdated Show resolved Hide resolved
recipe_scrapers/irishcentral.py Outdated Show resolved Hide resolved
recipe_scrapers/irishcentral.py Outdated Show resolved Hide resolved
tests/test_data/irishcentral.com/irishcentral.json Outdated Show resolved Hide resolved
recipe_scrapers/irishcentral.py Outdated Show resolved Hide resolved
recipe_scrapers/irishcentral.py Outdated Show resolved Hide resolved
bencondemi and others added 17 commits December 5, 2024 08:33
@bencondemi
Copy link
Contributor Author

Thanks for the contribution! Made some comments throughout, some of these changes will require test updates. Also once you're done with these be sure to run our test suite locally and the formatter as well. Let me know if you have any questions!

black .\recipe_scrapers\irishcentral.py && python -m unittest

The exception FieldNotProvidedByWebsiteException should only be used when the site never provides the field and should only be used for mandatory fields (not an issue here just mentioning). So there should only be one implementation for total_time in this instance.

@jknndy, I ran the formatter on the .py file, revised with your suggestions, and corrected some of the inconsistent test cases. Please let me know your thoughts.

@bencondemi
Copy link
Contributor Author

bencondemi commented Dec 5, 2024

@jknndy, I am having trouble passing the linter (seemingly due to isort inconsistencies within init.py). I have ran isort recipe_scrapers/__init__.py, which appeared to make several corrections within the file, and am wondering if you might be able to help me understand what else I can do to fix this?

For added context, I was also having isort issues within irishcentral.py; however, after I ran isort on that file, it passed the linter checks right away. Thanks!

recipe_scrapers/__init__.py Outdated Show resolved Hide resolved
@jknndy
Copy link
Collaborator

jknndy commented Dec 6, 2024

Thanks again for the contribution @bencondemi !

@jknndy jknndy merged commit 1a1e014 into hhursev:main Dec 6, 2024
19 checks passed
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.

IrishCentral
2 participants