[3.9] Check result of utc_to_seconds and skip fold probe in pure Python (GH-91581) #92748
Conversation
This file contains hidden or 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
…thonGH-91582) The `utc_to_seconds` call can fail, here's a minimal reproducer on Linux: TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)" The old behavior still raised an error in a similar way, but only because subsequent calculations happened to fail as well. Better to fail fast. This also refactors the tests to split out the `fromtimestamp` and `utcfromtimestamp` tests, and to get us closer to the actual desired limits of the functions. As part of this, we also changed the way we detect platforms where the same limits don't necessarily apply (e.g. Windows). As part of refactoring the tests to hit this condition explicitly (even though the user-facing behvior doesn't change in any way we plan to guarantee), I noticed that there was a difference in the places that `datetime.utcfromtimestamp` fails in the C and pure Python versions, which was fixed by skipping the "probe for fold" logic for UTC specifically — since UTC doesn't have any folds or gaps, we were never going to find a fold value anyway. This should prevent some failures in the pure python `utcfromtimestamp` method on timestamps close to 0001-01-01. There are two separate news entries for this because one is a potentially user-facing change, the other is an internal code correctness change that, if anything, changes some error messages. The two happen to be coupled because of the test refactoring, but they are probably best thought of as independent changes. Fixes pythonGH-91581 (cherry picked from commit 83c0247) Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
|
@pganssle: Status check is done, and it's a failure ❌ . |
1 similar comment
|
@pganssle: Status check is done, and it's a failure ❌ . |
|
@pganssle: Status check is done, and it's a failure ❌ . |
pganssle
approved these changes
May 12, 2022
hello-adam
pushed a commit
to hello-adam/cpython
that referenced
this pull request
Jun 2, 2022
…thonGH-91582) (pythonGH-92748) The `utc_to_seconds` call can fail, here's a minimal reproducer on Linux: TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)" The old behavior still raised an error in a similar way, but only because subsequent calculations happened to fail as well. Better to fail fast. This also refactors the tests to split out the `fromtimestamp` and `utcfromtimestamp` tests, and to get us closer to the actual desired limits of the functions. As part of this, we also changed the way we detect platforms where the same limits don't necessarily apply (e.g. Windows). As part of refactoring the tests to hit this condition explicitly (even though the user-facing behvior doesn't change in any way we plan to guarantee), I noticed that there was a difference in the places that `datetime.utcfromtimestamp` fails in the C and pure Python versions, which was fixed by skipping the "probe for fold" logic for UTC specifically — since UTC doesn't have any folds or gaps, we were never going to find a fold value anyway. This should prevent some failures in the pure python `utcfromtimestamp` method on timestamps close to 0001-01-01. There are two separate news entries for this because one is a potentially user-facing change, the other is an internal code correctness change that, if anything, changes some error messages. The two happen to be coupled because of the test refactoring, but they are probably best thought of as independent changes. Fixes pythonGH-91581 (cherry picked from commit 83c0247) Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
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.
The
utc_to_secondscall can fail, here's a minimal reproducer onLinux:
TZ=UTC python -c "from datetime import *; datetime.fromtimestamp(253402300799 + 1)"
The old behavior still raised an error in a similar way, but only
because subsequent calculations happened to fail as well. Better to fail
fast.
This also refactors the tests to split out the
fromtimestampandutcfromtimestamptests, and to get us closer to the actual desiredlimits of the functions. As part of this, we also changed the way we
detect platforms where the same limits don't necessarily apply (e.g.
Windows).
As part of refactoring the tests to hit this condition explicitly (even
though the user-facing behvior doesn't change in any way we plan to
guarantee), I noticed that there was a difference in the places that
datetime.utcfromtimestampfails in the C and pure Python versions, whichwas fixed by skipping the "probe for fold" logic for UTC specifically —
since UTC doesn't have any folds or gaps, we were never going to find a
fold value anyway. This should prevent some failures in the pure python
utcfromtimestampmethod on timestamps close to 0001-01-01.There are two separate news entries for this because one is a
potentially user-facing change, the other is an internal code
correctness change that, if anything, changes some error messages. The
two happen to be coupled because of the test refactoring, but they are
probably best thought of as independent changes.
Fixes GH-91581
(cherry picked from commit 83c0247)
Co-authored-by: Paul Ganssle 1377457+pganssle@users.noreply.github.com