Skip to content

BUG: Allow addition of Timedelta to Timestamp interval #32107

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 28 commits into from
Apr 25, 2020
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
95f2fbe
Add test
dsaxton Feb 19, 2020
7438110
Check for Timedelta
dsaxton Feb 19, 2020
4e2c07a
Update test
dsaxton Feb 19, 2020
5b30740
whatsnew
dsaxton Feb 19, 2020
d88aca0
Remove unreachable
dsaxton Feb 19, 2020
48e4794
Revert "Remove unreachable"
dsaxton Feb 19, 2020
4fdc04f
Merge branch 'master' into add-timedelta
dsaxton Feb 19, 2020
ceb54d1
Merge branch 'master' into add-timedelta
dsaxton Feb 20, 2020
fe10a03
Update tests
dsaxton Feb 21, 2020
b5f169f
Tests
dsaxton Feb 21, 2020
6cc226d
Merge remote-tracking branch 'upstream/master' into add-timedelta
dsaxton Feb 21, 2020
d052240
Merge remote-tracking branch 'upstream/master' into add-timedelta
dsaxton Feb 25, 2020
a46a0ba
Merge remote-tracking branch 'upstream/master' into add-timedelta
dsaxton Apr 2, 2020
784706f
Check some other types
dsaxton Apr 2, 2020
63c7825
Merge remote-tracking branch 'upstream/master' into add-timedelta
dsaxton Apr 5, 2020
51c3c66
Maybe fix something
dsaxton Apr 10, 2020
fa93176
Merge remote-tracking branch 'upstream/master' into add-timedelta
dsaxton Apr 10, 2020
9ff29a9
Remove np.timdelta64 test case for now
dsaxton Apr 11, 2020
ab97990
Lint
dsaxton Apr 11, 2020
4955456
Black
dsaxton Apr 11, 2020
e11023a
Merge remote-tracking branch 'upstream/master' into add-timedelta
dsaxton Apr 11, 2020
985f94e
timedelta64 -> np.timedelta64
dsaxton Apr 11, 2020
ec57620
Merge remote-tracking branch 'upstream/master' into add-timedelta
dsaxton Apr 11, 2020
0c3a0ed
__array_priority__
dsaxton Apr 11, 2020
746f542
Merge remote-tracking branch 'upstream/master' into add-timedelta
dsaxton Apr 14, 2020
3d2f913
Update
dsaxton Apr 15, 2020
6e49272
Merge remote-tracking branch 'upstream/master' into add-timedelta
dsaxton Apr 15, 2020
1bc6304
Merge remote-tracking branch 'upstream/master' into add-timedelta
dsaxton Apr 24, 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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.1.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,7 @@ Indexing
- Bug in :class:`Index` constructor where an unhelpful error message was raised for ``numpy`` scalars (:issue:`33017`)
- Bug in :meth:`DataFrame.lookup` incorrectly raising an ``AttributeError`` when ``frame.index`` or ``frame.columns`` is not unique; this will now raise a ``ValueError`` with a helpful error message (:issue:`33041`)
- Bug in :meth:`DataFrame.iloc.__setitem__` creating a new array instead of overwriting ``Categorical`` values in-place (:issue:`32831`)
- Bug in :class:`Interval` where a :class:`Timedelta` could not be added or subtracted from a :class:`Timestamp` interval (:issue:`32023`)
- Bug in :meth:`DataFrame.copy` _item_cache not invalidated after copy causes post-copy value updates to not be reflected (:issue:`31784`)
- Bug in `Series.__getitem__` with an integer key and a :class:`MultiIndex` with leading integer level failing to raise ``KeyError`` if the key is not present in the first level (:issue:`33355`)
- Bug in :meth:`DataFrame.iloc` when slicing a single column-:class:`DataFrame`` with ``ExtensionDtype`` (e.g. ``df.iloc[:, :1]``) returning an invalid result (:issue:`32957`)
Expand Down
13 changes: 9 additions & 4 deletions pandas/_libs/interval.pyx
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import numbers
from operator import le, lt

from cpython.datetime cimport PyDelta_Check, PyDateTime_IMPORT
PyDateTime_IMPORT

from cpython.object cimport (
Py_EQ,
Py_GE,
Expand All @@ -11,7 +14,6 @@ from cpython.object cimport (
PyObject_RichCompare,
)


import cython
from cython import Py_ssize_t

Expand Down Expand Up @@ -398,14 +400,17 @@ cdef class Interval(IntervalMixin):
return f'{start_symbol}{left}, {right}{end_symbol}'

def __add__(self, y):
if isinstance(y, numbers.Number):
if isinstance(y, numbers.Number) or PyDelta_Check(y):
Copy link
Member

Choose a reason for hiding this comment

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

also need is_timedelta64_object (basically wherever you have PyDelta_Check)

return Interval(self.left + y, self.right + y, closed=self.closed)
elif isinstance(y, Interval) and isinstance(self, numbers.Number):
elif (
isinstance(y, Interval)
and (isinstance(self, numbers.Number) or PyDelta_Check(self))
):
return Interval(y.left + self, y.right + self, closed=y.closed)
return NotImplemented

def __sub__(self, y):
if isinstance(y, numbers.Number):
if isinstance(y, numbers.Number) or PyDelta_Check(y):
return Interval(self.left - y, self.right - y, closed=self.closed)
return NotImplemented

Expand Down
52 changes: 52 additions & 0 deletions pandas/tests/scalar/interval/test_arithmetic.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
from datetime import timedelta

from numpy import timedelta64
import pytest

from pandas import Interval, Timedelta, Timestamp


Copy link
Member

Choose a reason for hiding this comment

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

There are some additional Interval related arithmetic tests that can be moved here from test_interval.py but doesn't need to be done in this PR.

@pytest.mark.parametrize("method", ["__add__", "__sub__"])
@pytest.mark.parametrize(
"delta", [Timedelta(days=7), timedelta(7), timedelta64(7, "D")]
)
def test_timestamp_interval_add_subtract_timedelta(method, delta):
# https://github.com/pandas-dev/pandas/issues/32023
interval = Interval(
Timestamp("2017-01-01 00:00:00"), Timestamp("2018-01-01 00:00:00")
)

result = getattr(interval, method)(delta)

left = getattr(interval.left, method)(delta)
right = getattr(interval.right, method)(delta)
expected = Interval(left, right)

assert result == expected


@pytest.mark.parametrize("delta", [Timedelta(days=7), timedelta(7)])
def test_timedelta_add_timestamp_interval(delta):
Copy link
Member

Choose a reason for hiding this comment

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

this would be somewhat clearer as test_timestamp_interval_radd_timedelta (or just rolled into the previous test)

# https://github.com/pandas-dev/pandas/issues/32023
interval = Interval(
Timestamp("2017-01-01 00:00:00"), Timestamp("2018-01-01 00:00:00")
)
Copy link
Member

Choose a reason for hiding this comment

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

should we get a Timedelta interval here too? also some NaTs

Copy link
Member Author

Choose a reason for hiding this comment

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

Is it possible to put NaT in an Interval?

Copy link
Member

Choose a reason for hiding this comment

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

Intervals can't contain NA values as endpoints. It doesn't look like this gracefully handles adding NaT to intervals with Timestamp/Timedelta endpoints, but this is also true for numeric intervals, so I'm fine fixing the general case of interval arithmetic with NA in a separate PR.

In [1]: import numpy as np; import pandas as pd; pd.__version__
Out[1]: '1.1.0.dev0+1299.g6e492722d'

In [2]: pd.Interval(0, 1) + np.nan
---------------------------------------------------------------------------
ValueError: left side of interval must be <= right side

In [3]: pd.Interval(pd.Timestamp("2020"), pd.Timestamp("2021")) + pd.NaT
---------------------------------------------------------------------------
TypeError: unsupported operand type(s) for +: 'pandas._libs.interval.Interval' and 'NaTType'


result = delta + interval

left = interval.left + delta
right = interval.right + delta
expected = Interval(left, right)

assert result == expected


@pytest.mark.parametrize("method", ["__add__", "__sub__"])
def test_numeric_interval_add_subtract_timedelta_raises(method):
# https://github.com/pandas-dev/pandas/issues/32023
interval = Interval(1, 2)
delta = Timedelta(days=7)
Copy link
Member

Choose a reason for hiding this comment

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

can you parametrize over delta like in the previous test? (id probably not parametrize over __add__ and __sub__, but your call)

some more intervals that might be worth checking

  • float dtype
  • float nan
  • datetime NaT
  • timedelta NaT

Copy link
Member

Choose a reason for hiding this comment

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

the raising should also work in the other direction, i.e. Interval(Timestamp) + numeric should fail

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated with some more cases


msg = "unsupported operand"
with pytest.raises(TypeError, match=msg):
getattr(interval, method)(delta)