Skip to content

BUG: TimedeltaArray+Period, PeriodArray-TimedeltaArray #33883

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 3 commits into from
May 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -640,6 +640,7 @@ Datetimelike
- :meth:`DataFrame.min`/:meth:`DataFrame.max` not returning consistent result with :meth:`Series.min`/:meth:`Series.max` when called on objects initialized with empty :func:`pd.to_datetime`
- Bug in :meth:`DatetimeIndex.intersection` and :meth:`TimedeltaIndex.intersection` with results not having the correct ``name`` attribute (:issue:`33904`)
- Bug in :meth:`DatetimeArray.__setitem__`, :meth:`TimedeltaArray.__setitem__`, :meth:`PeriodArray.__setitem__` incorrectly allowing values with ``int64`` dtype to be silently cast (:issue:`33717`)
- Bug in subtracting :class:`TimedeltaIndex` from :class:`Period` incorrectly raising ``TypeError`` in some cases where it should succeed and ``IncompatibleFrequency`` in some cases where it should raise ``TypeError`` (:issue:`33883`)

Timedelta
^^^^^^^^^
Expand Down
6 changes: 6 additions & 0 deletions pandas/core/arrays/datetimelike.py
Original file line number Diff line number Diff line change
Expand Up @@ -1187,6 +1187,10 @@ def _sub_period(self, other):
# Overridden by PeriodArray
raise TypeError(f"cannot subtract Period from a {type(self).__name__}")

def _add_period(self, other: Period):
# Overriden by TimedeltaArray
raise TypeError(f"cannot add Period to a {type(self).__name__}")

def _add_offset(self, offset):
raise AbstractMethodError(self)

Expand Down Expand Up @@ -1391,6 +1395,8 @@ def __add__(self, other):
result = self._add_offset(other)
elif isinstance(other, (datetime, np.datetime64)):
result = self._add_datetimelike_scalar(other)
elif isinstance(other, Period) and is_timedelta64_dtype(self.dtype):
result = self._add_period(other)
elif lib.is_integer(other):
# This check must come after the check for np.timedelta64
# as is_integer returns True for these
Expand Down
13 changes: 10 additions & 3 deletions pandas/core/arrays/period.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@
pandas_dtype,
)
from pandas.core.dtypes.dtypes import PeriodDtype
from pandas.core.dtypes.generic import ABCIndexClass, ABCPeriodIndex, ABCSeries
from pandas.core.dtypes.generic import (
ABCIndexClass,
ABCPeriodIndex,
ABCSeries,
ABCTimedeltaArray,
)
from pandas.core.dtypes.missing import isna, notna

import pandas.core.algorithms as algos
Expand Down Expand Up @@ -676,7 +681,9 @@ def _add_timedelta_arraylike(self, other):
"""
if not isinstance(self.freq, Tick):
# We cannot add timedelta-like to non-tick PeriodArray
raise raise_on_incompatible(self, other)
raise TypeError(
f"Cannot add or subtract timedelta64[ns] dtype from {self.dtype}"
)

if not np.all(isna(other)):
delta = self._check_timedeltalike_freq_compat(other)
Expand Down Expand Up @@ -753,7 +760,7 @@ def raise_on_incompatible(left, right):
Exception to be raised by the caller.
"""
# GH#24283 error message format depends on whether right is scalar
if isinstance(right, np.ndarray) or right is None:
if isinstance(right, (np.ndarray, ABCTimedeltaArray)) or right is None:
other_freq = None
elif isinstance(right, (ABCPeriodIndex, PeriodArray, Period, DateOffset)):
other_freq = right.freqstr
Expand Down
13 changes: 12 additions & 1 deletion pandas/core/arrays/timedeltas.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import numpy as np

from pandas._libs import lib, tslibs
from pandas._libs.tslibs import NaT, Timedelta, Timestamp, iNaT
from pandas._libs.tslibs import NaT, Period, Timedelta, Timestamp, iNaT
from pandas._libs.tslibs.fields import get_timedelta_field
from pandas._libs.tslibs.timedeltas import (
array_to_timedelta64,
Expand Down Expand Up @@ -404,6 +404,17 @@ def _add_offset(self, other):
f"cannot add the type {type(other).__name__} to a {type(self).__name__}"
)

def _add_period(self, other: Period):
"""
Add a Period object.
"""
# We will wrap in a PeriodArray and defer to the reversed operation
from .period import PeriodArray

i8vals = np.broadcast_to(other.ordinal, self.shape)
oth = PeriodArray(i8vals, freq=other.freq)
return oth + self

def _add_datetime_arraylike(self, other):
"""
Add DatetimeArray/Index or ndarray[datetime64] to TimedeltaArray.
Expand Down
52 changes: 47 additions & 5 deletions pandas/tests/arithmetic/test_period.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from pandas.errors import PerformanceWarning

import pandas as pd
from pandas import Period, PeriodIndex, Series, period_range
from pandas import Period, PeriodIndex, Series, TimedeltaIndex, Timestamp, period_range
import pandas._testing as tm
from pandas.core import ops
from pandas.core.arrays import TimedeltaArray
Expand Down Expand Up @@ -730,13 +730,13 @@ def test_pi_add_sub_td64_array_non_tick_raises(self):
tdi = pd.TimedeltaIndex(["-1 Day", "-1 Day", "-1 Day"])
tdarr = tdi.values

msg = r"Input has different freq=None from PeriodArray\(freq=Q-DEC\)"
with pytest.raises(IncompatibleFrequency, match=msg):
msg = r"Cannot add or subtract timedelta64\[ns\] dtype from period\[Q-DEC\]"
with pytest.raises(TypeError, match=msg):
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't IncompatibleFrequency inherit from ValueError? so this is technically an api change (ok though)

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, this is a change. in the whatsnew i called it a bugfix

rng + tdarr
with pytest.raises(IncompatibleFrequency, match=msg):
with pytest.raises(TypeError, match=msg):
tdarr + rng

with pytest.raises(IncompatibleFrequency, match=msg):
with pytest.raises(TypeError, match=msg):
rng - tdarr
msg = r"cannot subtract PeriodArray from timedelta64\[ns\]"
with pytest.raises(TypeError, match=msg):
Expand Down Expand Up @@ -773,6 +773,48 @@ def test_pi_add_sub_td64_array_tick(self):
with pytest.raises(TypeError, match=msg):
tdi - rng

@pytest.mark.parametrize("pi_freq", ["D", "W", "Q", "H"])
@pytest.mark.parametrize("tdi_freq", [None, "H"])
def test_parr_sub_td64array(self, box_with_array, tdi_freq, pi_freq):
box = box_with_array
xbox = box if box is not tm.to_array else pd.Index

tdi = TimedeltaIndex(["1 hours", "2 hours"], freq=tdi_freq)
dti = Timestamp("2018-03-07 17:16:40") + tdi
pi = dti.to_period(pi_freq)

# TODO: parametrize over box for pi?
td64obj = tm.box_expected(tdi, box)

if pi_freq == "H":
result = pi - td64obj
expected = (pi.to_timestamp("S") - tdi).to_period(pi_freq)
expected = tm.box_expected(expected, xbox)
tm.assert_equal(result, expected)

# Subtract from scalar
result = pi[0] - td64obj
expected = (pi[0].to_timestamp("S") - tdi).to_period(pi_freq)
expected = tm.box_expected(expected, box)
tm.assert_equal(result, expected)

elif pi_freq == "D":
# Tick, but non-compatible
msg = "Input has different freq=None from PeriodArray"
with pytest.raises(IncompatibleFrequency, match=msg):
pi - td64obj
with pytest.raises(IncompatibleFrequency, match=msg):
pi[0] - td64obj

else:
# With non-Tick freq, we could not add timedelta64 array regardless
# of what its resolution is
msg = "Cannot add or subtract timedelta64"
with pytest.raises(TypeError, match=msg):
pi - td64obj
with pytest.raises(TypeError, match=msg):
pi[0] - td64obj

# -----------------------------------------------------------------
# operations with array/Index of DateOffset objects

Expand Down
7 changes: 0 additions & 7 deletions pandas/tests/arithmetic/test_timedelta64.py
Original file line number Diff line number Diff line change
Expand Up @@ -1081,16 +1081,9 @@ def test_td64arr_sub_periodlike(self, box_with_array, tdi_freq, pi_freq):
with pytest.raises(TypeError, match=msg):
tdi - pi

# FIXME: don't leave commented-out
# FIXME: this raises with period scalar but not with PeriodIndex?
# with pytest.raises(TypeError):
# pi - tdi

# GH#13078 subtraction of Period scalar not supported
with pytest.raises(TypeError, match=msg):
tdi - pi[0]
with pytest.raises(TypeError, match=msg):
pi[0] - tdi

@pytest.mark.parametrize(
"other",
Expand Down