DOC: update the pandas.Series.shift docstring #20472

ZackStone · 2018-03-23T18:54:42Z

Checklist for the pandas documentation sprint (ignore this if you are doing an unrelated PR):

PR title is "DOC: update the docstring"
The validation script passes: scripts/validate_docstrings.py <your-function-or-method>
The PEP8 style check passes: git diff upstream/master -u -- "*.py" | flake8 --diff
The html version looks good: python doc/make.py --single <your-function-or-method>
It has been proofread on language by another sprint participant

Please include the output of the validation script below between the "```" ticks:

################################################################################
####################### Docstring (pandas.Series.shift)  #######################
################################################################################

Shift index by desired number of periods with an optional time freq.

When freq is not passed, shift the index without realign the data.
If freq is passed (in this case, the index must be date or datetime),
the index will be increased using the periods and the freq.

Parameters
----------
periods : int
    Number of periods to move, can be positive or negative.
freq : DateOffset, timedelta, or time rule string, optional
    Increment to use from the tseries module or time rule (e.g. 'EOM').
    See Notes.
axis : int or str
    Shift direction. {0, 'index'}.

Notes
-----
If freq is specified then the index values are shifted but the data
is not realigned. That is, use freq if you would like to extend the
index when shifting and preserve the original data.

Returns
-------
shifted : Series

Examples
--------
>>> df = pd.DataFrame({'Col1': [10, 20, 30, 20, 15, 30, 45],
...                    'Col2': [13, 23, 33, 23, 18, 33, 48],
...                    'Col3': [17, 27, 37, 27, 22, 37, 52]})
>>> print(df)
   Col1  Col2  Col3
0    10    13    17
1    20    23    27
2    30    33    37
3    20    23    27
4    15    18    22
5    30    33    37
6    45    48    52

>>> df.shift(periods=3)
   Col1  Col2  Col3
0   NaN   NaN   NaN
1   NaN   NaN   NaN
2   NaN   NaN   NaN
3  10.0  13.0  17.0
4  20.0  23.0  27.0
5  30.0  33.0  37.0
6  20.0  23.0  27.0

>>> df.shift(periods=1, axis=1)
   Col1  Col2  Col3
0   NaN  10.0  13.0
1   NaN  20.0  23.0
2   NaN  30.0  33.0
3   NaN  20.0  23.0
4   NaN  15.0  18.0
5   NaN  30.0  33.0
6   NaN  45.0  48.0

>>> import datetime
>>> names = ['João', 'Maria', 'Emanuel', 'Jussara', 'José']
>>> dates = [datetime.datetime(2018, 3, 1, 11, 15),
...          datetime.datetime(2018, 3, 5, 11, 15),
...          datetime.datetime(2018, 3, 10, 11, 15),
...          datetime.datetime(2018, 3, 15, 11, 15),
...          datetime.datetime(2018, 3, 20, 11, 15)]
>>> df = pd.DataFrame(data={'names': names}, index=dates)
>>> print(df)
                       names
2018-03-01 11:15:00     João
2018-03-05 11:15:00    Maria
2018-03-10 11:15:00  Emanuel
2018-03-15 11:15:00  Jussara
2018-03-20 11:15:00     José

>>> df.shift(periods=2, freq='D')
                       names
2018-03-03 11:15:00     João
2018-03-07 11:15:00    Maria
2018-03-12 11:15:00  Emanuel
2018-03-17 11:15:00  Jussara
2018-03-22 11:15:00     José

>>> df.shift(periods=75, freq='min')
                       names
2018-03-01 12:30:00     João
2018-03-05 12:30:00    Maria
2018-03-10 12:30:00  Emanuel
2018-03-15 12:30:00  Jussara
2018-03-20 12:30:00     José

See Also
--------
slice_shift: Equivalent to shift without copying data.
tshift: Shift the time index, using the index’s frequency if available.

################################################################################
################################## Validation ##################################
################################################################################

Docstring for "pandas.Series.shift" correct. :)

WillAyd · 2018-03-26T02:46:45Z

pandas/core/generic.py

@@ -6815,16 +6815,21 @@ def mask(self, cond, other=np.nan, inplace=False, axis=None, level=None,
                          errors=errors)

    _shared_docs['shift'] = ("""
-        Shift index by desired number of periods with an optional time freq
+        Shift index by desired number of periods with an optional time freq.


References to parameters in the descriptions / notes should be put in backticks, so `freq` wherever applicable (couple of other instances below)

WillAyd · 2018-03-26T02:47:19Z

pandas/core/generic.py

-        Shift index by desired number of periods with an optional time freq
+        Shift index by desired number of periods with an optional time freq.
+
+        When freq is not passed, shift the index without realign the data.


"realigning" instead of "realign"

WillAyd · 2018-03-26T02:47:58Z

pandas/core/generic.py

+        Shift index by desired number of periods with an optional time freq.
+
+        When freq is not passed, shift the index without realign the data.
+        If freq is passed (in this case, the index must be date or datetime),


What happens if freq is passed and the index is not datetime-like? Perhaps this should have a Raises section?

Where is the docs to "Raises section"? I could not find it in https://python-sprints.github.io/pandas/guide/pandas_docstring.html

It would look something like:

Returns -------- same_class : yada yada Raises ------ ValueError : something went terribly horribly wrong

WillAyd · 2018-03-26T02:50:10Z

pandas/core/generic.py

        freq : DateOffset, timedelta, or time rule string, optional
            Increment to use from the tseries module or time rule (e.g. 'EOM').
            See Notes.
-        axis : %(axes_single_arg)s
+        axis : int or str


The axis parameter has it's own standard (see the guide):

axis : {0 or ‘index’, 1 or ‘columns’, None}, default None

WillAyd · 2018-03-26T02:51:33Z

pandas/core/generic.py

+
+        Examples
+        --------
+        >>> df = pd.DataFrame({'Col1': [10, 20, 30, 20, 15, 30, 45],


Not a bad example but I think we could save some space - could you limit the example to 5 rows of data? Shouldn't drastically affect your examples below

WillAyd · 2018-03-26T02:52:45Z

pandas/core/generic.py

+        >>> df = pd.DataFrame({'Col1': [10, 20, 30, 20, 15, 30, 45],
+        ...                    'Col2': [13, 23, 33, 23, 18, 33, 48],
+        ...                    'Col3': [17, 27, 37, 27, 22, 37, 52]})
+        >>> print(df)


Hmm well your DataFrame may be simple enough that you don't need to explicitly print it. That said, if you do need to print you can just say df instead of print(df).

I'll leave it as a judgment call on your end if you want this, but if so definitely get rid of the surrounding print statement

WillAyd · 2018-03-26T02:54:54Z

pandas/core/generic.py

+
+        See Also
+        --------
+        slice_shift: Equivalent to shift without copying data.


Need whitespace before each colon in this section

WillAyd · 2018-03-26T02:55:49Z

pandas/core/generic.py


        Parameters
        ----------
        periods : int
-            Number of periods to move, can be positive or negative
+            Number of periods to move, can be positive or negative.


Very minor but you want a semicolon instead of a comma here. Perhaps saying "shift" instead of move would be better

jreback · 2018-03-26T10:29:23Z

pandas/core/generic.py

+
+        See Also
+        --------
+        slice_shift: Equivalent to shift without copying data.


don't list slice_shift, this is being deprecated / removed

codecov · 2018-03-26T11:47:49Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #20472      +/-   ##
==========================================
+ Coverage   92.22%   92.23%   +<.01%     
==========================================
  Files         161      161              
  Lines       51187    51197      +10     
==========================================
+ Hits        47209    47220      +11     
+ Misses       3978     3977       -1

Flag	Coverage Δ
#multiple	`90.61% <100%> (ø)`	⬆️
#single	`42.27% <100%> (+0.01%)`	⬆️

Impacted Files	Coverage Δ
pandas/core/generic.py	`96.81% <100%> (ø)`	⬆️
pandas/core/arrays/timedeltas.py	`93.75% <0%> (-0.56%)`	⬇️
pandas/core/series.py	`93.87% <0%> (ø)`	⬆️
pandas/core/indexes/timedeltas.py	`90.74% <0%> (+0.12%)`	⬆️
pandas/core/indexes/frozen.py	`92.1% <0%> (+0.43%)`	⬆️
pandas/core/arrays/period.py	`98.08% <0%> (+0.59%)`	⬆️

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 4f71755...4d9c3c0. Read the comment docs.

ZackStone · 2018-03-26T12:12:21Z

@jreback, @WillAyd: updated.

WillAyd

Couple more changes. Can you also add a Raises section to describe what happens when freq is provided for something other than a DateTimeIndex?

WillAyd · 2018-03-26T15:11:25Z

pandas/core/generic.py

+
+        >>> import datetime
+        >>> names = ['João', 'Maria', 'Emanuel', 'Jussara', 'José']
+        >>> dates = [datetime.datetime(2018, 3, 1, 11, 15),


This can also be done in a more succinct manner by using pd.date_range. Trying something like:

pd.date_range(start='3/1/18 11:15:00', freq='3D', periods=5)

Can modify date / freq to whatever you want. Can get rid of the date time import then as well

WillAyd · 2018-03-26T15:13:02Z

pandas/core/generic.py


        Notes
        -----
-        If freq is specified then the index values are shifted but the data
-        is not realigned. That is, use freq if you would like to extend the
+        If `freq` is specified then the index values are shifted but the data


To reinforce this concept it would be good to add into the examples where appropriate (see below)

WillAyd · 2018-03-26T15:15:03Z

pandas/core/generic.py

+        3   NaN  30.0  33.0
+        4   NaN  45.0  48.0
+
+        >>> import datetime


Related to above comment, this example could use an introduction along the lines of "When using the freq parameter with a :class:pandas.DateTimeIndex the index values are shifted but the data is not realigned."

WillAyd · 2018-03-26T15:17:09Z

pandas/core/generic.py

-        If freq is specified then the index values are shifted but the data
-        is not realigned. That is, use freq if you would like to extend the
+        If `freq` is specified then the index values are shifted but the data
+        is not realigned. That is, use `freq` if you would like to extend the
        index when shifting and preserve the original data.

        Returns
        -------
        shifted : %(klass)s


If you are just returning a single element then you do not need to list the name of that variable in the returns, so here get rid of "shifted :" The returned object also needs a description indented on the subsequent line

TomAugspurger · 2018-03-26T17:53:25Z

pandas/core/generic.py


        Parameters
        ----------
        periods : int
-            Number of periods to move, can be positive or negative
+            Number of periods to shift; can be positive or negative.
        freq : DateOffset, timedelta, or time rule string, optional
            Increment to use from the tseries module or time rule (e.g. 'EOM').
            See Notes.


Since the entire Notes section is specific to freq, I'd prefer to remove it and move the content here.

TomAugspurger · 2018-03-26T17:54:41Z

pandas/core/generic.py


        Parameters
        ----------
        periods : int
-            Number of periods to move, can be positive or negative
+            Number of periods to shift; can be positive or negative.
        freq : DateOffset, timedelta, or time rule string, optional
            Increment to use from the tseries module or time rule (e.g. 'EOM').


Is "Increment" supposed to be a DateOffest? If so, let's use that term consistently.

Could you also link to the frequency aliases in timeseries.rst? timeseries.offset_aliases

Sorry, I didn't get it... Can you explain in other words?

TomAugspurger · 2018-03-26T17:56:03Z

pandas/core/generic.py

+        4   NaN  45.0  48.0
+
+        >>> import datetime
+        >>> names = ['João', 'Maria', 'Emanuel', 'Jussara', 'José']


The non-ASCII characters will cause encoding issues on Py2 without an encoding declaration. Probably best to stcik to ASCII till we're py3 only.

TomAugspurger · 2018-03-26T17:57:33Z

pandas/core/generic.py

+        2018-03-10 11:15:00  Emanuel
+        2018-03-15 11:15:00  Jussara
+        2018-03-20 11:15:00     José
+


Could you add a bit of an explanation between these? Especially explaining the freq argument, as people may not know what 'D' or 'min' refer to.

TomAugspurger · 2018-03-26T17:57:44Z

pandas/core/generic.py

+        2018-03-15 12:30:00  Jussara
+        2018-03-20 12:30:00     José
+
+        See Also


See Also goes before Examples

datapythonista · 2018-07-22T00:37:54Z

@YitzhakAndrade seems like you've got other changes besides yours in this PR.

Can you do the next in your branch:

git fetch upstream
git merge upstream/master
git reset --soft upstream/master
git commit -m "DOC: update the pandas.Series.shift docstring"

Thanks!

ZackStone

DOC: update the pandas.Series.shift docstring

datapythonista · 2018-07-22T14:48:23Z

Thanks for the update @YitzhakAndrade. Seems like you still have unrelated changes in this PR. Could you try what I said in my last comment?

For the next time, I think the problem can be related to using master for the changes, it's better to use a branch.

jreback · 2018-11-01T01:43:12Z

can you merge master and update

pep8speaks · 2018-11-03T06:28:19Z

Hello @YitzhakAndrade! Thanks for updating the PR.

There are no PEP8 issues in the file pandas/core/generic.py !

datapythonista

@jbrockmendel one more if you don't mind.

jbrockmendel · 2018-11-03T21:52:33Z

pandas/core/generic.py

-        PeriodIndex.shift : Shift values of PeriodIndex.
+            Number of periods to shift. Can be positive or negative.
+        freq : DateOffset, timedelta, or str, optional
+            Offset to use from the tseries module or time rule (e.g. 'EOM').


worth specifying tseries.offsets?

jbrockmendel · 2018-11-03T21:53:05Z

pandas/core/generic.py

+            If `freq` is specified then the index values are shifted but the
+            data is not realigned. That is, use `freq` if you would like to
+            extend the index when shifting and preserve the original data.
+        axis : {0 or ‘index’, 1 or ‘columns’, None}, default None


don't we usually avoid fancy quotes?

jbrockmendel · 2018-11-03T21:56:34Z

Couple of comments, agreement with points from Will and Tom. Generally looks nice.

…licate content with the freq param description

datapythonista · 2018-11-04T04:33:48Z

Thanks for the review @jbrockmendel, very useful. The docstring can surely be improved, with the raises section, and more examples, but I prefer to leave that for someone else in a separate PR, and just merge this as it is, as the PR is abandoned.

jreback · 2018-11-04T15:24:23Z

thanks @YitzhakAndrade

WillAyd requested changes Mar 26, 2018

View reviewed changes

jreback added Docs Datetime Datetime data dtype labels Mar 26, 2018

jreback reviewed Mar 26, 2018

View reviewed changes

WillAyd requested changes Mar 26, 2018

View reviewed changes

TomAugspurger reviewed Mar 26, 2018

View reviewed changes

ZackStone commented Jul 22, 2018

View reviewed changes

Original PR changes to shift docstring

8854ee3

datapythonista force-pushed the master branch from c6274d3 to 8854ee3 Compare November 3, 2018 06:28

Minor fixes and removing some examples

0e0970d

datapythonista approved these changes Nov 3, 2018

View reviewed changes

datapythonista mentioned this pull request Nov 3, 2018

Docstring shift #21039

Closed

4 tasks

jbrockmendel reviewed Nov 3, 2018

View reviewed changes

Addressing comments from code review, and removing notes, as it's dup…

f6afea0

…licate content with the freq param description

Fixing non-ascii quote

4d9c3c0

jreback added this to the 0.24.0 milestone Nov 4, 2018

jreback merged commit 471fb0e into pandas-dev:master Nov 4, 2018

JustinZhengBC pushed a commit to JustinZhengBC/pandas that referenced this pull request Nov 14, 2018

DOC: update the pandas.Series.shift docstring (pandas-dev#20472)

23fae32

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018

DOC: update the pandas.Series.shift docstring (pandas-dev#20472)

c8dbc8c

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

DOC: update the pandas.Series.shift docstring (pandas-dev#20472)

aa6f40f

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

DOC: update the pandas.Series.shift docstring (pandas-dev#20472)

c0b7766

DOC: update the pandas.Series.shift docstring #20472

DOC: update the pandas.Series.shift docstring #20472

Conversation

ZackStone commented Mar 23, 2018 • edited Loading

WillAyd Mar 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 26, 2018 • edited Loading

Codecov Report

ZackStone commented Mar 26, 2018

WillAyd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datapythonista commented Jul 22, 2018

ZackStone left a comment

Choose a reason for hiding this comment

datapythonista commented Jul 22, 2018

jreback commented Nov 1, 2018

pep8speaks commented Nov 3, 2018

datapythonista left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented Nov 3, 2018

datapythonista commented Nov 4, 2018

jreback commented Nov 4, 2018

ZackStone commented Mar 23, 2018 •

edited

Loading

WillAyd Mar 26, 2018 •

edited

Loading

codecov bot commented Mar 26, 2018 •

edited

Loading