8000 BUG: Fix PeriodIndex +/- TimedeltaIndex (#23031) · pandas-dev/pandas@cf11f71 · GitHub
[go: up one dir, main page]
Skip to content

Commit cf11f71

Browse files
jbrockmendeljbrockmendel
authored
BUG: Fix PeriodIndex +/- TimedeltaIndex (#23031)
* Fix PeriodIndex +- TimedeltaIndex * better comment * typo fixup * Allow _add_delta_tdi to handle ndarray[timedelta64] gracefully * Implement specialized versions of _add_delta, _add_delta_tdi * Add tests for adding offset scalar * add tests for subtracting offset scalar * remove superfluous comment * whatsnew * Revert incorrect docstring change * docstring * comment * comment
1 parent f9d237b commit cf11f71

File tree

6 files changed

+183
-30
lines changed

6 files changed

+183
-30
lines changed

‎doc/source/whatsnew/v0.24.0.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,7 @@ Datetimelike
789789
- Bug in :class:`DatetimeIndex` where frequency was being set if original frequency was ``None`` (:issue:`22150`)
790790
- Bug in rounding methods of :class:`DatetimeIndex` (:meth:`~DatetimeIndex.round`, :meth:`~DatetimeIndex.ceil`, :meth:`~DatetimeIndex.floor`) and :class:`Timestamp` (:meth:`~Timestamp.round`, :meth:`~Timestamp.ceil`, :meth:`~Timestamp.floor`) could give rise to loss of precision (:issue:`22591`)
791791
- Bug in :func:`to_datetime` with an :class:`Index` argument that would drop the ``name`` from the result (:issue:`21697`)
792+
- Bug in :class:`PeriodIndex` where adding or subtracting a :class:`timedelta` or :class:`Tick` object produced incorrect results (:issue:`22988`)
792793

793794
Timedelta
794795
^^^^^^^^^

‎pandas/core/arrays/datetimelike.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,11 @@ def _add_delta_tdi(self, other):
383383
if not len(self) == len(other):
384384
raise ValueError("cannot add indices of unequal length")
385385

386+
if isinstance(other, np.ndarray):
387< 8000 code class="diff-text syntax-highlighted-line addition">+
# ndarray[timedelta64]; wrap in TimedeltaIndex for op
388+
from pandas import TimedeltaIndex
389+
other = TimedeltaIndex(other)
390+
386391
self_i8 = self.asi8
387392
other_i8 = other.asi8
388393
new_values = checked_add_with_arr(self_i8, other_i8,
@@ -633,11 +638,17 @@ def __add__(self, other):
633638
return self._add_datelike(other)
634639
elif is_integer_dtype(other):
635640
result = self._addsub_int_array(other, operator.add)
636-
elif is_float_dtype(other) or is_period_dtype(other):
641+
elif is_float_dtype(other):
637642
# Explicitly catch invalid dtypes
638643
raise TypeError("cannot add {dtype}-dtype to {cls}"
639644
.format(dtype=other.dtype,
640645
cls=type(self).__name__))
646+
elif is_period_dtype(other):
647+
# if self is a TimedeltaArray and other is a PeriodArray with
648+
# a timedelta-like (i.e. Tick) freq, this operation is valid.
649+
# Defer to the PeriodArray implementation.
650+
# In remaining cases, this will end up raising TypeError.
651+
return NotImplemented
641652
elif is_extension_array_dtype(other):
642653
# Categorical op will raise; defer explicitly
643654
return NotImplemented

‎pandas/core/arrays/datetimes.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ def _add_delta(self, delta):
497497
Parameters
498498
----------
499499
delta : {timedelta, np.timedelta64, DateOffset,
500-
TimedelaIndex, ndarray[timedelta64]}
500+
TimedeltaIndex, ndarray[timedelta64]}
501501
502502
Returns
503503
-------

‎pandas/core/arrays/period.py

Lines changed: 101 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# -*- coding: utf-8 -*-
22
from datetime import timedelta
3+
import operator
34
import warnings
45

56
import numpy as np
@@ -17,8 +18,8 @@
1718
from pandas.util._decorators import (cache_readonly, deprecate_kwarg)
1819

1920
from pandas.core.dtypes.common import (
20-
is_integer_dtype, is_float_dtype, is_period_dtype,
21-
is_datetime64_dtype)
21+
is_integer_dtype, is_float_dtype, is_period_dtype, is_timedelta64_dtype,
22+
is_datetime64_dtype, _TD_DTYPE)
2223
from pandas.core.dtypes.dtypes import PeriodDtype
2324
from pandas.core.dtypes.generic import ABCSeries
2425

@@ -362,24 +363,54 @@ def _add_offset(self, other):
362363
return self._time_shift(other.n)
363364

364365
def _add_delta_td(self, other):
366+
assert isinstance(self.freq, Tick) # checked by calling function
365367
assert isinstance(other, (timedelta, np.timedelta64, Tick))
366-
nanos = delta_to_nanoseconds(other)
367-
own_offset = frequencies.to_offset(self.freq.rule_code)
368368

369-
if isinstance(own_offset, Tick):
370-
offset_nanos = delta_to_nanoseconds(own_offset)
371-
if np.all(nanos % offset_nanos == 0):
372-
return self._time_shift(nanos // offset_nanos)
369+
delta = self._check_timedeltalike_freq_compat(other)
373370

374-
# raise when input doesn't have freq
375-
raise IncompatibleFrequency("Input has different freq from "
376-
"{cls}(freq={freqstr})"
377-
.format(cls=type(self).__name__,
378-
freqstr=self.freqstr))
371+
# Note: when calling parent class's _add_delta_td, it will call
372+
# delta_to_nanoseconds(delta). Because delta here is an integer,
373+
# delta_to_nanoseconds will return it unchanged.
374+
return DatetimeLikeArrayMixin._add_delta_td(self, delta)
375+
376+
def _add_delta_tdi(self, other):
377+
assert isinstance(self.freq, Tick) # checked by calling function
378+
379+
delta = self._check_timedeltalike_freq_compat(other)
380+
return self._addsub_int_array(delta, operator.add)
379381

380382
def _add_delta(self, other):
381-
ordinal_delta = self._maybe_convert_timedelta(other)
382-
return self._time_shift(ordinal_delta)
383+
"""
384+
Add a timedelta-like, Tick, or TimedeltaIndex-like object
385+
to self.
386+
387+
Parameters
388+
----------
389+
other : {timedelta, np.timedelta64, Tick,
390+
TimedeltaIndex, ndarray[timedelta64]}
391+
392+
Returns
393+
-------
394+
result : same type as self
395+
"""
396+
if not isinstance(self.freq, Tick):
397+
# We cannot add timedelta-like to non-tick PeriodArray
398+
raise IncompatibleFrequency("Input has different freq from "
399+
"{cls}(freq={freqstr})"
400+
.format(cls=type(self).__name__,
401+
freqstr=self.freqstr))
402+
403+
# TODO: standardize across datetimelike subclasses whether to return
404+
# i8 view or _shallow_copy
405+
if isinstance(other, (Tick, timedelta, np.timedelta64)):
406+
new_values = self._add_delta_td(other)
407+
return self._shallow_copy(new_values)
408+
elif is_timedelta64_dtype(other):
409+
# ndarray[timedelta64] or TimedeltaArray/index
410+
new_values = self._add_delta_tdi(other)
411+
return self._shallow_copy(new_values)
412+
else: # pragma: no cover
413+
raise TypeError(type(other).__name__)
383414

384415
@deprecate_kwarg(old_arg_name='n', new_arg_name='periods')
385416
def shift(self, periods):
@@ -435,14 +466,9 @@ def _maybe_convert_timedelta(self, other):
435466
other, (timedelta, np.timedelta64, Tick, np.ndarray)):
436467
offset = frequencies.to_offset(self.freq.rule_code)
437468
if isinstance(offset, Tick):
438-
if isinstance(other, np.ndarray):
439-
nanos = np.vectorize(delta_to_nanoseconds)(other)
440-
else:
441-
nanos = delta_to_nanoseconds(other)
442-
offset_nanos = delta_to_nanoseconds(offset)
443-
check = np.all(nanos % offset_nanos == 0)
444-
if check:
445-
return nanos // offset_nanos
469+
# _check_timedeltalike_freq_compat will raise if incompatible
470+
delta = self._check_timedeltalike_freq_compat(other)
471+
return delta
446472
elif isinstance(other, DateOffset):
447473
freqstr = other.rule_code
448474
base = frequencies.get_base_alias(freqstr)
@@ -461,6 +487,58 @@ def _maybe_convert_timedelta(self, other):
461487
raise IncompatibleFrequency(msg.format(cls=type(self).__name__,
462488
freqstr=self.freqstr))
463489

490+
def _check_timedeltalike_freq_compat(self, other):
491+
"""
492+
Arithmetic operations with timedelta-like scalars or array `other`
493+
are only valid if `other` is an integer multiple of `self.freq`.
494+
If the operation is valid, find that integer multiple. Otherwise,
495+
raise because the operation is invalid.
496+
497+
Parameters
498+
----------
499+
other : timedelta, np.timedelta64, Tick,
500+
ndarray[timedelta64], TimedeltaArray, TimedeltaIndex
501+
502+
Returns
503+
-------
504+
multiple : int or ndarray[int64]
505+
506+
Raises
507+
------
508+
IncompatibleFrequency
509+
"""
510+
assert isinstance(self.freq, Tick) # checked by calling function
511+
own_offset = frequencies.to_offset(self.freq.rule_code)
512+
base_nanos = delta_to_nanoseconds(own_offset)
513+
514+
if isinstance(other, (timedelta, np.timedelta64, Tick)):
515+
nanos = delta_to_nanoseconds(other)
516+
517+
elif isinstance(other, np.ndarray):
518+
# numpy timedelta64 array; all entries must be compatible
519+
assert other.dtype.kind == 'm'
520+
if other.dtype != _TD_DTYPE:
521+
# i.e. non-nano unit
522+
# TODO: disallow unit-less timedelta64
523+
other = other.astype(_TD_DTYPE)
524+
nanos = other.view('i8')
525+
else:
526+
# TimedeltaArray/Index
527+
nanos = other.asi8
528+
529+
if np.all(nanos % base_nanos == 0):
530+
# nanos being added is an integer multiple of the
531+
# base-frequency to self.freq
532+
delta = nanos // base_nanos
533+
# delta is the integer (or integer-array) number of periods
534+
# by which will be added to self.
535+
return delta
536+
537+
raise IncompatibleFrequency("Input has different freq from "
538+
"{cls}(freq={freqstr})"
539+
.format(cls=type(self).__name__,
540+
freqstr=self.freqstr))
541+
464542

465543
PeriodArrayMixin._add_comparison_ops()
466544
PeriodArrayMixin._add_datetimelike_methods()

‎pandas/tests/arithmetic/test_period.py

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -446,26 +446,36 @@ def test_pi_add_sub_td64_array_non_tick_raises(self):
446446
with pytest.raises(period.IncompatibleFrequency):
447447
tdarr - rng
448448

449-
@pytest.mark.xfail(reason='op with TimedeltaIndex raises, with ndarray OK',
450-
strict=True)
451449
def test_pi_add_sub_td64_array_tick(self):
452-
rng = pd.period_range('1/1/2000', freq='Q', periods=3)
450+
# PeriodIndex + Timedelta-like is allowed only with
451+
# tick-like frequencies
452+
rng = pd.period_range('1/1/2000', freq='90D', periods=3)
453453
tdi = pd.TimedeltaIndex(['-1 Day', '-1 Day', '-1 Day'])
454454
tdarr = tdi.values
455455

456-
expected = rng + tdi
456+
expected = pd.period_range('12/31/1999', freq='90D', periods=3)
457+
result = rng + tdi
458+
tm.assert_index_equal(result, expected)
457459
result = rng + tdarr
458460
tm.assert_index_equal(result, expected)
461+
result = tdi + rng
462+
tm.assert_index_equal(result, expected)
459463
result = tdarr + rng
460464
tm.assert_index_equal(result, expected)
461465

462-
expected = rng - tdi
466+
expected = pd.period_range('1/2/2000', freq='90D', periods=3)
467+
468+
result = rng - tdi
469+
tm.assert_index_equal(result, expected)
463470
result = rng - tdarr
464471
tm.assert_index_equal(result, expected)
465472

466473
with pytest.raises(TypeError):
467474
tdarr - rng
468475

476+
with pytest.raises(TypeError):
477+
tdi - rng
478+
469479
# -----------------------------------------------------------------
470480
# operations with array/Index of DateOffset objects
471481

@@ -596,6 +606,56 @@ def test_pi_sub_intarray(self, box):
596606
# Timedelta-like (timedelta, timedelta64, Timedelta, Tick)
597607
# TODO: Some of these are misnomers because of non-Tick DateOffsets
598608

609+
def test_pi_add_timedeltalike_minute_gt1(self, three_days):
610+
# GH#23031 adding a time-delta-like offset to a PeriodArray that has
611+
# minute frequency with n != 1. A more general case is tested below
612+
# in test_pi_add_timedeltalike_tick_gt1, but here we write out the
613+
# expected result more explicitly.
614+
other = three_days
615+
rng = pd.period_range('2014-05-01', periods=3, freq='2D')
616+
617+
expected = pd.PeriodIndex(['2014-05-04', '2014-05-06', '2014-05-08'],
618+
freq='2D')
619+
620+
result = rng + other
621+
tm.assert_index_equal(result, expected)
622+
623+
result = other + rng
624+
tm.assert_index_equal(result, expected)
625+
626+
# subtraction
627+
expected = pd.PeriodIndex(['2014-04-28', '2014-04-30', '2014-05-02'],
628+
freq='2D')
629+
result = rng - other
630+
tm.assert_index_equal(result, expected)
631+
632+
with pytest.raises(TypeError):
633+
other - rng
634+
635+
@pytest.mark.parametrize('freqstr', ['5ns', '5us', '5ms',
636+
'5s', '5T', '5h', '5d'])
637+
def test_pi_add_timedeltalike_tick_gt1(self, three_days, freqstr):
638+
# GH#23031 adding a time-delta-like offset to a PeriodArray that has
639+
# tick-like frequency with n != 1
640+
other = three_days
641+
rng = pd.period_range('2014-05-01', periods=6, freq=freqstr)
642+
643+
expected = pd.period_range(rng[0] + other, periods=6, freq=freqstr)
644+
645+
result = rng + other
646+
tm.assert_index_equal(result, expected)
647+
648+
result = other + rng
649+
tm.assert_index_equal(result, expected)
650+
651+
# subtraction
652+
expected = pd.period_range(rng[0] - other, periods=6, freq=freqstr)
653+
result = rng - other
654+
tm.assert_index_equal(result, expected)
655+
656+
with pytest.raises(TypeError):
657+
other - rng
658+
599659
def test_pi_add_iadd_timedeltalike_daily(self, three_days):
600660
# Tick
601661
other = three_days

‎pandas/util/testing.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,7 @@ def assert_index_equal(left, right, exact='equiv', check_names=True,
805805
Specify object name being compared, internally used to show appropriate
806806
assertion message
807807
"""
808+
__tracebackhide__ = True
808809

809810
def _check_types(l, r, obj='Index'):
810811
if exact:
@@ -1048,6 +1049,8 @@ def assert_interval_array_equal(left, right, exact='equiv',
10481049

10491050

10501051
def raise_assert_detail(obj, message, left, right, diff=None):
1052+
__tracebackhide__ = True
1053+
10511054
if isinstance(left, np.ndarray):
10521055
left = pprint_thing(left)
10531056
elif is_categorical_dtype(left):

0 commit comments

Comments
 (0)
0