From 2aa90a03a1ecc4cb93f8165bd5513c22778b4fd2 Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Mon, 13 Jan 2020 14:41:09 +0300 Subject: [PATCH 1/5] BUG: fix Timestamp constructor value change on DST Timestamp --- doc/source/whatsnew/v1.0.0.rst | 1 + pandas/_libs/tslibs/conversion.pyx | 7 +++++++ pandas/tests/indexes/datetimes/test_timezones.py | 8 +------- pandas/tests/scalar/timestamp/test_timestamp.py | 11 +++++++++++ 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/doc/source/whatsnew/v1.0.0.rst b/doc/source/whatsnew/v1.0.0.rst index c423933d4c438..0a3eb18b1f44a 100755 --- a/doc/source/whatsnew/v1.0.0.rst +++ b/doc/source/whatsnew/v1.0.0.rst @@ -968,6 +968,7 @@ Datetimelike - Bug in :func:`date_range` with custom business hours as ``freq`` and given number of ``periods`` (:issue:`30593`) - Bug in :class:`PeriodIndex` comparisons with incorrectly casting integers to :class:`Period` objects, inconsistent with the :class:`Period` comparison behavior (:issue:`30722`) - Bug in :meth:`DatetimeIndex.insert` raising a ``ValueError`` instead of a ``TypeError`` when trying to insert a timezone-aware :class:`Timestamp` into a timezone-naive :class:`DatetimeIndex`, or vice-versa (:issue:`30806`) +- Bug in :class:`Timestamp` where constructing :class:`Timestamp` from ambiguous epoch time and calling constructor again changed :meth:`Timestamp.value` property (:issue:`24329`) Timedelta ^^^^^^^^^ diff --git a/pandas/_libs/tslibs/conversion.pyx b/pandas/_libs/tslibs/conversion.pyx index c8d354328a0f6..36aa34a0cbb83 100644 --- a/pandas/_libs/tslibs/conversion.pyx +++ b/pandas/_libs/tslibs/conversion.pyx @@ -5,6 +5,8 @@ cimport numpy as cnp from numpy cimport int64_t, int32_t, intp_t, ndarray cnp.import_array() +from dateutil import zoneinfo + import pytz # stdlib datetime imports @@ -362,6 +364,11 @@ cdef _TSObject convert_datetime_to_tsobject(datetime ts, object tz, obj.tzinfo = tz else: obj.value = pydatetime_to_dt64(ts, &obj.dts) + # GH 24329 Take DST offset into account + if isinstance(ts.tzinfo, zoneinfo.tzfile): + if ts.tzinfo.is_ambiguous(ts): + dst_offset = ts.tzinfo.dst(ts) + obj.value += int(dst_offset.total_seconds() * 1e9) obj.tzinfo = ts.tzinfo if obj.tzinfo is not None and not is_utc(obj.tzinfo): diff --git a/pandas/tests/indexes/datetimes/test_timezones.py b/pandas/tests/indexes/datetimes/test_timezones.py index 1505ac1dff29c..df64820777f3f 100644 --- a/pandas/tests/indexes/datetimes/test_timezones.py +++ b/pandas/tests/indexes/datetimes/test_timezones.py @@ -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") @pytest.mark.parametrize( "tz, option, expected", diff --git a/pandas/tests/scalar/timestamp/test_timestamp.py b/pandas/tests/scalar/timestamp/test_timestamp.py index f1fcf46a936fd..c60406fdbc8a6 100644 --- a/pandas/tests/scalar/timestamp/test_timestamp.py +++ b/pandas/tests/scalar/timestamp/test_timestamp.py @@ -1081,3 +1081,14 @@ def test_dt_subclass_add_timedelta(lh, rh): result = lh + rh expected = SubDatetime(2000, 1, 1, 1) assert result == expected + + +def test_constructor_ambigous_dst(): + # GH 24329 + # Make sure that calling Timestamp constructor + # on Timestamp created from ambiguous time + # doesn't change Timestamp.value + ts = Timestamp(1382835600000000000, tz="dateutil/Europe/London") + expected = ts.value + result = Timestamp(ts).value + assert result == expected From b4fd497411019a929c2e320a1b08eb4a76fe8d11 Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Tue, 14 Jan 2020 16:13:10 +0300 Subject: [PATCH 2/5] BUG: expand if check for Linux --- pandas/_libs/tslibs/conversion.pyx | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pandas/_libs/tslibs/conversion.pyx b/pandas/_libs/tslibs/conversion.pyx index 36aa34a0cbb83..e3601f78e7edb 100644 --- a/pandas/_libs/tslibs/conversion.pyx +++ b/pandas/_libs/tslibs/conversion.pyx @@ -5,7 +5,8 @@ cimport numpy as cnp from numpy cimport int64_t, int32_t, intp_t, ndarray cnp.import_array() -from dateutil import zoneinfo +from dateutil.zoneinfo import tzfile as du_tzfile1 +from dateutil.tz.tz import tzfile as du_tzfile2 import pytz @@ -365,7 +366,10 @@ cdef _TSObject convert_datetime_to_tsobject(datetime ts, object tz, else: obj.value = pydatetime_to_dt64(ts, &obj.dts) # GH 24329 Take DST offset into account - if isinstance(ts.tzinfo, zoneinfo.tzfile): + # Two check in if necessary because class + # differs on Windows and Linux + if (isinstance(ts.tzinfo, du_tzfile1) or + isinstance(ts.tzinfo, du_tzfile2)): if ts.tzinfo.is_ambiguous(ts): dst_offset = ts.tzinfo.dst(ts) obj.value += int(dst_offset.total_seconds() * 1e9) From 5053889c665b9053628a4fc2fc941711fcae0573 Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Tue, 14 Jan 2020 19:04:26 +0300 Subject: [PATCH 3/5] CLN: switch from isinstance to treat_tz_asdateutil --- pandas/_libs/tslibs/conversion.pyx | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/pandas/_libs/tslibs/conversion.pyx b/pandas/_libs/tslibs/conversion.pyx index e3601f78e7edb..b159567e512b8 100644 --- a/pandas/_libs/tslibs/conversion.pyx +++ b/pandas/_libs/tslibs/conversion.pyx @@ -5,9 +5,6 @@ cimport numpy as cnp from numpy cimport int64_t, int32_t, intp_t, ndarray cnp.import_array() -from dateutil.zoneinfo import tzfile as du_tzfile1 -from dateutil.tz.tz import tzfile as du_tzfile2 - import pytz # stdlib datetime imports @@ -32,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 @@ -366,10 +363,7 @@ cdef _TSObject convert_datetime_to_tsobject(datetime ts, object tz, else: obj.value = pydatetime_to_dt64(ts, &obj.dts) # GH 24329 Take DST offset into account - # Two check in if necessary because class - # differs on Windows and Linux - if (isinstance(ts.tzinfo, du_tzfile1) or - isinstance(ts.tzinfo, du_tzfile2)): + if treat_tz_as_dateutil(ts.tzinfo): if ts.tzinfo.is_ambiguous(ts): dst_offset = ts.tzinfo.dst(ts) obj.value += int(dst_offset.total_seconds() * 1e9) From 322178c9cca6f6f325ea87b46c00ddccb06718d9 Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Wed, 15 Jan 2020 09:56:35 +0300 Subject: [PATCH 4/5] CLN: add comment to solution and move to v1.1.0 --- doc/source/whatsnew/v1.0.0.rst | 1 - doc/source/whatsnew/v1.1.0.rst | 1 + pandas/_libs/tslibs/conversion.pyx | 3 +++ 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.0.0.rst b/doc/source/whatsnew/v1.0.0.rst index 0a3eb18b1f44a..c423933d4c438 100755 --- a/doc/source/whatsnew/v1.0.0.rst +++ b/doc/source/whatsnew/v1.0.0.rst @@ -968,7 +968,6 @@ Datetimelike - Bug in :func:`date_range` with custom business hours as ``freq`` and given number of ``periods`` (:issue:`30593`) - Bug in :class:`PeriodIndex` comparisons with incorrectly casting integers to :class:`Period` objects, inconsistent with the :class:`Period` comparison behavior (:issue:`30722`) - Bug in :meth:`DatetimeIndex.insert` raising a ``ValueError`` instead of a ``TypeError`` when trying to insert a timezone-aware :class:`Timestamp` into a timezone-naive :class:`DatetimeIndex`, or vice-versa (:issue:`30806`) -- Bug in :class:`Timestamp` where constructing :class:`Timestamp` from ambiguous epoch time and calling constructor again changed :meth:`Timestamp.value` property (:issue:`24329`) Timedelta ^^^^^^^^^ diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index 721bcb0758992..b5a7b19f160a4 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -59,6 +59,7 @@ Categorical Datetimelike ^^^^^^^^^^^^ +- Bug in :class:`Timestamp` where constructing :class:`Timestamp` from ambiguous epoch time and calling constructor again changed :meth:`Timestamp.value` property (:issue:`24329`) - - diff --git a/pandas/_libs/tslibs/conversion.pyx b/pandas/_libs/tslibs/conversion.pyx index b159567e512b8..594f6ec664754 100644 --- a/pandas/_libs/tslibs/conversion.pyx +++ b/pandas/_libs/tslibs/conversion.pyx @@ -363,6 +363,9 @@ cdef _TSObject convert_datetime_to_tsobject(datetime ts, object tz, else: obj.value = pydatetime_to_dt64(ts, &obj.dts) # GH 24329 Take DST offset into account + # pydatetime_to_dt64 doesn't take DST into account + # but get_utcoffset does. dateutil assumes DST + # when time is ambiguous, so we need to correct for it if treat_tz_as_dateutil(ts.tzinfo): if ts.tzinfo.is_ambiguous(ts): dst_offset = ts.tzinfo.dst(ts) From 85767afcc9de4cfef0c1bd46f801f4eb11c8b82e Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Wed, 15 Jan 2020 10:33:56 +0300 Subject: [PATCH 5/5] DOC: reword bugfix comment --- pandas/_libs/tslibs/conversion.pyx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pandas/_libs/tslibs/conversion.pyx b/pandas/_libs/tslibs/conversion.pyx index 594f6ec664754..77f46016ee846 100644 --- a/pandas/_libs/tslibs/conversion.pyx +++ b/pandas/_libs/tslibs/conversion.pyx @@ -362,10 +362,10 @@ cdef _TSObject convert_datetime_to_tsobject(datetime ts, object tz, obj.tzinfo = tz else: obj.value = pydatetime_to_dt64(ts, &obj.dts) - # GH 24329 Take DST offset into account + # GH 24329 When datetime is ambiguous, # pydatetime_to_dt64 doesn't take DST into account - # but get_utcoffset does. dateutil assumes DST - # when time is ambiguous, so we need to correct for it + # but with dateutil timezone, get_utcoffset does + # so we need to correct for it if treat_tz_as_dateutil(ts.tzinfo): if ts.tzinfo.is_ambiguous(ts): dst_offset = ts.tzinfo.dst(ts)