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

Adds support for www.grandfrais.com #912

Merged
merged 1 commit into from
Oct 23, 2023
Merged

Conversation

xunleii
Copy link
Contributor

@xunleii xunleii commented Oct 22, 2023

Grand Frais is a French supermarket specializing in fresh produce and world groceries, and offers recipes on its website (https://www.grandfrais.com/recettes/)

@xunleii xunleii marked this pull request as ready for review October 22, 2023 13:00
@jayaddison
Copy link
Collaborator

Thank you @xunleii for contributing! I've been reviewing this and will add a few comments now.

Comment on lines +75 to +70
for fraction, replacement in FRACTIONS.items():
ingredients = [i.replace(fraction, str(replacement)) for i in ingredients]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems OK; I'm wondering whether we should create a utility function that can do this. But perhaps not until there are multiple places that do this kind of remapping.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was surprised it hadn't been used before for other websites.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be interesting to try moving it into the normalize_string method, and then to run the unit tests to see how many sites are affected.

I don't think we've had any complaints about the library returning fractions from sites before, though, so I suppose people are (mostly?) OK with it so far.

Signed-off-by: Alexandre Nicolaie <alexandre.nicolaie@gmail.com>
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.

This looks good to me - thanks again @xunleii. I'll likely merge this within the next day or so.

@jayaddison jayaddison merged commit 69d334f into hhursev:main Oct 23, 2023
16 checks passed
@jayaddison
Copy link
Collaborator

This functionality has been included in version 14.52.0 of the library - thank you, @xunleii!

strangetom pushed a commit to strangetom/recipe-scrapers that referenced this pull request Nov 12, 2023
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