Fix JSON encoding of datetimes with zoneinfo tzinfos #534
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
86f7305 to
b65b7cd
Compare
This fixes a bug where datetime objects with `zoneinfo.ZoneInfo` instances set for `tzinfo` would encode as UTC, ignoring the zoneinfo. This issue was due to `datetime.utcoffset` being called with `None` instead of the `datetime` instance. This worked fine for other `tzinfo` types (like `datetime.timezone`) since those use fixed offsets that aren't date aware. This also fixes a bug where `datetime.time` instances with a `tzinfo` that returns `None` from `datetime.time.utcoffset` were treated as UTC rather than naive. Quoting from the standard library docs: > A datetime object d is aware if both of the following hold: > > 1. d.tzinfo is not None > 2. d.tzinfo.utcoffset(d) does not return None > > Otherwise, d is naive. > > A time object t is aware if both of the following hold: > > 1. t.tzinfo is not None > 2. t.tzinfo.utcoffset(None) does not return None. > > Otherwise, t is naive. Both bugs fixed above were due to incorrect behavior around the 2nd condition using `utcoffset`.
b65b7cd to
ee2b306
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes a bug where datetime objects with
zoneinfo.ZoneInfoinstances set fortzinfowould encode as UTC, ignoring the zoneinfo. This issue was due todatetime.utcoffsetbeing called withNoneinstead of thedatetimeinstance. This worked fine for othertzinfotypes (likedatetime.timezone) since those use fixed offsets that aren't date aware.This also fixes a bug where
datetime.timeinstances with atzinfothat returnsNonefromdatetime.time.utcoffsetwere treated as UTC rather than naive. Quoting from the standard library docs:Both bugs fixed above were due to incorrect behavior around the 2nd condition using
utcoffset.Fixes #533.