Skip to content
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

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

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 @@ -528,6 +528,7 @@ Datetimelike API Changes
- :class:`PeriodIndex` subtraction of another ``PeriodIndex`` will now return an object-dtype :class:`Index` of :class:`DateOffset` objects instead of raising a ``TypeError`` (:issue:`20049`)
- :func:`cut` and :func:`qcut` now returns a :class:`DatetimeIndex` or :class:`TimedeltaIndex` bins when the input is datetime or timedelta dtype respectively and ``retbins=True`` (:issue:`19891`)
- :meth:`DatetimeIndex.to_period` and :meth:`Timestamp.to_period` will issue a warning when timezone information will be lost (:issue:`21333`)
- :func:`shift` now accepts ``periods`` argument instead of ``n`` for consistency with `Index.shift`. Using ``n`` throws a deprecation warning (:issue:`22458`)
arminv marked this conversation as resolved.
Show resolved Hide resolved

.. _whatsnew_0240.api.other:

Expand Down
36 changes: 24 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 @@ -521,40 +522,51 @@ 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.
arminv marked this conversation as resolved.
Show resolved Hide resolved
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
--------
arminv marked this conversation as resolved.
Show resolved Hide resolved
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
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 :
arminv marked this conversation as resolved.
Show resolved Hide resolved
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