-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: Fix Timestamp constructor changes value on ambiguous DST #30995
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
Changes from all commits
2aa90a0
b4fd497
5053889
322178c
85767af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ from pandas._libs.tslibs.util cimport ( | |
from pandas._libs.tslibs.timedeltas cimport cast_from_unit | ||
from pandas._libs.tslibs.timezones cimport ( | ||
is_utc, is_tzlocal, is_fixed_offset, get_utcoffset, get_dst_info, | ||
get_timezone, maybe_get_tz, tz_compare) | ||
get_timezone, maybe_get_tz, tz_compare, treat_tz_as_dateutil) | ||
from pandas._libs.tslibs.timezones import UTC | ||
from pandas._libs.tslibs.parsing import parse_datetime_string | ||
|
||
|
@@ -362,6 +362,14 @@ cdef _TSObject convert_datetime_to_tsobject(datetime ts, object tz, | |
obj.tzinfo = tz | ||
else: | ||
obj.value = pydatetime_to_dt64(ts, &obj.dts) | ||
# GH 24329 When datetime is ambiguous, | ||
# pydatetime_to_dt64 doesn't take DST into account | ||
# but with dateutil timezone, get_utcoffset does | ||
# so we need to correct for it | ||
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. Added a comment explaining the reasoning for this solution. Tried to make it as brief as possible. |
||
if treat_tz_as_dateutil(ts.tzinfo): | ||
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. @mroeschke Switched to |
||
if ts.tzinfo.is_ambiguous(ts): | ||
dst_offset = ts.tzinfo.dst(ts) | ||
obj.value += int(dst_offset.total_seconds() * 1e9) | ||
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. Because 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. can you add a comment to this effect here |
||
obj.tzinfo = ts.tzinfo | ||
|
||
if obj.tzinfo is not None and not is_utc(obj.tzinfo): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -573,13 +573,7 @@ def test_dti_construction_ambiguous_endpoint(self, tz): | |
"2013-10-26 23:00", "2013-10-27 01:00", freq="H", tz=tz, ambiguous="infer" | ||
) | ||
assert times[0] == Timestamp("2013-10-26 23:00", tz=tz, freq="H") | ||
|
||
if str(tz).startswith("dateutil"): | ||
# fixed ambiguous behavior | ||
# see GH#14621 | ||
assert times[-1] == Timestamp("2013-10-27 01:00:00+0100", tz=tz, freq="H") | ||
else: | ||
assert times[-1] == Timestamp("2013-10-27 01:00:00+0000", tz=tz, freq="H") | ||
assert times[-1] == Timestamp("2013-10-27 01:00:00+0000", tz=tz, freq="H") | ||
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. Now no matter whether we use |
||
|
||
@pytest.mark.parametrize( | ||
"tz, option, expected", | ||
|
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.
Moved to whatsnew for v1.1.0