Merge branch 'master' of https://github.com/pandas-dev/pandas into dlike8

Author: jbrockmendel

doc/source/whatsnew/v0.24.0.txt

@@ -789,6 +789,7 @@ Datetimelike
 - Bug in :class:`DatetimeIndex` where frequency was being set if original frequency was ``None`` (:issue:`22150`)
 - 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`)
 - Bug in :func:`to_datetime` with an :class:`Index` argument that would drop the ``name`` from the result (:issue:`21697`)
+- Bug in :class:`PeriodIndex` where adding or subtracting a :class:`timedelta` or :class:`Tick` object produced incorrect results (:issue:`22988`)
 
 Timedelta
 ^^^^^^^^^

pandas/core/arrays/datetimelike.py

@@ -399,6 +399,11 @@ def _add_delta_tdi(self, other):
         if not len(self) == len(other):
             raise ValueError("cannot add indices of unequal length")
 
+        if isinstance(other, np.ndarray):
+            # ndarray[timedelta64]; wrap in TimedeltaIndex for op
+            from pandas import TimedeltaIndex
+            other = TimedeltaIndex(other)
+
         self_i8 = self.asi8
         other_i8 = other.asi8
         new_values = checked_add_with_arr(self_i8, other_i8,
@@ -652,11 +657,17 @@ def __add__(self, other):
                 return self._add_datelike(other)
             elif is_integer_dtype(other):
                 result = self._addsub_int_array(other, operator.add)
-            elif is_float_dtype(other) or is_period_dtype(other):
+            elif is_float_dtype(other):
                 # Explicitly catch invalid dtypes
                 raise TypeError("cannot add {dtype}-dtype to {cls}"
                                 .format(dtype=other.dtype,
                                         cls=type(self).__name__))
+            elif is_period_dtype(other):
+                # if self is a TimedeltaArray and other is a PeriodArray with
+                #  a timedelta-like (i.e. Tick) freq, this operation is valid.
+                #  Defer to the PeriodArray implementation.
+                # In remaining cases, this will end up raising TypeError.
+                return NotImplemented
             elif is_extension_array_dtype(other):
                 # Categorical op will raise; defer explicitly
                 return NotImplemented

pandas/core/arrays/datetimes.py

@@ -497,7 +497,7 @@ def _add_delta(self, delta):
         Parameters
         ----------
         delta : {timedelta, np.timedelta64, DateOffset,
-                 TimedelaIndex, ndarray[timedelta64]}
+                 TimedeltaIndex, ndarray[timedelta64]}
 
         Returns
         -------

pandas/core/arrays/period.py

@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 from datetime import timedelta
+import operator
 import warnings
 
 import numpy as np
@@ -17,9 +18,9 @@
 from pandas.util._decorators import (cache_readonly, deprecate_kwarg)
 
 from pandas.core.dtypes.common import (
-    is_integer_dtype, is_float_dtype, is_period_dtype,
+    is_integer_dtype, is_float_dtype, is_period_dtype, is_timedelta64_dtype,
     is_object_dtype,
-    is_datetime64_dtype)
+    is_datetime64_dtype, _TD_DTYPE)
 from pandas.core.dtypes.dtypes import PeriodDtype
 from pandas.core.dtypes.generic import ABCSeries
 
@@ -377,24 +378,54 @@ def _add_offset(self, other):
         return self._time_shift(other.n)
 
     def _add_delta_td(self, other):
+        assert isinstance(self.freq, Tick)  # checked by calling function
         assert isinstance(other, (timedelta, np.timedelta64, Tick))
-        nanos = delta_to_nanoseconds(other)
-        own_offset = frequencies.to_offset(self.freq.rule_code)
 
-        if isinstance(own_offset, Tick):
-            offset_nanos = delta_to_nanoseconds(own_offset)
-            if np.all(nanos % offset_nanos == 0):
-                return self._time_shift(nanos // offset_nanos)
+        delta = self._check_timedeltalike_freq_compat(other)
 
-        # raise when input doesn't have freq
-        raise IncompatibleFrequency("Input has different freq from "
-                                    "{cls}(freq={freqstr})"
-                                    .format(cls=type(self).__name__,
-                                            freqstr=self.freqstr))
+        # Note: when calling parent class's _add_delta_td, it will call
+        #  delta_to_nanoseconds(delta).  Because delta here is an integer,
+        #  delta_to_nanoseconds will return it unchanged.
+        return DatetimeLikeArrayMixin._add_delta_td(self, delta)
+
+    def _add_delta_tdi(self, other):
+        assert isinstance(self.freq, Tick)  # checked by calling function
+
+        delta = self._check_timedeltalike_freq_compat(other)
+        return self._addsub_int_array(delta, operator.add)
 
     def _add_delta(self, other):
-        ordinal_delta = self._maybe_convert_timedelta(other)
-        return self._time_shift(ordinal_delta)
+        """
+        Add a timedelta-like, Tick, or TimedeltaIndex-like object
+        to self.
+
+        Parameters
+        ----------
+        other : {timedelta, np.timedelta64, Tick,
+                 TimedeltaIndex, ndarray[timedelta64]}
+
+        Returns
+        -------
+        result : same type as self
+        """
+        if not isinstance(self.freq, Tick):
+            # We cannot add timedelta-like to non-tick PeriodArray
+            raise IncompatibleFrequency("Input has different freq from "
+                                        "{cls}(freq={freqstr})"
+                                        .format(cls=type(self).__name__,
+                                                freqstr=self.freqstr))
+
+        # TODO: standardize across datetimelike subclasses whether to return
+        #  i8 view or _shallow_copy
+        if isinstance(other, (Tick, timedelta, np.timedelta64)):
+            new_values = self._add_delta_td(other)
+            return self._shallow_copy(new_values)
+        elif is_timedelta64_dtype(other):
+            # ndarray[timedelta64] or TimedeltaArray/index
+            new_values = self._add_delta_tdi(other)
+            return self._shallow_copy(new_values)
+        else:  # pragma: no cover
+            raise TypeError(type(other).__name__)
 
     @deprecate_kwarg(old_arg_name='n', new_arg_name='periods')
     def shift(self, periods):
@@ -450,14 +481,9 @@ def _maybe_convert_timedelta(self, other):
                 other, (timedelta, np.timedelta64, Tick, np.ndarray)):
             offset = frequencies.to_offset(self.freq.rule_code)
             if isinstance(offset, Tick):
-                if isinstance(other, np.ndarray):
-                    nanos = np.vectorize(delta_to_nanoseconds)(other)
-                else:
-                    nanos = delta_to_nanoseconds(other)
-                offset_nanos = delta_to_nanoseconds(offset)
-                check = np.all(nanos % offset_nanos == 0)
-                if check:
-                    return nanos // offset_nanos
+                # _check_timedeltalike_freq_compat will raise if incompatible
+                delta = self._check_timedeltalike_freq_compat(other)
+                return delta
         elif isinstance(other, DateOffset):
             freqstr = other.rule_code
             base = frequencies.get_base_alias(freqstr)
@@ -476,6 +502,58 @@ def _maybe_convert_timedelta(self, other):
         raise IncompatibleFrequency(msg.format(cls=type(self).__name__,
                                                freqstr=self.freqstr))
 
+    def _check_timedeltalike_freq_compat(self, other):
+        """
+        Arithmetic operations with timedelta-like scalars or array `other`
+        are only valid if `other` is an integer multiple of `self.freq`.
+        If the operation is valid, find that integer multiple.  Otherwise,
+        raise because the operation is invalid.
+
+        Parameters
+        ----------
+        other : timedelta, np.timedelta64, Tick,
+                ndarray[timedelta64], TimedeltaArray, TimedeltaIndex
+
+        Returns
+        -------
+        multiple : int or ndarray[int64]
+
+        Raises
+        ------
+        IncompatibleFrequency
+        """
+        assert isinstance(self.freq, Tick)  # checked by calling function
+        own_offset = frequencies.to_offset(self.freq.rule_code)
+        base_nanos = delta_to_nanoseconds(own_offset)
+
+        if isinstance(other, (timedelta, np.timedelta64, Tick)):
+            nanos = delta_to_nanoseconds(other)
+
+        elif isinstance(other, np.ndarray):
+            # numpy timedelta64 array; all entries must be compatible
+            assert other.dtype.kind == 'm'
+            if other.dtype != _TD_DTYPE:
+                # i.e. non-nano unit
+                # TODO: disallow unit-less timedelta64
+                other = other.astype(_TD_DTYPE)
+            nanos = other.view('i8')
+        else:
+            # TimedeltaArray/Index
+            nanos = other.asi8
+
+        if np.all(nanos % base_nanos == 0):
+            # nanos being added is an integer multiple of the
+            #  base-frequency to self.freq
+            delta = nanos // base_nanos
+            # delta is the integer (or integer-array) number of periods
+            # by which will be added to self.
+            return delta
+
+        raise IncompatibleFrequency("Input has different freq from "
+                                    "{cls}(freq={freqstr})"
+                                    .format(cls=type(self).__name__,
+                                            freqstr=self.freqstr))
+
 
 PeriodArrayMixin._add_comparison_ops()
 PeriodArrayMixin._add_datetimelike_methods()