From 7adfe00762e4fcc1563c1ec117b334538e9971c2 Mon Sep 17 00:00:00 2001 From: Brock Date: Fri, 23 Sep 2022 10:30:54 -0700 Subject: [PATCH 1/2] API: Change Timestamp/Timedelta arithmetic to match numpy --- pandas/_libs/tslibs/timedeltas.pyx | 19 ++-- pandas/_libs/tslibs/timestamps.pyx | 53 +++++------ .../tests/scalar/timedelta/test_timedelta.py | 31 ++++--- .../tests/scalar/timestamp/test_timestamp.py | 87 ++++++++++++------- 4 files changed, 98 insertions(+), 92 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index c9e997ffb405c..bf22967f615c4 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -787,19 +787,12 @@ def _binary_op_method_timedeltalike(op, name): # e.g. if original other was timedelta64('NaT') return NaT - # We allow silent casting to the lower resolution if and only - # if it is lossless. - try: - if self._reso < other._reso: - other = (<_Timedelta>other)._as_reso(self._reso, round_ok=False) - elif self._reso > other._reso: - self = (<_Timedelta>self)._as_reso(other._reso, round_ok=False) - except ValueError as err: - raise ValueError( - "Timedelta addition/subtraction with mismatched resolutions is not " - "allowed when casting to the lower resolution would require " - "lossy rounding." - ) from err + # Matching numpy, we cast to the higher resolution. Unlike numpy, + # we raise instead of silently overflowing during this casting. + if self._reso < other._reso: + self = (<_Timedelta>self)._as_reso(other._reso, round_ok=True) + elif self._reso > other._reso: + other = (<_Timedelta>other)._as_reso(self._reso, round_ok=True) res = op(self.value, other.value) if res == NPY_NAT: diff --git a/pandas/_libs/tslibs/timestamps.pyx b/pandas/_libs/tslibs/timestamps.pyx index 07c6e32028942..2c6969a72bf74 100644 --- a/pandas/_libs/tslibs/timestamps.pyx +++ b/pandas/_libs/tslibs/timestamps.pyx @@ -440,23 +440,23 @@ cdef class _Timestamp(ABCTimestamp): # corresponding DateOffsets? # TODO: no tests get here other = ensure_td64ns(other) + if other_reso > NPY_DATETIMEUNIT.NPY_FR_ns: + # TODO: no tests + other = ensure_td64ns(other) + if other_reso > self._reso: + # Following numpy, we cast to the higher resolution + # test_sub_timedelta64_mismatched_reso + self = (<_Timestamp>self)._as_reso(other_reso) + if isinstance(other, _Timedelta): # TODO: share this with __sub__, Timedelta.__add__ - # We allow silent casting to the lower resolution if and only - # if it is lossless. See also Timestamp.__sub__ - # and Timedelta.__add__ - try: - if self._reso < other._reso: - other = (<_Timedelta>other)._as_reso(self._reso, round_ok=False) - elif self._reso > other._reso: - self = (<_Timestamp>self)._as_reso(other._reso, round_ok=False) - except ValueError as err: - raise ValueError( - "Timestamp addition with mismatched resolutions is not " - "allowed when casting to the lower resolution would require " - "lossy rounding." - ) from err + # Matching numpy, we cast to the higher resolution. Unlike numpy, + # we raise instead of silently overflowing during this casting. + if self._reso < other._reso: + self = (<_Timestamp>self)._as_reso(other._reso, round_ok=True) + elif self._reso > other._reso: + other = (<_Timedelta>other)._as_reso(self._reso, round_ok=True) try: nanos = delta_to_nanoseconds( @@ -464,12 +464,6 @@ cdef class _Timestamp(ABCTimestamp): ) except OutOfBoundsTimedelta: raise - except ValueError as err: - raise ValueError( - "Addition between Timestamp and Timedelta with mismatched " - "resolutions is not allowed when casting to the lower " - "resolution would require lossy rounding." - ) from err try: new_value = self.value + nanos @@ -556,19 +550,12 @@ cdef class _Timestamp(ABCTimestamp): "Cannot subtract tz-naive and tz-aware datetime-like objects." ) - # We allow silent casting to the lower resolution if and only - # if it is lossless. - try: - if self._reso < other._reso: - other = (<_Timestamp>other)._as_reso(self._reso, round_ok=False) - elif self._reso > other._reso: - self = (<_Timestamp>self)._as_reso(other._reso, round_ok=False) - except ValueError as err: - raise ValueError( - "Timestamp subtraction with mismatched resolutions is not " - "allowed when casting to the lower resolution would require " - "lossy rounding." - ) from err + # Matching numpy, we cast to the higher resolution. Unlike numpy, + # we raise instead of silently overflowing during this casting. + if self._reso < other._reso: + self = (<_Timestamp>self)._as_reso(other._reso, round_ok=False) + elif self._reso > other._reso: + other = (<_Timestamp>other)._as_reso(self._reso, round_ok=False) # scalar Timestamp/datetime - Timestamp/datetime -> yields a # Timedelta diff --git a/pandas/tests/scalar/timedelta/test_timedelta.py b/pandas/tests/scalar/timedelta/test_timedelta.py index 21f32cf2d2d1e..8d3738d40601b 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -237,38 +237,37 @@ def test_floordiv_numeric(self, td): assert res._reso == td._reso def test_addsub_mismatched_reso(self, td): - other = Timedelta(days=1) # can losslessly convert to other resos + # need to cast to since td is out of bounds for ns, so + # so we would raise OverflowError without casting + other = Timedelta(days=1)._as_unit("us") + # td is out of bounds for ns result = td + other - assert result._reso == td._reso + assert result._reso == other._reso assert result.days == td.days + 1 result = other + td - assert result._reso == td._reso + assert result._reso == other._reso assert result.days == td.days + 1 result = td - other - assert result._reso == td._reso + assert result._reso == other._reso assert result.days == td.days - 1 result = other - td - assert result._reso == td._reso + assert result._reso == other._reso assert result.days == 1 - td.days - other2 = Timedelta(500) # can't cast losslessly - - msg = ( - "Timedelta addition/subtraction with mismatched resolutions is " - "not allowed when casting to the lower resolution would require " - "lossy rounding" - ) - with pytest.raises(ValueError, match=msg): + other2 = Timedelta(500) + # TODO: should be OutOfBoundsTimedelta + msg = "value too large" + with pytest.raises(OverflowError, match=msg): td + other2 - with pytest.raises(ValueError, match=msg): + with pytest.raises(OverflowError, match=msg): other2 + td - with pytest.raises(ValueError, match=msg): + with pytest.raises(OverflowError, match=msg): td - other2 - with pytest.raises(ValueError, match=msg): + with pytest.raises(OverflowError, match=msg): other2 - td def test_min(self, td): diff --git a/pandas/tests/scalar/timestamp/test_timestamp.py b/pandas/tests/scalar/timestamp/test_timestamp.py index dc3ddc7361afd..48c016434a97d 100644 --- a/pandas/tests/scalar/timestamp/test_timestamp.py +++ b/pandas/tests/scalar/timestamp/test_timestamp.py @@ -18,7 +18,10 @@ utc, ) -from pandas._libs.tslibs.dtypes import NpyDatetimeUnit +from pandas._libs.tslibs.dtypes import ( + NpyDatetimeUnit, + npy_unit_to_abbrev, +) from pandas._libs.tslibs.timezones import ( dateutil_gettz as gettz, get_timezone, @@ -884,22 +887,29 @@ def test_to_period(self, dt64, ts): ) def test_addsub_timedeltalike_non_nano(self, dt64, ts, td): + if isinstance(td, Timedelta): + # td._reso is ns + exp_reso = td._reso + else: + # effective td._reso is s + exp_reso = ts._reso + result = ts - td expected = Timestamp(dt64) - td assert isinstance(result, Timestamp) - assert result._reso == ts._reso + assert result._reso == exp_reso assert result == expected result = ts + td expected = Timestamp(dt64) + td assert isinstance(result, Timestamp) - assert result._reso == ts._reso + assert result._reso == exp_reso assert result == expected result = td + ts expected = td + Timestamp(dt64) assert isinstance(result, Timestamp) - assert result._reso == ts._reso + assert result._reso == exp_reso assert result == expected def test_addsub_offset(self, ts_tz): @@ -944,27 +954,35 @@ def test_sub_datetimelike_mismatched_reso(self, ts_tz): result = ts - other assert isinstance(result, Timedelta) assert result.value == 0 - assert result._reso == min(ts._reso, other._reso) + assert result._reso == max(ts._reso, other._reso) result = other - ts assert isinstance(result, Timedelta) assert result.value == 0 - assert result._reso == min(ts._reso, other._reso) + assert result._reso == max(ts._reso, other._reso) - msg = "Timestamp subtraction with mismatched resolutions" if ts._reso < other._reso: # Case where rounding is lossy other2 = other + Timedelta._from_value_and_reso(1, other._reso) - with pytest.raises(ValueError, match=msg): - ts - other2 - with pytest.raises(ValueError, match=msg): - other2 - ts + exp = ts._as_unit(npy_unit_to_abbrev(other._reso)) - other2 + + res = ts - other2 + assert res == exp + assert res._reso == max(ts._reso, other._reso) + + res = other2 - ts + assert res == -exp + assert res._reso == max(ts._reso, other._reso) else: ts2 = ts + Timedelta._from_value_and_reso(1, ts._reso) - with pytest.raises(ValueError, match=msg): - ts2 - other - with pytest.raises(ValueError, match=msg): - other - ts2 + exp = ts2 - other._as_unit(npy_unit_to_abbrev(ts2._reso)) + + res = ts2 - other + assert res == exp + assert res._reso == max(ts._reso, other._reso) + res = other - ts2 + assert res == -exp + assert res._reso == max(ts._reso, other._reso) def test_sub_timedeltalike_mismatched_reso(self, ts_tz): # case with non-lossy rounding @@ -984,32 +1002,41 @@ def test_sub_timedeltalike_mismatched_reso(self, ts_tz): result = ts + other assert isinstance(result, Timestamp) assert result == ts - assert result._reso == min(ts._reso, other._reso) + assert result._reso == max(ts._reso, other._reso) result = other + ts assert isinstance(result, Timestamp) assert result == ts - assert result._reso == min(ts._reso, other._reso) + assert result._reso == max(ts._reso, other._reso) - msg = "Timestamp addition with mismatched resolutions" if ts._reso < other._reso: # Case where rounding is lossy other2 = other + Timedelta._from_value_and_reso(1, other._reso) - with pytest.raises(ValueError, match=msg): - ts + other2 - with pytest.raises(ValueError, match=msg): - other2 + ts + exp = ts._as_unit(npy_unit_to_abbrev(other._reso)) + other2 + res = ts + other2 + assert res == exp + assert res._reso == max(ts._reso, other._reso) + res = other2 + ts + assert res == exp + assert res._reso == max(ts._reso, other._reso) else: ts2 = ts + Timedelta._from_value_and_reso(1, ts._reso) - with pytest.raises(ValueError, match=msg): - ts2 + other - with pytest.raises(ValueError, match=msg): - other + ts2 + exp = ts2 + other._as_unit(npy_unit_to_abbrev(ts2._reso)) - msg = "Addition between Timestamp and Timedelta with mismatched resolutions" - with pytest.raises(ValueError, match=msg): - # With a mismatched td64 as opposed to Timedelta - ts + np.timedelta64(1, "ns") + res = ts2 + other + assert res == exp + assert res._reso == max(ts._reso, other._reso) + res = other + ts2 + assert res == exp + assert res._reso == max(ts._reso, other._reso) + + def test_sub_timedelta64_mismatched_reso(self, ts_tz): + ts = ts_tz + + res = ts + np.timedelta64(1, "ns") + exp = ts._as_unit("ns") + np.timedelta64(1, "ns") + assert exp == res + assert exp._reso == NpyDatetimeUnit.NPY_FR_ns.value def test_min(self, ts): assert ts.min <= ts From d357360a86ded820e1077c6bdb177b7a875ccd62 Mon Sep 17 00:00:00 2001 From: Brock Date: Fri, 23 Sep 2022 12:18:38 -0700 Subject: [PATCH 2/2] fix interval test --- pandas/_libs/tslibs/timestamps.pyx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pandas/_libs/tslibs/timestamps.pyx b/pandas/_libs/tslibs/timestamps.pyx index 2c6969a72bf74..18bcb5a97bee0 100644 --- a/pandas/_libs/tslibs/timestamps.pyx +++ b/pandas/_libs/tslibs/timestamps.pyx @@ -433,6 +433,7 @@ cdef class _Timestamp(ABCTimestamp): # TODO: deprecate allowing this? We only get here # with test_timedelta_add_timestamp_interval other = np.timedelta64(other.view("i8"), "ns") + other_reso = NPY_DATETIMEUNIT.NPY_FR_ns elif ( other_reso == NPY_DATETIMEUNIT.NPY_FR_Y or other_reso == NPY_DATETIMEUNIT.NPY_FR_M ): @@ -440,6 +441,8 @@ cdef class _Timestamp(ABCTimestamp): # corresponding DateOffsets? # TODO: no tests get here other = ensure_td64ns(other) + other_reso = NPY_DATETIMEUNIT.NPY_FR_ns + if other_reso > NPY_DATETIMEUNIT.NPY_FR_ns: # TODO: no tests other = ensure_td64ns(other)