BUG: Allow addition of Timedelta to Timestamp interval #32107
Conversation
|
@dsaxton can you merge master to resolve conflicts (and fix failing tests 😄 ) |
|
@jbrockmendel You think this might be good to merge? |
pandas/_libs/interval.pyx
Outdated
| @@ -398,14 +401,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): | |||
There was a problem hiding this comment.
also need is_timedelta64_object (basically wherever you have PyDelta_Check)
| 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) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
the raising should also work in the other direction, i.e. Interval(Timestamp) + numeric should fail
There was a problem hiding this comment.
Updated with some more cases
| # https://github.com/pandas-dev/pandas/issues/32023 | ||
| interval = Interval( | ||
| Timestamp("2017-01-01 00:00:00"), Timestamp("2018-01-01 00:00:00") | ||
| ) |
There was a problem hiding this comment.
should we get a Timedelta interval here too? also some NaTs
There was a problem hiding this comment.
Is it possible to put NaT in an Interval?
There was a problem hiding this comment.
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'| @pytest.mark.parametrize( | ||
| "delta", [Timedelta(days=7), timedelta(7), np.timedelta64(7, "D")] | ||
| ) | ||
| def test_timedelta_add_timestamp_interval(delta): |
There was a problem hiding this comment.
this would be somewhat clearer as test_timestamp_interval_radd_timedelta (or just rolled into the previous test)
|
@jschendel anything else this needs? |
There was a problem hiding this comment.
lgtm, thanks @dsaxton! @jbrockmendel feel free to merge if you're happy with this.
|
|
||
| from pandas import Interval, Timedelta, Timestamp | ||
|
|
||
|
|
There was a problem hiding this comment.
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.
| # https://github.com/pandas-dev/pandas/issues/32023 | ||
| interval = Interval( | ||
| Timestamp("2017-01-01 00:00:00"), Timestamp("2018-01-01 00:00:00") | ||
| ) |
There was a problem hiding this comment.
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'
|
thanks @dsaxton |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff