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

Fix api #21

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

Fix api #21

wants to merge 3 commits into from

Conversation

lillykml
Copy link
Collaborator

@lillykml lillykml commented Sep 2, 2021

I had the issue that I didn't get any recipes if I didn't add additional ingredients. Then I saw that we join them basically like this f"{list_a}", {list_b}" which gives results like "tomato, "
Also I took out the offset parameter to get better results, since we anyway display only 3 results

Copy link
Collaborator Author

@lillykml lillykml left a comment

Choose a reason for hiding this comment

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

Btw I'm not sure what the second commit means the merge branch master, so pls make sure I don't break anything :D - I don't remember merging anything tbh

@@ -47,7 +47,8 @@ def get_recipes(ingredients, exclusions, cuisine, diet):
# The number of results to skip (between 0 and 900).
# This is a bit risky in cases where less recipes were found than the offset value;
# --> the API is then returning no recipes.
"offset": offset}
#"offset": offset
Copy link
Collaborator

Choose a reason for hiding this comment

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

I added this for good reasons: to get (hopefully) different results every time the user is clicking the button. Without the offset, the user will always see the same recipes when no options are changed.

@kickermeister
Copy link
Collaborator

I'm surprised that the API has an issue with something like "tomatoe, " since it expects the parameter here as. comma-separated value anyway.. however, if it works more reliable that way I'm fine with it.

The offset parameter, however, would I prefer to keep.

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