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
don't automatically unpack datetime objects in the message spec #4497
Conversation
protects user-defined strings from being deserialized to datetime objects
closes ipython#4493
datetime objects are no longer automatically deserialized outside of headers.
looks good. |
Yes, I'll add some specifically for the changed behavior |
@@ -675,7 +675,7 @@ def _extract_metadata(self, msg): | |||
if 'date' in parent: | |||
md['submitted'] = parent['date'] | |||
if 'started' in msg_meta: | |||
md['started'] = msg_meta['started'] | |||
md['started'] = extract_dates(msg_meta['started']) |
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.
Do we want to separate out the function for turning a single iso8601 string into a datetime, so that it's clearer where we're handling a single value as opposed to a container?
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.
I could, though almost everywhere there might be a datetime the value could also be None
, and extract_dates
handles that logic.
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.
I'd still be inclined to split that bit out - extract_dates
could simply call the new function on anything that's not a list or a dict.
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.
yup, will do.
makes code more readable
header and parent_header date time objects are automatically parsed. metadata and content are not, but at least they are automatically serialized.
added parse_date for single-object parsing, and a few more tests of datetime handling for jsonutil and Session. |
👍 |
Merging from my phone (at least trying) |
don't automatically unpack datetime objects in the message spec only unpack in headers move handling of datetime objects in content and metadata to application-level code minor fix to ISO8601 re (closes #4493)
don't automatically unpack datetime objects in the message spec only unpack in headers move handling of datetime objects in content and metadata to application-level code minor fix to ISO8601 re (closes ipython#4493)
I'm relying a bit on the IPython.parallel test coverage, so we'll see how this goes.
If anything is broken after this, at least we know where we need to add tests.