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 fromisoformat from standard library (or backport) #2078

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

lafrech
Copy link
Member

@lafrech lafrech commented Dec 8, 2022

This PR shows what it would look like if we used Python 3.11's improved fromisoformat (backported for older releases).

Less code on our side and it should be more efficient (C implementation).

(We could simplify even more by passing those functions directly to the class and remove everything from utils.py.)

There are 2 failing tests.

  • "2018-01-01" is accepted as a datetime while it is currently rejected in marshmallow. I guess this would be a breaking change. We may keep the old behaviour with a temporary trick until marshmallow 4 (keep the regex or even just check input string length).

  • Time deserialization respects timezones. It is not clear to me what a time with timezone but without date means. It could be ambiguous for timezones with DST. In any case, this can be considered either bugfix or breaking change, and it could be "fixed" to keep old behaviour.


Consider this as an RFC.

Meanwhile, anyone willing to do it in their app may set those functions as class attributes and use the backport if needed.

@lafrech
Copy link
Member Author

lafrech commented Dec 8, 2022

We may want to keep things as is in marshmallow 3 but I think we should be using standard lib fromisoformat in marshmallow 4 and this is at least a preview of the minor changes that involves.

@lafrech lafrech added this to the 4.0 milestone Dec 11, 2022
@lafrech
Copy link
Member Author

lafrech commented Sep 14, 2023

I'm leaning towards using this and accepting the slightly breaking changes.

The docs mention limitations. We could check wether our current implementation also has those limitations.

https://docs.python.org/3/library/datetime.html#datetime.datetime.fromisoformat

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.

1 participant