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

Handle case where ingredients are in subheaders #598

Conversation

ptoczko
Copy link
Contributor

@ptoczko ptoczko commented Sep 5, 2022

Fixes #595

Copy link
Collaborator

@jayaddison jayaddison left a comment

Choose a reason for hiding this comment

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

Thanks @ptoczko!

Local review: looks good, the fix works as expected 👍

>>> from recipe_scrapers import scrape_me
>>> scraper = scrape_me('https://www.greatbritishchefs.com/recipes/beef-mushroom-lasagne-recipe')
>>> print(scraper.ingredients())
['1 lasagne pasta, packet', '50g of Parmesan', 'oil', '2 onions, finely chopped', '700g of beef mince', '6 bacon rashers, diced', '3 tin of chopped tomatoes', '1 bay leaf', '250ml of red wine', '', '70g of butter', '50g of plain flour', '500ml of milk', '200g of chestnut mushrooms, sliced', '100g of button mushrooms, sliced', '100g of girolles, sliced', '1 handful of porcini mushroom', '6 sage leaves']

...plus no regressions discovered in the existing test coverage or the additional example provided in the issue.

An optional extra / nice-to-have: an additional scraper test for multi-heading ingredient lists on this website?

(and one small lint error to cleanup before merge)

@ptoczko
Copy link
Contributor Author

ptoczko commented Sep 5, 2022

Hey @jayaddison quick question i'm having trouble with precommit hooks was wondering if you've seen this before

    × Encountered error while trying to install package.
    ╰─> ruamel.yaml.clib

@jayaddison
Copy link
Collaborator

Hey @jayaddison quick question i'm having trouble with precommit hooks was wondering if you've seen this before

    × Encountered error while trying to install package.
    ╰─> ruamel.yaml.clib

Hey - can't say I have; that's weird. My guess is that this could be from a global commit hook, because recipe-scrapers doesn't add any of its' own.

Wild guess: perhaps a failure to find a pre-built wheel for your development OS/environment ( https://pypi.org/project/ruamel.yaml.clib/#files shows pre-built options available), followed by a failure to compile the package from scratch? If so, attempting to install the same package name into a local virtualenv might display the problem.

@ptoczko
Copy link
Contributor Author

ptoczko commented Sep 5, 2022

I think ive fixed the lint errors now

@jayaddison
Copy link
Collaborator

Nice work @ptoczko - thanks for adding the test coverage. lgtm 👍

@jayaddison jayaddison merged commit 9ecaec3 into hhursev:main Sep 5, 2022
@jayaddison
Copy link
Collaborator

Fix released to PyPi in recipe-scrapers v14.14.0 - thanks again!

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.

Missing ingredients when subheadings exist in ingredients list - greatbritishchefs.com
2 participants