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

arminv · 2018-09-13T17:47:41Z

closes Inconsistent keyword argument name for the 'periods' in pandas.Index.shift for a DatetimeIndex #22458
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

In order to be consistent with Index.shift & Series.shift, n argument was deprecated in favor of periods.

In [2]: idx = pd.DatetimeIndex(start='2014-08-01 10:00', freq='H',
   ...:                         periods=3, tz='Asia/Calcutta')
   ...:                         

In [3]: idx
Out[3]: 
DatetimeIndex(['2014-08-01 10:00:00+05:30', '2014-08-01 11:00:00+05:30',
               '2014-08-01 12:00:00+05:30'],
              dtype='datetime64[ns, Asia/Calcutta]', freq='H')

In [4]: idx.shift(1, freq=None)
Out[4]: 
DatetimeIndex(['2014-08-01 11:00:00+05:30', '2014-08-01 12:00:00+05:30',
               '2014-08-01 13:00:00+05:30'],
              dtype='datetime64[ns, Asia/Calcutta]', freq='H')

In [5]: idx.shift(periods=1, freq=None)
Out[5]: 
DatetimeIndex(['2014-08-01 11:00:00+05:30', '2014-08-01 12:00:00+05:30',
               '2014-08-01 13:00:00+05:30'],
              dtype='datetime64[ns, Asia/Calcutta]', freq='H')

In [6]: idx.shift(n=1, freq=None)
//anaconda/envs/pandas_dev/bin/ipython:1: FutureWarning: the 'n' keyword is deprecated, use 'periods' instead
  #!//anaconda/envs/pandas_dev/bin/python
Out[6]: 
DatetimeIndex(['2014-08-01 11:00:00+05:30', '2014-08-01 12:00:00+05:30',
               '2014-08-01 13:00:00+05:30'],
              dtype='datetime64[ns, Asia/Calcutta]', freq='H')

pep8speaks · 2018-09-13T17:47:44Z

Hello @arminv! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/core/arrays/datetimelike.py !
There are no PEP8 issues in the file pandas/tests/indexes/datetimes/test_ops.py !

Comment last updated on September 15, 2018 at 18:58 Hours UTC

TomAugspurger · 2018-09-13T20:25:32Z

pandas/core/arrays/datetimelike.py

@@ -521,40 +521,52 @@ def _addsub_offset_array(self, other, op):
            kwargs['freq'] = 'infer'
        return type(self)(res_values, **kwargs)

-    def shift(self, n, freq=None):
+    def shift(self, periods, freq=None):


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.

@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.

doc/source/whatsnew/v0.24.0.txt

pandas/tests/indexes/datetimes/test_ops.py

pandas/core/arrays/datetimelike.py

codecov · 2018-09-18T20:11:28Z

Codecov Report

Merging #22697 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #22697      +/-   ##
==========================================
+ Coverage   92.17%   92.17%   +<.01%     
==========================================
  Files         169      169              
  Lines       50778    50780       +2     
==========================================
+ Hits        46807    46809       +2     
  Misses       3971     3971

Flag	Coverage Δ
#multiple	`90.59% <100%> (ø)`	⬆️
#single	`42.33% <42.85%> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/core/generic.py	`96.67% <ø> (ø)`	⬆️
pandas/core/arrays/datetimelike.py	`95.53% <100%> (+0.02%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c113db...d9bbfe8. Read the comment docs.

arminv · 2018-09-18T20:12:54Z

pandas/core/generic.py

+        --------
+        Index.shift : Shift values of Index.
+        DatetimeIndex.shift : Shift values of DatetimeIndex.
+


@jreback I also added Index.shift here.

TomAugspurger · 2018-09-19T21:00:20Z

pandas/tests/indexes/datetimes/test_ops.py

+        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):


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

@TomAugspurger done, test passed too.

TomAugspurger · 2018-09-20T22:18:34Z

Thanks!

…pandas-dev#22697)

initial commit

3e3c5a0

TomAugspurger reviewed Sep 13, 2018

View reviewed changes

Merge remote-tracking branch 'upstream/master' into periods_keyword

39141c0

gfyoung added API Design Datetime Datetime data dtype Deprecate Functionality to remove in pandas labels Sep 14, 2018

arminv added 6 commits September 14, 2018 14:22

Merge remote-tracking branch 'upstream/master' into periods_keyword

1df9a9f

added args/kwargs signature

5b592f6

Change signature and add tests

f684245

use decorator for parsing/deprecating kwargs

240aa7e

Merge remote-tracking branch 'upstream/master' into periods_keyword

11169fa

PEP8

6f627ac

arminv changed the title ~~API: 'periods' argument instead of 'n' for DatetimeIndex.shift()~~ API/DEPR: 'periods' argument instead of 'n' for DatetimeIndex.shift() Sep 15, 2018

jreback requested changes Sep 18, 2018

View reviewed changes

doc/source/whatsnew/v0.24.0.txt Outdated Show resolved Hide resolved

pandas/tests/indexes/datetimes/test_ops.py Outdated Show resolved Hide resolved

pandas/core/arrays/datetimelike.py Show resolved Hide resolved

pandas/core/arrays/datetimelike.py Show resolved Hide resolved

arminv added 2 commits September 18, 2018 14:55

Merge remote-tracking branch 'upstream/master' into periods_keyword

d0d8b83

doc changes

ff335a5

arminv commented Sep 18, 2018

View reviewed changes

TomAugspurger reviewed Sep 19, 2018

View reviewed changes

arminv added 2 commits September 19, 2018 17:46

Merge remote-tracking branch 'upstream/master' into periods_keyword

de54cce

check_stacklevel=True

d9bbfe8

TomAugspurger merged commit 4612a82 into pandas-dev:master Sep 20, 2018

TomAugspurger added this to the 0.24.0 milestone Sep 20, 2018

arminv deleted the periods_keyword branch September 22, 2018 17:55

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

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

54f313b

…pandas-dev#22697)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

arminv commented Sep 13, 2018 •

edited

Loading

pep8speaks commented Sep 13, 2018 •

edited

Loading

TomAugspurger Sep 13, 2018

arminv Sep 15, 2018

codecov bot commented Sep 18, 2018 •

edited

Loading

arminv Sep 18, 2018

TomAugspurger Sep 19, 2018

arminv Sep 19, 2018

TomAugspurger commented Sep 20, 2018

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

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

Conversation

arminv commented Sep 13, 2018 • edited Loading

pep8speaks commented Sep 13, 2018 • edited Loading

Comment last updated on September 15, 2018 at 18:58 Hours UTC

TomAugspurger Sep 13, 2018

Choose a reason for hiding this comment

arminv Sep 15, 2018

Choose a reason for hiding this comment

codecov bot commented Sep 18, 2018 • edited Loading

Codecov Report

arminv Sep 18, 2018

Choose a reason for hiding this comment

TomAugspurger Sep 19, 2018

Choose a reason for hiding this comment

arminv Sep 19, 2018

Choose a reason for hiding this comment

TomAugspurger commented Sep 20, 2018

arminv commented Sep 13, 2018 •

edited

Loading

pep8speaks commented Sep 13, 2018 •

edited

Loading

codecov bot commented Sep 18, 2018 •

edited

Loading