-
Notifications
You must be signed in to change notification settings - Fork 192
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: python datetime [de]serialization bug #734
Conversation
|
||
|
||
converter31.register_structure_hook(datetime.datetime, datetime_structure_hook) | ||
converter40.register_structure_hook(datetime.datetime, datetime_structure_hook) | ||
cattr.register_unstructure_hook(model.Model, unstructure_hook) # type: ignore | ||
cattr.register_unstructure_hook(datetime.datetime, lambda dt: dt.strftime(DATETIME_FMT)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the whole fix
@@ -99,6 +99,7 @@ class Model(ml.Model): | |||
# standard types | |||
id: Optional[int] = None | |||
name: Optional[str] = None | |||
datetime_field: Optional[datetime.datetime] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding this datetime_field
everywhere in the tests identically reproduced the issue in #720
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good fix
@@ -144,6 +144,7 @@ def unstructure_hook(api_model): | |||
return data | |||
|
|||
|
|||
DATETIME_FMT = "%Y-%m-%dT%H:%M:%S.%f%z" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the infinite syntax variation for datetime string formatting
Python Tests 10 files 10 suites 1m 55s ⏱️ Results for commit 5032ce4. |
we'd forgotten to register a
cattr.register_unstructure_hook
fordatetime.datetime
objects.Clearly no one had been trying to send in any data that included a datetime.datetime till now
closes #720