-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
BUG: nonexistent Timestamp pre-summer/winter DST w/dateutil timezone #31155
Merged
Merged
Changes from 13 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
651a55f
BUG: nonexistent Timestamp pre-summer/winter DST change with dateutil…
AlexKirko c9a87bd
switch from scientific number notation
AlexKirko ca34eed
move back to scientific notation to reset tests
AlexKirko 65b3bb8
move away from scientific notation to reset tests
AlexKirko 1eb9500
add casts to force correct division
AlexKirko 2f3850e
switch to dateutil implementation
AlexKirko 6c87f1b
TST: expand test for debugging
AlexKirko 43f6645
TST: fix test expansion
AlexKirko b1defde
go back to previous implementation
AlexKirko e46c774
hopefully a more robust implementation
AlexKirko 4f8b490
add rounding
AlexKirko f8dfb36
TST: remove exception from another test
AlexKirko 3ad3212
add more to the test
AlexKirko 2174ca0
try removing rounding, see if it breaks
AlexKirko 0cf53c1
add value stability and skip condition to the test
AlexKirko 3f79c3d
TST: add xfails for dateutil version < 2.7.0
AlexKirko 1c147d3
TST: remove duplicate test
AlexKirko f03e7c9
TST: remove packaging import to find the error
AlexKirko c88e354
TST: switch to LooseVersion for dateutil version check
AlexKirko 109a285
TST: use get_distribution to correct mypy error
AlexKirko 8e754f7
TST: fix value stability test
AlexKirko 894ed16
Revert "TST: fix value stability test"
AlexKirko 614176f
Revert Revert "TST: fix value stability test"
AlexKirko 56a3e71
TST: move to compat._optional._get_version
AlexKirko 89a5c01
Merge branch 'master' into fix-nonexistent-time
AlexKirko 9d21a81
restart checks
AlexKirko File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1092,3 +1092,17 @@ def test_constructor_ambigous_dst(): | |
expected = ts.value | ||
result = Timestamp(ts).value | ||
assert result == expected | ||
|
||
|
||
def test_constructor_before_dst_switch(): | ||
# GH 31043 | ||
# Make sure that calling Timestamp constructor | ||
# on time just before DST switch doesn't lead to | ||
# nonexistent time | ||
epoch = 1552211999999999872 | ||
ts = Timestamp(epoch, tz="dateutil/US/Pacific") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Forgot to mention earlier: you should probably use |
||
delta = ts.replace(tzinfo=None) - datetime.utcfromtimestamp(0) | ||
assert 1552183199.99 < delta.total_seconds() < 1552183200 | ||
expected = timedelta(seconds=0) | ||
result = ts.tz.dst(ts) | ||
assert result == expected | ||
AlexKirko marked this conversation as resolved.
Show resolved
Hide resolved
|
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.
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.
Can we test the example in the issue here? (i.e. that
Timestamp(Timestamp(epoch_time, tz=..))
doesn't change value)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.
@mroeschke Sure!
But the value breaks only 1 nanosecond before the switch. Currently, I'm struggling to make everything else work on every azure pipeline (see comment below).
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.
@mroeschke
Added to the test. Pinging you on change.