Skip to content

API/DEPR: 'periods' argument instead of 'n' for DatetimeIndex.shift() #22697

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 12 commits into from
Sep 20, 2018
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.24.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,7 @@ Deprecations
- :meth:`Series.str.cat` has deprecated using arbitrary list-likes *within* list-likes. A list-like container may still contain
many ``Series``, ``Index`` or 1-dimensional ``np.ndarray``, or alternatively, only scalar values. (:issue:`21950`)
- :meth:`FrozenNDArray.searchsorted` has deprecated the ``v`` parameter in favor of ``value`` (:issue:`14645`)
- :func:`DatetimeIndex.shift` now accepts ``periods`` argument instead of ``n`` for consistency with :func:`Index.shift` and :func:`Series.shift`. Using ``n`` throws a deprecation warning (:issue:`22458`)

.. _whatsnew_0240.prior_deprecations:

Expand Down
39 changes: 27 additions & 12 deletions pandas/core/arrays/datetimelike.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
from pandas.core.algorithms import checked_add_with_arr

from .base import ExtensionOpsMixin
from pandas.util._decorators import deprecate_kwarg


def _make_comparison_op(op, cls):
Expand Down Expand Up @@ -522,40 +523,54 @@ def _addsub_offset_array(self, other, op):
kwargs['freq'] = 'infer'
return type(self)(res_values, **kwargs)

def shift(self, n, freq=None):
@deprecate_kwarg(old_arg_name='n', new_arg_name='periods')
def shift(self, periods, freq=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to deprecate periods. Your change would break people doing .shift(n=1).

This could be somewhat tricky to do... I think you may have to make the signature *args, **kwargs, and manually do the parsing

  • shift(1): no warning
  • shift(n=1): warning
  • shift(periods=1): no warning

lmk if you need help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TomAugspurger Thanks. I started parsing manually but couldn't make some tests pass. Then I realized there is a decorator in pandas/pandas/util/_decorators.py that I could use for deprecating n. Lmk if there is anything else to be done.

"""
Specialized shift which produces a Datetime/Timedelta Array/Index
Shift index by desired number of time frequency increments.

This method is for shifting the values of datetime-like indexes
by a specified time increment a given number of times.

Parameters
----------
n : int
Periods to shift by
freq : DateOffset or timedelta-like, optional
periods : int
Number of periods (or increments) to shift by,
can be positive or negative.

.. versionchanged:: 0.24.0

freq : pandas.DateOffset, pandas.Timedelta or string, optional
Frequency increment to shift by.
If None, the index is shifted by its own `freq` attribute.
Offset aliases are valid strings, e.g., 'D', 'W', 'M' etc.

Returns
-------
shifted : same type as self
pandas.DatetimeIndex
Shifted index.

See Also
--------
Index.shift : Shift values of Index.
"""
if freq is not None and freq != self.freq:
if isinstance(freq, compat.string_types):
freq = frequencies.to_offset(freq)
offset = n * freq
offset = periods * freq
result = self + offset

if hasattr(self, 'tz'):
result._tz = self.tz

return result

if n == 0:
if periods == 0:
# immutable so OK
return self.copy()

if self.freq is None:
raise NullFrequencyError("Cannot shift with no freq")

start = self[0] + n * self.freq
end = self[-1] + n * self.freq
start = self[0] + periods * self.freq
end = self[-1] + periods * self.freq
attribs = self._get_attributes_dict()
return self._generate_range(start=start, end=end, periods=None,
**attribs)
Expand Down
5 changes: 5 additions & 0 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -8298,6 +8298,11 @@ def mask(self, cond, other=np.nan, inplace=False, axis=None, level=None,
See Notes.
axis : %(axes_single_arg)s

See Also
--------
Index.shift : Shift values of Index.
DatetimeIndex.shift : Shift values of DatetimeIndex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jreback I also added Index.shift here.

Notes
-----
If freq is specified then the index values are shifted but the data
Expand Down
10 changes: 10 additions & 0 deletions pandas/tests/indexes/datetimes/test_ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,16 @@ def test_shift(self):
shifted = rng.shift(1, freq=CDay())
assert shifted[0] == rng[0] + CDay()

def test_shift_periods(self):
# GH #22458 : argument 'n' was deprecated in favor of 'periods'
idx = pd.DatetimeIndex(start=START, end=END,
periods=3)
tm.assert_index_equal(idx.shift(periods=0), idx)
tm.assert_index_equal(idx.shift(0), idx)
with tm.assert_produces_warning(FutureWarning,
check_stacklevel=False):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you change this to true? If the test fails, you may need to increase the stacklevel 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@TomAugspurger done, test passed too.

tm.assert_index_equal(idx.shift(n=0), idx)

def test_pickle_unpickle(self):
unpickled = tm.round_trip_pickle(self.rng)
assert unpickled.freq is not None
Expand Down