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

Use isodate library to attempt ISO 8601 duration parsing in get_minutes #610

Merged
merged 5 commits into from Oct 12, 2022

Conversation

jayaddison
Copy link
Collaborator

@jayaddison jayaddison commented Sep 30, 2022

Use the isodate library to attempt to parse time durations from strings that look like potential ISO8601 date strings in get_minutes utility method.

Note that we already transitively depend on the isodate library (via extruct -> rdflib -> isodate).

The get_minutes method began failing a flake8 complexity check after this functionality was added; that method would benefit from some simplification and refactoring (see #551).

Fixes #606.

Note: we already currently depend on this library implicitly; it is required by rdflib which is required by extruct
…hat look like potential ISO8601 date strings in get_minutes utility method
Refactoring and simplification would be useful here
@bfcarpio
Copy link
Collaborator

bfcarpio commented Oct 1, 2022

Do you know what features this specifically brings us compared to the standard library implementation?

I don't necessarily mind formalizing a transitive dependency, but I have a (highly subjective) bias to using the stdlib when possible and ergonomic to.

@jayaddison
Copy link
Collaborator Author

@bfcarpio agreed - I share the same preference for the standard library when possible too.

Currently in the Python 3.10 stdlib, only datetime objects can be parsed from ISO 8601 format - timedelta objects (as we want for the time measurements here) cannot.

It's possible that this functionality may appear in Python at some point, though... python/cpython#86260 exists tracking that as a request.

@jayaddison jayaddison merged commit 39fa50b into main Oct 12, 2022
@jayaddison jayaddison deleted the issue-606/get-minutes-iso8601-durations branch October 12, 2022 10:24
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.

total_time(), cook_time() and prep_time() return error or 0 for recipes from thekitchn.com
2 participants