Skip to content

BUG: nonexistent Timestamp pre-summer/winter DST w/dateutil timezone #31155

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

Merged
merged 26 commits into from
Jan 24, 2020
Merged
Show file tree
Hide file tree
Changes from 20 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 Jan 20, 2020
c9a87bd
switch from scientific number notation
AlexKirko Jan 20, 2020
ca34eed
move back to scientific notation to reset tests
AlexKirko Jan 20, 2020
65b3bb8
move away from scientific notation to reset tests
AlexKirko Jan 20, 2020
1eb9500
add casts to force correct division
AlexKirko Jan 20, 2020
2f3850e
switch to dateutil implementation
AlexKirko Jan 20, 2020
6c87f1b
TST: expand test for debugging
AlexKirko Jan 20, 2020
43f6645
TST: fix test expansion
AlexKirko Jan 20, 2020
b1defde
go back to previous implementation
AlexKirko Jan 20, 2020
e46c774
hopefully a more robust implementation
AlexKirko Jan 21, 2020
4f8b490
add rounding
AlexKirko Jan 21, 2020
f8dfb36
TST: remove exception from another test
AlexKirko Jan 21, 2020
3ad3212
add more to the test
AlexKirko Jan 21, 2020
2174ca0
try removing rounding, see if it breaks
AlexKirko Jan 21, 2020
0cf53c1
add value stability and skip condition to the test
AlexKirko Jan 21, 2020
3f79c3d
TST: add xfails for dateutil version < 2.7.0
AlexKirko Jan 21, 2020
1c147d3
TST: remove duplicate test
AlexKirko Jan 21, 2020
f03e7c9
TST: remove packaging import to find the error
AlexKirko Jan 21, 2020
c88e354
TST: switch to LooseVersion for dateutil version check
AlexKirko Jan 21, 2020
109a285
TST: use get_distribution to correct mypy error
AlexKirko Jan 21, 2020
8e754f7
TST: fix value stability test
AlexKirko Jan 22, 2020
894ed16
Revert "TST: fix value stability test"
AlexKirko Jan 22, 2020
614176f
Revert Revert "TST: fix value stability test"
AlexKirko Jan 22, 2020
56a3e71
TST: move to compat._optional._get_version
AlexKirko Jan 22, 2020
89a5c01
Merge branch 'master' into fix-nonexistent-time
AlexKirko Jan 22, 2020
9d21a81
restart checks
AlexKirko Jan 22, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion doc/source/whatsnew/v1.1.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@ 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`)
- :meth:`DatetimeArray.searchsorted`, :meth:`TimedeltaArray.searchsorted`, :meth:`PeriodArray.searchsorted` not recognizing non-pandas scalars and incorrectly raising ``ValueError`` instead of ``TypeError`` (:issue:`30950`)
-
- Bug in :class:`Timestamp` where constructing :class:`Timestamp` with dateutil timezone less than 128 nanoseconds before daylight saving time switch from winter to summer would result in nonexistent time (:issue:`31043`)

Timedelta
^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion pandas/_libs/tslibs/nattype.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ cdef class _NaT(datetime):

def total_seconds(self):
"""
Total duration of timedelta in seconds (to ns precision).
Total duration of timedelta in seconds (to microsecond precision).
"""
# GH#10939
return np.nan
Expand Down
6 changes: 4 additions & 2 deletions pandas/_libs/tslibs/timedeltas.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -861,9 +861,11 @@ cdef class _Timedelta(timedelta):

def total_seconds(self):
"""
Total duration of timedelta in seconds (to ns precision).
Total duration of timedelta in seconds (to microsecond precision).
"""
return self.value / 1e9
# GH 31043
# Microseconds precision to avoid confusing tzinfo.utcoffset
return (self.value - self.value % 1000) / 1e9

def view(self, dtype):
"""
Expand Down
8 changes: 7 additions & 1 deletion pandas/tests/indexes/datetimes/test_timezones.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
Tests for DatetimeIndex timezone-related methods
"""
from datetime import date, datetime, time, timedelta, tzinfo
from distutils.version import LooseVersion
Copy link
Member Author

@AlexKirko AlexKirko Jan 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to parse version properly, so have to import this.


import dateutil
from dateutil.tz import gettz, tzlocal
import numpy as np
from pkg_resources import get_distribution
import pytest
import pytz

Expand Down Expand Up @@ -585,7 +587,11 @@ def test_dti_construction_ambiguous_endpoint(self, tz):
"dateutil/US/Pacific",
"shift_backward",
"2019-03-10 01:00",
marks=pytest.mark.xfail(reason="GH 24329"),
marks=pytest.mark.xfail(
LooseVersion(get_distribution("python-dateutil").version)
< LooseVersion("2.7.0"),
reason="GH 31043",
),
),
["US/Pacific", timedelta(hours=1), "2019-03-10 03:00"],
],
Expand Down
22 changes: 22 additions & 0 deletions pandas/tests/scalar/timestamp/test_timestamp.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@

import calendar
from datetime import datetime, timedelta
from distutils.version import LooseVersion
import locale
import unicodedata

import dateutil
from dateutil.tz import tzutc
import numpy as np
from pkg_resources import get_distribution
import pytest
import pytz
from pytz import timezone, utc
Expand Down Expand Up @@ -1092,3 +1094,23 @@ def test_constructor_ambigous_dst():
expected = ts.value
result = Timestamp(ts).value
assert result == expected


@pytest.mark.xfail(
LooseVersion(get_distribution("python-dateutil").version) < LooseVersion("2.7.0"),
reason="dateutil moved to Timedelta.total_seconds() in 2.7.0",
)
@pytest.mark.parametrize("epoch", [1552211999999999872, 1552211999999999999])
def test_constructor_before_dst_switch(epoch):
# GH 31043
# Make sure that calling Timestamp constructor
# on time just before DST switch doesn't lead to
# nonexistent time or value change
# Works only with dateutil >= 2.7.0 as dateutil overrid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/overrid/overrode

# pandas.Timedelta.total_seconds with
# datetime.timedelta.total_seconds before
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not make sense to me - datetime.timedelta.total_seconds should succeed, because it's equivalent to:

def total_seconds(td):
  useconds = td.days * 86400
  useconds += td.seconds 
  useconds *= 1000000
  useconds += td.microseconds
  return useconds / 1e6

I think there's actually a deeper issue here, which is that td.microseconds and td.seconds are rounded rather than truncated. Consider this:

def to_ns(td):
  ns = td.days * 86400
  ns += td.seconds
  ns *= 1000000
  ns += td.microseconds
  ns *= 1000
  ns += td.nanoseconds
  return ns

td = timedelta(1552211999999999872, unit="ns")
print(td.value)  # 1552211999999999872
print(to_ns(td))  # 1552212000000000872

That seems to be the actual root cause of this issue and should probably be fixed.

ts = Timestamp(epoch, tz="dateutil/US/Pacific")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to mention earlier: you should probably use dateutil/America/Los_Angeles, as that is the canonical name for this zone. The US/... zones are symlinks for backwards compatibility.

result = ts.tz.dst(ts)
expected = timedelta(seconds=0)
assert ts.value == epoch
assert result == expected