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

Skip unset attributes #24

Merged
merged 3 commits into from
Jun 30, 2021
Merged

Conversation

cydanil
Copy link
Contributor

@cydanil cydanil commented Jun 28, 2021

Hi Micah,
Thanks again for your library!

This PR aims to solve a parsing issue when time are not set.

Drinks at allrecipes.com tend to not have a cooking time (cookTime) set.
In their schema, the value is set to null:


{
  "@context": "http://schema.org",
  "@type": "Recipe",

[...]

  "prepTime": "P0DT0H10M",
  "cookTime": null,
  "totalTime":  "P0DT0H10M",
}

In their webpage, the null value is used to hide fields. A couple of examples are:

https://www.allrecipes.com/recipe/237874/simple-moscow-mule/
https://www.allrecipes.com/recipe/272987/watermelon-mojitos/

Here, we add a check for None and remove the key from the recipe dictionary.
I've also added a unit test with one of the above recipe checking that the key is correctly removed.

Thanks!
Cyril

@cydanil
Copy link
Contributor Author

cydanil commented Jun 29, 2021

I added the attribution for the test recipe I added, but I'm not sure what's the license...

@micahcochran micahcochran merged commit 534dd5f into micahcochran:master Jun 30, 2021
@micahcochran
Copy link
Owner

Hello Cyril,

Thank you for the patch. Everything looks good.

I'd say the license for Allrecipes is Proprietary. I've consciously shied away from proprietary sources, but this seems to be fair use.

Travis CI isn't working, probably due to their migration from .org to .com website and I have probably not updated some setting. :-(

Thanks,
Micah

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