From 081c3313231a53aff104aa449d925330ef591d25 Mon Sep 17 00:00:00 2001 From: Brock Date: Thu, 12 Oct 2023 16:46:38 -0700 Subject: [PATCH 1/3] BUG: tslibs uncaught overflows --- doc/source/whatsnew/v2.2.0.rst | 8 +++--- pandas/_libs/tslibs/offsets.pyx | 7 +++++- pandas/_libs/tslibs/period.pyx | 7 +++--- pandas/_libs/tslibs/timedeltas.pyx | 25 +++++++++++++------ pandas/tests/scalar/period/test_period.py | 13 ++++++++++ .../scalar/timedelta/test_constructors.py | 9 +++++++ .../tests/scalar/timestamp/test_arithmetic.py | 9 ++----- pandas/tests/tseries/offsets/test_ticks.py | 9 +++++++ 8 files changed, 66 insertions(+), 21 deletions(-) diff --git a/doc/source/whatsnew/v2.2.0.rst b/doc/source/whatsnew/v2.2.0.rst index eec82ae26afcc..7bc2ce1d90eeb 100644 --- a/doc/source/whatsnew/v2.2.0.rst +++ b/doc/source/whatsnew/v2.2.0.rst @@ -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 addition or subtraction of very large :class:`Tick` objects with :class:`Timestamp` or :class:`Timedelta` objects raising ``OverflowError`` instead of ``OutOfBoundsTimedelta`` (:issue:`??`) +- Bug in :meth:`Tick.delta` with very large ticks raising ``OverflowError`` instead of ``OutOfBoundsTimedelta`` (:issue:`??`) + Timedelta ^^^^^^^^^ - 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`) -- +- Bug in :class:`Timedelta` construction raising ``OverflowError`` instead of ``OutOfBoundsTimedelta`` (:issue:`??`) Timezones ^^^^^^^^^ @@ -352,7 +354,7 @@ I/O Period ^^^^^^ -- +- Bug in :class:`Period` addition silently wrapping around instead of raising ``OverflowError`` (:issue:`??`) - Plotting diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 6a6f30de8dade..a0f47f8d47cd4 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -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: + # 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: diff --git a/pandas/_libs/tslibs/period.pyx b/pandas/_libs/tslibs/period.pyx index cacfe43b236d8..d305f27dd1090 100644 --- a/pandas/_libs/tslibs/period.pyx +++ b/pandas/_libs/tslibs/period.pyx @@ -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 " @@ -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": @@ -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__ diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index e5d81bd5928b9..5d547b724ff38 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -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(( @@ -1797,12 +1797,23 @@ 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: + 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 " diff --git a/pandas/tests/scalar/period/test_period.py b/pandas/tests/scalar/period/test_period.py index 6c27881e44b56..53df32d220ee6 100644 --- a/pandas/tests/scalar/period/test_period.py +++ b/pandas/tests/scalar/period/test_period.py @@ -1182,6 +1182,19 @@ def test_comparison_numpy_zerodim_arr(self, zerodim_arr, expected): class TestArithmetic: + def test_add_overflow_raises(self): + per = Timestamp.max.to_period("ns") + + msg = "Python int too large to convert to C long" + 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 diff --git a/pandas/tests/scalar/timedelta/test_constructors.py b/pandas/tests/scalar/timedelta/test_constructors.py index 0d876fbb9bde8..4d445d0ef0de8 100644 --- a/pandas/tests/scalar/timedelta/test_constructors.py +++ b/pandas/tests/scalar/timedelta/test_constructors.py @@ -15,6 +15,15 @@ ) +def test_construct_from_kwargs_overflow(): + 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"): diff --git a/pandas/tests/scalar/timestamp/test_arithmetic.py b/pandas/tests/scalar/timestamp/test_arithmetic.py index f5c9c576abc24..9c24d364841d1 100644 --- a/pandas/tests/scalar/timestamp/test_arithmetic.py +++ b/pandas/tests/scalar/timestamp/test_arithmetic.py @@ -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): @@ -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): diff --git a/pandas/tests/tseries/offsets/test_ticks.py b/pandas/tests/tseries/offsets/test_ticks.py index 69953955ebbce..08bbbef8ba6d4 100644 --- a/pandas/tests/tseries/offsets/test_ticks.py +++ b/pandas/tests/tseries/offsets/test_ticks.py @@ -15,6 +15,7 @@ import pytest from pandas._libs.tslibs.offsets import delta_to_tick +from pandas.errors import OutOfBoundsTimedelta from pandas import ( Timedelta, @@ -237,6 +238,14 @@ def test_tick_addition(kls, expected): assert result == expected +def test_tick_delta_overflow(): + # 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) From 59980c105c57170ee39f6f471aae494b4c991da7 Mon Sep 17 00:00:00 2001 From: Brock Date: Thu, 12 Oct 2023 16:49:18 -0700 Subject: [PATCH 2/3] GH refs --- doc/source/whatsnew/v2.2.0.rst | 8 ++++---- pandas/_libs/tslibs/offsets.pyx | 2 +- pandas/_libs/tslibs/timedeltas.pyx | 1 + pandas/tests/scalar/period/test_period.py | 1 + pandas/tests/scalar/timedelta/test_constructors.py | 1 + pandas/tests/tseries/offsets/test_ticks.py | 2 +- 6 files changed, 9 insertions(+), 6 deletions(-) diff --git a/doc/source/whatsnew/v2.2.0.rst b/doc/source/whatsnew/v2.2.0.rst index 7bc2ce1d90eeb..c869dfa808f57 100644 --- a/doc/source/whatsnew/v2.2.0.rst +++ b/doc/source/whatsnew/v2.2.0.rst @@ -290,14 +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 addition or subtraction of very large :class:`Tick` objects with :class:`Timestamp` or :class:`Timedelta` objects raising ``OverflowError`` instead of ``OutOfBoundsTimedelta`` (:issue:`??`) -- Bug in :meth:`Tick.delta` with very large ticks raising ``OverflowError`` instead of ``OutOfBoundsTimedelta`` (:issue:`??`) +- 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`) -- Bug in :class:`Timedelta` construction raising ``OverflowError`` instead of ``OutOfBoundsTimedelta`` (:issue:`??`) Timezones ^^^^^^^^^ @@ -354,7 +354,7 @@ I/O Period ^^^^^^ -- Bug in :class:`Period` addition silently wrapping around instead of raising ``OverflowError`` (:issue:`??`) +- Bug in :class:`Period` addition silently wrapping around instead of raising ``OverflowError`` (:issue:`55503`) - Plotting diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index a0f47f8d47cd4..6c5cdde20da5f 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -964,7 +964,7 @@ cdef class Tick(SingleConstructorOffset): try: return self.n * Timedelta(self._nanos_inc) except OverflowError as err: - # as_unit will raise a more useful OutOfBoundsTimedelta + # GH#55503 as_unit will raise a more useful OutOfBoundsTimedelta Timedelta(self).as_unit("ns") raise AssertionError("This should not be reached.") diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 5d547b724ff38..a573d9a8ed0c0 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1808,6 +1808,7 @@ class Timedelta(_Timedelta): + seconds ) except OverflowError as err: + # GH#55503 msg = ( f"seconds={seconds}, milliseconds={ms}, " f"microseconds={us}, nanoseconds={ns}" diff --git a/pandas/tests/scalar/period/test_period.py b/pandas/tests/scalar/period/test_period.py index 53df32d220ee6..0bfc08d719be0 100644 --- a/pandas/tests/scalar/period/test_period.py +++ b/pandas/tests/scalar/period/test_period.py @@ -1183,6 +1183,7 @@ 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 = "Python int too large to convert to C long" diff --git a/pandas/tests/scalar/timedelta/test_constructors.py b/pandas/tests/scalar/timedelta/test_constructors.py index 4d445d0ef0de8..858ab29d79c5e 100644 --- a/pandas/tests/scalar/timedelta/test_constructors.py +++ b/pandas/tests/scalar/timedelta/test_constructors.py @@ -16,6 +16,7 @@ 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) diff --git a/pandas/tests/tseries/offsets/test_ticks.py b/pandas/tests/tseries/offsets/test_ticks.py index 08bbbef8ba6d4..abf187ace7cb3 100644 --- a/pandas/tests/tseries/offsets/test_ticks.py +++ b/pandas/tests/tseries/offsets/test_ticks.py @@ -239,7 +239,7 @@ def test_tick_addition(kls, expected): def test_tick_delta_overflow(): - # raise OutOfBoundsTimedelta, not OverflowError + # 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): From e70d5826af285eb9230e7fbdd2823b1a1ab47e2b Mon Sep 17 00:00:00 2001 From: Brock Date: Thu, 12 Oct 2023 19:23:15 -0700 Subject: [PATCH 3/3] windows/32bit builds --- pandas/tests/scalar/period/test_period.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pandas/tests/scalar/period/test_period.py b/pandas/tests/scalar/period/test_period.py index 0bfc08d719be0..dc2938ec345f3 100644 --- a/pandas/tests/scalar/period/test_period.py +++ b/pandas/tests/scalar/period/test_period.py @@ -1186,7 +1186,13 @@ def test_add_overflow_raises(self): # GH#55503 per = Timestamp.max.to_period("ns") - msg = "Python int too large to convert to C long" + 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