From 15eb9cb54a07faab88109137758cfdd45a063b0d Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Fri, 6 Mar 2020 11:10:26 -0600 Subject: [PATCH 1/5] Better error message for OOB result Closes #31774 --- doc/source/whatsnew/v1.0.2.rst | 1 + pandas/_libs/tslibs/c_timestamp.pyx | 8 +++++++- pandas/tests/scalar/timestamp/test_arithmetic.py | 9 +++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.0.2.rst b/doc/source/whatsnew/v1.0.2.rst index 808e6ae709ce9..27e84e11d0342 100644 --- a/doc/source/whatsnew/v1.0.2.rst +++ b/doc/source/whatsnew/v1.0.2.rst @@ -64,6 +64,7 @@ Bug fixes - Bug in :meth:`DataFrame.reindex` and :meth:`Series.reindex` when reindexing with a tz-aware index (:issue:`26683`) - Bug where :func:`to_datetime` would raise when passed ``pd.NA`` (:issue:`32213`) +- Improved error message when subtracting two :class:`Timestamp` that result in an invalide :class:`Timedelta` (:issue:`31774`) **Categorical** diff --git a/pandas/_libs/tslibs/c_timestamp.pyx b/pandas/_libs/tslibs/c_timestamp.pyx index 2c72cec18f096..8f6514439decd 100644 --- a/pandas/_libs/tslibs/c_timestamp.pyx +++ b/pandas/_libs/tslibs/c_timestamp.pyx @@ -299,9 +299,15 @@ cdef class _Timestamp(datetime): # scalar Timestamp/datetime - Timestamp/datetime -> yields a # Timedelta from pandas._libs.tslibs.timedeltas import Timedelta + from pandas._libs.tslibs.timestamps import Timestamp try: return Timedelta(self.value - other.value) - except (OverflowError, OutOfBoundsDatetime): + except (OverflowError, OutOfBoundsDatetime) as e: + if isinstance(other, Timestamp): + raise OverflowError( + "Result is too large for pandas.Timestamp. Convert inputs " + "to datetime.datetime before subtracting." + ) from e pass elif is_datetime64_object(self): # GH#28286 cython semantics for __rsub__, `other` is actually diff --git a/pandas/tests/scalar/timestamp/test_arithmetic.py b/pandas/tests/scalar/timestamp/test_arithmetic.py index 1cab007c20a0e..039ec713f74f0 100644 --- a/pandas/tests/scalar/timestamp/test_arithmetic.py +++ b/pandas/tests/scalar/timestamp/test_arithmetic.py @@ -60,6 +60,15 @@ def test_overflow_offset_raises(self): with pytest.raises(OverflowError, match=msg): stamp - offset_overflow + def test_overflow_timestamp_raises(self): + # https://github.com/pandas-dev/pandas/issues/31774 + msg = "Result is too large" + a = Timestamp("2101-01-01 00:00:00") + b = Timestamp("1688-01-01 00:00:00") + + with pytest.raises(OverflowError, match=msg): + a - b + def test_delta_preserve_nanos(self): val = Timestamp(1337299200000000123) result = val + timedelta(1) From 815050fcad992b7e82677f463e1eaa1380d8f8e9 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Mon, 9 Mar 2020 15:43:33 -0500 Subject: [PATCH 2/5] wip --- pandas/_libs/tslibs/c_timestamp.pyx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pandas/_libs/tslibs/c_timestamp.pyx b/pandas/_libs/tslibs/c_timestamp.pyx index 8f6514439decd..cefb2d1cb9ee3 100644 --- a/pandas/_libs/tslibs/c_timestamp.pyx +++ b/pandas/_libs/tslibs/c_timestamp.pyx @@ -299,15 +299,15 @@ cdef class _Timestamp(datetime): # scalar Timestamp/datetime - Timestamp/datetime -> yields a # Timedelta from pandas._libs.tslibs.timedeltas import Timedelta - from pandas._libs.tslibs.timestamps import Timestamp try: return Timedelta(self.value - other.value) - except (OverflowError, OutOfBoundsDatetime) as e: - if isinstance(other, Timestamp): - raise OverflowError( - "Result is too large for pandas.Timestamp. Convert inputs " - "to datetime.datetime before subtracting." - ) from e + except (OverflowError, OutOfBoundsDatetime) as err: + if isinstance(other, _Timestamp): + raise OutOfBoundsDatetime( + "Result is too large for pandas.Timedelta. Convert inputs " + "to datetime.datetime with 'Timestamp.to_pydatetime()' " + "before subtracting." + ) from err pass elif is_datetime64_object(self): # GH#28286 cython semantics for __rsub__, `other` is actually From 6817c4581e52e504b230233b0bd5371cd580955f Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Mon, 9 Mar 2020 15:54:27 -0500 Subject: [PATCH 3/5] fixups --- pandas/_libs/tslibs/c_timestamp.pyx | 19 ++++++++++++------- .../tests/scalar/timestamp/test_arithmetic.py | 7 ++++++- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/pandas/_libs/tslibs/c_timestamp.pyx b/pandas/_libs/tslibs/c_timestamp.pyx index cefb2d1cb9ee3..a9964e18bb929 100644 --- a/pandas/_libs/tslibs/c_timestamp.pyx +++ b/pandas/_libs/tslibs/c_timestamp.pyx @@ -287,9 +287,13 @@ cdef class _Timestamp(datetime): if (PyDateTime_Check(self) and (PyDateTime_Check(other) or is_datetime64_object(other))): if isinstance(self, _Timestamp): - other = type(self)(other) + both_timestamps = isinstance(other, _Timestamp) + if not both_timestamps: + other = type(self)(other) else: - self = type(other)(self) + both_timestamps = isinstance(self, _Timestamp) + if not both_timestamps: + self = type(other)(self) # validate tz's if not tz_compare(self.tzinfo, other.tzinfo): @@ -303,11 +307,12 @@ cdef class _Timestamp(datetime): return Timedelta(self.value - other.value) except (OverflowError, OutOfBoundsDatetime) as err: if isinstance(other, _Timestamp): - raise OutOfBoundsDatetime( - "Result is too large for pandas.Timedelta. Convert inputs " - "to datetime.datetime with 'Timestamp.to_pydatetime()' " - "before subtracting." - ) from err + if both_timestamps: + raise OutOfBoundsDatetime( + "Result is too large for pandas.Timedelta. Convert inputs " + "to datetime.datetime with 'Timestamp.to_pydatetime()' " + "before subtracting." + ) from err pass elif is_datetime64_object(self): # GH#28286 cython semantics for __rsub__, `other` is actually diff --git a/pandas/tests/scalar/timestamp/test_arithmetic.py b/pandas/tests/scalar/timestamp/test_arithmetic.py index 039ec713f74f0..ccd7bf721430a 100644 --- a/pandas/tests/scalar/timestamp/test_arithmetic.py +++ b/pandas/tests/scalar/timestamp/test_arithmetic.py @@ -3,6 +3,8 @@ import numpy as np import pytest +from pandas.errors import OutOfBoundsDatetime + from pandas import Timedelta, Timestamp from pandas.tseries import offsets @@ -66,9 +68,12 @@ def test_overflow_timestamp_raises(self): a = Timestamp("2101-01-01 00:00:00") b = Timestamp("1688-01-01 00:00:00") - with pytest.raises(OverflowError, match=msg): + with pytest.raises(OutOfBoundsDatetime, match=msg): a - b + # but we're OK for timestamp and datetime.datetime + assert (a - b.to_pydatetime()) == (a.to_pydatetime() - b) + def test_delta_preserve_nanos(self): val = Timestamp(1337299200000000123) result = val + timedelta(1) From d4aa8e81808e8e4c10346f7cab2ddff28ca03336 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Mon, 9 Mar 2020 16:38:00 -0500 Subject: [PATCH 4/5] note --- doc/source/whatsnew/v1.0.2.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.0.2.rst b/doc/source/whatsnew/v1.0.2.rst index 715140291fbfc..8a8b77fbbb426 100644 --- a/doc/source/whatsnew/v1.0.2.rst +++ b/doc/source/whatsnew/v1.0.2.rst @@ -65,7 +65,7 @@ Bug fixes - Bug in :meth:`DataFrame.reindex` and :meth:`Series.reindex` when reindexing with a tz-aware index (:issue:`26683`) - Bug where :func:`to_datetime` would raise when passed ``pd.NA`` (:issue:`32213`) -- Improved error message when subtracting two :class:`Timestamp` that result in an invalide :class:`Timedelta` (:issue:`31774`) +- Improved error message when subtracting two :class:`Timestamp` that result in an out-of-bounds :class:`Timedelta` (:issue:`31774`) **Categorical** From b748784615f8de1184fd4944d54d415da081aaed Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Tue, 10 Mar 2020 08:02:33 -0500 Subject: [PATCH 5/5] fixup --- pandas/_libs/tslibs/c_timestamp.pyx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pandas/_libs/tslibs/c_timestamp.pyx b/pandas/_libs/tslibs/c_timestamp.pyx index a9964e18bb929..3c30460a74ece 100644 --- a/pandas/_libs/tslibs/c_timestamp.pyx +++ b/pandas/_libs/tslibs/c_timestamp.pyx @@ -286,14 +286,14 @@ cdef class _Timestamp(datetime): # coerce if necessary if we are a Timestamp-like if (PyDateTime_Check(self) and (PyDateTime_Check(other) or is_datetime64_object(other))): + # both_timestamps is to determine whether Timedelta(self - other) + # should raise the OOB error, or fall back returning a timedelta. + both_timestamps = (isinstance(other, _Timestamp) and + isinstance(self, _Timestamp)) if isinstance(self, _Timestamp): - both_timestamps = isinstance(other, _Timestamp) - if not both_timestamps: - other = type(self)(other) + other = type(self)(other) else: - both_timestamps = isinstance(self, _Timestamp) - if not both_timestamps: - self = type(other)(self) + self = type(other)(self) # validate tz's if not tz_compare(self.tzinfo, other.tzinfo):