Skip to content
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: tslibs uncaught overflows #55503

Merged
merged 3 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 5 additions & 3 deletions doc/source/whatsnew/v2.2.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,14 @@ Categorical
Datetimelike
^^^^^^^^^^^^
- Bug in :meth:`DatetimeIndex.union` returning object dtype for tz-aware indexes with the same timezone but different units (:issue:`55238`)
-
- Bug in :meth:`Tick.delta` with very large ticks raising ``OverflowError`` instead of ``OutOfBoundsTimedelta`` (:issue:`55503`)
- Bug in addition or subtraction of very large :class:`Tick` objects with :class:`Timestamp` or :class:`Timedelta` objects raising ``OverflowError`` instead of ``OutOfBoundsTimedelta`` (:issue:`55503`)


Timedelta
^^^^^^^^^
- Bug in :class:`Timedelta` construction raising ``OverflowError`` instead of ``OutOfBoundsTimedelta`` (:issue:`55503`)
- Bug in rendering (``__repr__``) of :class:`TimedeltaIndex` and :class:`Series` with timedelta64 values with non-nanosecond resolution entries that are all multiples of 24 hours failing to use the compact representation used in the nanosecond cases (:issue:`55405`)
-

Timezones
^^^^^^^^^
Expand Down Expand Up @@ -352,7 +354,7 @@ I/O

Period
^^^^^^
-
- Bug in :class:`Period` addition silently wrapping around instead of raising ``OverflowError`` (:issue:`55503`)
-

Plotting
Expand Down
7 changes: 6 additions & 1 deletion pandas/_libs/tslibs/offsets.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,12 @@ cdef class Tick(SingleConstructorOffset):

@property
def delta(self):
return self.n * Timedelta(self._nanos_inc)
try:
return self.n * Timedelta(self._nanos_inc)
except OverflowError as err:
# GH#55503 as_unit will raise a more useful OutOfBoundsTimedelta
Timedelta(self).as_unit("ns")
raise AssertionError("This should not be reached.")

@property
def nanos(self) -> int64_t:
Expand Down
7 changes: 4 additions & 3 deletions pandas/_libs/tslibs/period.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1814,7 +1814,7 @@ cdef class _Period(PeriodMixin):

def _add_timedeltalike_scalar(self, other) -> "Period":
cdef:
int64_t inc
int64_t inc, ordinal

if not self._dtype._is_tick_like():
raise IncompatibleFrequency("Input cannot be converted to "
Expand All @@ -1832,8 +1832,8 @@ cdef class _Period(PeriodMixin):
except ValueError as err:
raise IncompatibleFrequency("Input cannot be converted to "
f"Period(freq={self.freqstr})") from err
# TODO: overflow-check here
ordinal = self.ordinal + inc
with cython.overflowcheck(True):
ordinal = self.ordinal + inc
return Period(ordinal=ordinal, freq=self.freq)

def _add_offset(self, other) -> "Period":
Expand All @@ -1846,6 +1846,7 @@ cdef class _Period(PeriodMixin):
ordinal = self.ordinal + other.n
return Period(ordinal=ordinal, freq=self.freq)

@cython.overflowcheck(True)
def __add__(self, other):
if not is_period_object(self):
# cython semantics; this is analogous to a call to __radd__
Expand Down
26 changes: 19 additions & 7 deletions pandas/_libs/tslibs/timedeltas.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -1784,7 +1784,7 @@ class Timedelta(_Timedelta):
)

# GH43764, convert any input to nanoseconds first and then
# create the timestamp. This ensures that any potential
# create the timedelta. This ensures that any potential
# nanosecond contributions from kwargs parsed as floats
# are taken into consideration.
seconds = int((
Expand All @@ -1797,12 +1797,24 @@ class Timedelta(_Timedelta):
) * 1_000_000_000
)

value = np.timedelta64(
int(kwargs.get("nanoseconds", 0))
+ int(kwargs.get("microseconds", 0) * 1_000)
+ int(kwargs.get("milliseconds", 0) * 1_000_000)
+ seconds
)
ns = kwargs.get("nanoseconds", 0)
us = kwargs.get("microseconds", 0)
ms = kwargs.get("milliseconds", 0)
try:
value = np.timedelta64(
int(ns)
+ int(us * 1_000)
+ int(ms * 1_000_000)
+ seconds
)
except OverflowError as err:
# GH#55503
msg = (
f"seconds={seconds}, milliseconds={ms}, "
f"microseconds={us}, nanoseconds={ns}"
)
raise OutOfBoundsTimedelta(msg) from err

if unit in {"Y", "y", "M"}:
raise ValueError(
"Units 'M', 'Y', and 'y' are no longer supported, as they do not "
Expand Down
20 changes: 20 additions & 0 deletions pandas/tests/scalar/period/test_period.py
Original file line number Diff line number Diff line change
Expand Up @@ -1182,6 +1182,26 @@ def test_comparison_numpy_zerodim_arr(self, zerodim_arr, expected):


class TestArithmetic:
def test_add_overflow_raises(self):
# GH#55503
per = Timestamp.max.to_period("ns")

msg = "|".join(
[
"Python int too large to convert to C long",
# windows, 32bit linux builds
"int too big to convert",
]
)
with pytest.raises(OverflowError, match=msg):
per + 1

msg = "value too large"
with pytest.raises(OverflowError, match=msg):
per + Timedelta(1)
with pytest.raises(OverflowError, match=msg):
per + offsets.Nano(1)

@pytest.mark.parametrize("unit", ["ns", "us", "ms", "s", "m"])
def test_add_sub_td64_nat(self, unit):
# GH#47196
Expand Down
10 changes: 10 additions & 0 deletions pandas/tests/scalar/timedelta/test_constructors.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@
)


def test_construct_from_kwargs_overflow():
# GH#55503
msg = "seconds=86400000000000000000, milliseconds=0, microseconds=0, nanoseconds=0"
with pytest.raises(OutOfBoundsTimedelta, match=msg):
Timedelta(days=10**6)
msg = "seconds=60000000000000000000, milliseconds=0, microseconds=0, nanoseconds=0"
with pytest.raises(OutOfBoundsTimedelta, match=msg):
Timedelta(minutes=10**9)


def test_construct_with_weeks_unit_overflow():
# GH#47268 don't silently wrap around
with pytest.raises(OutOfBoundsTimedelta, match="without overflow"):
Expand Down
9 changes: 2 additions & 7 deletions pandas/tests/scalar/timestamp/test_arithmetic.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,12 @@ def test_overflow_offset_raises(self):

stamp = Timestamp("2017-01-13 00:00:00").as_unit("ns")
offset_overflow = 20169940 * offsets.Day(1)
msg = (
"the add operation between "
r"\<-?\d+ \* Days\> and \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} "
"will overflow"
)
lmsg2 = r"Cannot cast -?20169940 days \+?00:00:00 to unit='ns' without overflow"

with pytest.raises(OutOfBoundsTimedelta, match=lmsg2):
stamp + offset_overflow

with pytest.raises(OverflowError, match=msg):
with pytest.raises(OutOfBoundsTimedelta, match=lmsg2):
offset_overflow + stamp

with pytest.raises(OutOfBoundsTimedelta, match=lmsg2):
Expand All @@ -68,7 +63,7 @@ def test_overflow_offset_raises(self):
with pytest.raises(OutOfBoundsTimedelta, match=lmsg3):
stamp + offset_overflow

with pytest.raises(OverflowError, match=msg):
with pytest.raises(OutOfBoundsTimedelta, match=lmsg3):
offset_overflow + stamp

with pytest.raises(OutOfBoundsTimedelta, match=lmsg3):
Expand Down
9 changes: 9 additions & 0 deletions pandas/tests/tseries/offsets/test_ticks.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import pytest

from pandas._libs.tslibs.offsets import delta_to_tick
from pandas.errors import OutOfBoundsTimedelta

from pandas import (
Timedelta,
Expand Down Expand Up @@ -237,6 +238,14 @@ def test_tick_addition(kls, expected):
assert result == expected


def test_tick_delta_overflow():
# GH#55503 raise OutOfBoundsTimedelta, not OverflowError
tick = offsets.Day(10**9)
msg = "Cannot cast 1000000000 days 00:00:00 to unit='ns' without overflow"
with pytest.raises(OutOfBoundsTimedelta, match=msg):
tick.delta


@pytest.mark.parametrize("cls", tick_classes)
def test_tick_division(cls):
off = cls(10)
Expand Down
Loading