DOC: update the DataFrame.reindex_like docstring by math-and-data · Pull Request #22775 · pandas-dev/pandas

math-and-data · 2018-09-19T23:05:50Z

The PEP8 style check passes: git diff upstream/master -u -- "*.py" | flake8 --diff
The html version looks good: python doc/make.py --single
The validation script passes: scripts/validate_docstrings.py
Errors due to the fact that this method .reindex_like()can use the same input parameters as .reindex() (I avoided copy/paste and referred to the other method instead)

        Errors in parameters section
                Parameter "other" has no description
                Parameter "method" has no description
                Parameter "copy" has no description

pep8speaks · 2018-09-19T23:05:57Z

Hello @math-and-data! Thanks for updating the PR.

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

Comment last updated on November 04, 2018 at 21:16 Hours UTC

jorisvandenbossche

Thanks for working on the docstrings!

Added some first comments.

Can you make sure to run the validation script on all docstrings you edited? (not only for reindex_like)

pandas/core/generic.py

jorisvandenbossche · 2018-09-20T14:21:24Z

pandas/core/generic.py

        -------
-        reindexed : same as input
+        Object
+            same object type as input, but with changed indices on each axis


You can leave out the "Object" line, and directly put the explanation on that line.

And can you use Capital first character and end point punctuation?

pandas/core/generic.py

codecov · 2018-09-20T16:19:54Z

Codecov Report

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

@@            Coverage Diff             @@
##           master   #22775      +/-   ##
==========================================
+ Coverage   92.29%   92.29%   +<.01%     
==========================================
  Files         161      161              
  Lines       51500    51501       +1     
==========================================
+ Hits        47531    47534       +3     
+ Misses       3969     3967       -2

Flag	Coverage Δ
#multiple	`90.69% <100%> (ø)`	⬆️
#single	`42.42% <66.66%> (+0.11%)`	⬆️

Impacted Files	Coverage Δ
pandas/core/frame.py	`97.02% <ø> (ø)`	⬆️
pandas/core/generic.py	`96.84% <100%> (-0.01%)`	⬇️
pandas/io/parsers.py	`95.36% <0%> (-0.19%)`	⬇️
pandas/io/pytables.py	`92.3% <0%> (-0.05%)`	⬇️
pandas/core/base.py	`97.6% <0%> (-0.01%)`	⬇️
pandas/core/indexes/multi.py	`95.49% <0%> (-0.01%)`	⬇️
pandas/core/reshape/pivot.py	`96.55% <0%> (ø)`	⬆️
pandas/io/json/normalize.py	`96.87% <0%> (ø)`	⬆️
pandas/io/packers.py	`88.08% <0%> (ø)`	⬆️
... and 41 more

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 3e01c38...7df1f79. Read the comment docs.

math-and-data · 2018-09-23T21:59:34Z

The PEP8 style check passes: git diff upstream/master -u -- "*.py" | flake8 --diff
The html version looks good: python doc/make.py --single
The validation script passes: scripts/validate_docstrings.py
.reindex_like: formatting of bulleted list leads to missing dot

Errors found:
        Errors in parameters section
                Parameter "method" description should finish with "."

.reindex: formatting of bulleted list leads to missing dot

Errors found:
        Errors in parameters section
                Parameters {index, columns} not documented
                Unknown parameters {index, columns}
                Parameter "index, columns" description should finish with "."
                Parameter "method" description should finish with "."

.set_index, .reset_index:

Docstring for "pandas.DataFrame.reset_index" correct. :)

datapythonista

Looks good, added some comments about formatting

pandas/core/frame.py

datapythonista · 2018-11-03T07:21:08Z

pandas/core/frame.py

+        keys : str or list of str or array
+            Column label or list of column labels / arrays that will
+            form the new index.
        drop : boolean, default True


Can you replace all the boolean by bool? I think the validation script should warn about it now.

datapythonista · 2018-11-03T07:21:48Z

pandas/core/frame.py

+        --------
+        DataFrame.set_index : opposite of reset_index
+        DataFrame.reindex : change to new indices or expand indices
+        DataFrame.reindex_like : change to same indices as other DataFrame


Can you capitalize the first letter of each description, and add a period at the end please

datapythonista · 2018-11-03T07:23:06Z

pandas/core/generic.py


        Parameters
        ----------
        other : Object


I guess other can only be Series and DataFrame? Or can we be more specific?

datapythonista · 2018-11-03T07:23:39Z

pandas/core/generic.py

+            * backfill / bfill: use next valid observation to fill gap
+            * nearest: use nearest valid observations to fill gap
+
        copy : boolean, default True


again, bool instead of boolean

datapythonista · 2018-11-03T07:24:02Z

pandas/core/generic.py

-        method : string or None
+            Its row and column indices are used to define the new indices
+            of this object.
+        method : {None, 'backfill'/'bfill', 'pad'/'ffill', 'nearest'}, optional


I'd remove the None as it's already clear with the optional

datapythonista · 2018-11-03T07:24:23Z

pandas/core/generic.py

        -----
-        Like calling s.reindex(index=other.index, columns=other.columns,
-                               method=...)
+        Like calling `.reindex(index=other.index, columns=other.columns, ...)`


Can you finish with a period.

datapythonista · 2018-11-03T07:24:53Z

pandas/core/generic.py

+        --------
+        DataFrame.set_index : set row labels
+        DataFrame.reset_index : remove row labels or move them to new columns
+        DataFrame.reindex_like : change to same indices as other DataFrame


Again, capitalize and finish with periods.

datapythonista · 2018-11-03T07:25:03Z

pandas/core/generic.py

+        DataFrame.set_index : set row labels
+        DataFrame.reset_index : remove row labels or move them to new columns
+        DataFrame.reindex : change to new indices or expand indices
+        DataFrame.reindex_like : change to same indices as other DataFrame


Can you capitalize and finish with period these descriptions.

datapythonista

Great changes, few more minor things, and I think we're ready to merge. Thanks!

pandas/core/generic.py

datapythonista · 2018-11-04T19:22:39Z

pandas/core/generic.py

        Returns
        -------
-        reindexed : same as input
+        Same object type as input, but with changed indices on each axis.


Can you add a line before that with Series or DataFrame. This description should be indented in the next line.

pandas/core/generic.py

datapythonista · 2018-11-04T19:24:41Z

pandas/core/generic.py

@@ -3714,27 +3769,27 @@ def reindex(self, *args, **kwargs):
            New labels / index to conform to. Preferably an Index object to


Can you move the (should be specified using keywords) to the description. As a normal sentence, doesn't need to be in brackets.

datapythonista · 2018-11-04T19:26:13Z

pandas/core/generic.py

+        DataFrame.set_index : set row labels
+        DataFrame.reset_index : remove row labels or move them to new columns
+        DataFrame.reindex : change to new indices or expand indices
+        DataFrame.reindex_like : change to same indices as other DataFrame


Can you capitalize and finish with period these descriptions.

datapythonista · 2018-11-04T19:26:48Z

pandas/core/generic.py


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


Suggested change

reindexed : %(klass)s

%(klass)s

And add a short description of what is being returned in the next line.

Co-Authored-By: math-and-data <math-and-data@users.noreply.github.com>

…data/pandas into docstring_reindex_like

datapythonista

lgtm, really nice work.

jorisvandenbossche

@math-and-data Nice enhancements! I added a few more comments, specifically on the wording of the summary lines.

Also, can you see if you can remove set_index and reindex from

pandas/ci/code_checks.sh

Line 134 in 64c3491

    
           -k"-axes -combine -itertuples -join -nunique -pivot_table -quantile -query -reindex -reindex_axis -replace -round -set_index -stack -to_stata"

?

(so the doctests are run on our CI to make sure the examples are correct)

jorisvandenbossche · 2018-11-05T09:29:31Z

pandas/core/frame.py

-        inplace : boolean, default False
-            Modify the DataFrame in place (do not create a new object)
-        verify_integrity : boolean, default False
+        keys : str or list of str or array


To be strict, the keys can be something else as a string ... (column names can also be integers, timestamps, ..)
That also the reason that there was 'label' before.

jorisvandenbossche · 2018-11-05T09:30:26Z

pandas/core/frame.py

-        verify_integrity : boolean, default False
+        keys : str or list of str or array
+            Column label or list of column labels / arrays that will
+            form the new index.


Since this 'arrays' option is really something completely different (it are the actual index values passed as an array, not referring to one of the existing columns), I would put that in a separate sentence.

jorisvandenbossche · 2018-11-05T09:35:15Z

pandas/core/frame.py

    def set_index(self, keys, drop=True, append=False, inplace=False,
                  verify_integrity=False):
        """
+        An index is created with existing columns.


I find this a bit confusing as the summary line (because it seems to suggest that an index is returned, since that is created).
I found the previous "Set the DataFrame index using existing columns" a bit clearer. @datapythonista thoughts?

jorisvandenbossche · 2018-11-05T09:39:56Z

pandas/core/frame.py

    def reset_index(self, level=None, drop=False, inplace=False, col_level=0,
                    col_fill=''):
        """
+        An existing index is modified.


I think we can also do better here for the summary method, as just from this line "An existing index is modified.", a user won't get much clue of what the method is doing.

But of course, given that the method is doing different things, it might be difficult to have something that is still generally true but less vague as the above ...

Trying to think of better descriptions:

Remove index (level) (but that is also a bit short / cryptic)
Reset index to default index or remove index level

(will think further on it)

jorisvandenbossche · 2018-11-05T09:40:36Z

pandas/core/generic.py

        -----
-        Like calling s.reindex(index=other.index, columns=other.columns,
-                               method=...)
+        Like calling ``.reindex(index=other.index,columns=other.columns,...)``.


Suggested change

Like calling ``.reindex(index=other.index,columns=other.columns,...)``.

Like calling ``.reindex(index=other.index, columns=other.columns,...)``.

datapythonista · 2018-11-20T14:39:14Z

@math-and-data do you have time to make the changes asked in the last review?

datapythonista · 2018-11-21T18:36:47Z

@jorisvandenbossche addressed the comments from your code review. Let me know if there is anything else that needs to be changed.

…_like

WillAyd · 2018-11-26T03:41:32Z

pandas/core/frame.py

+
+        Reset the index of the DataFrame, and use the default one instead.
+        If the DataFrame has a MultiIndex, this method can remove one or more
+        of them.


I think levels would be clearer than of them

WillAyd · 2018-11-26T03:42:32Z

pandas/core/generic.py

+        Return an object with matching indices as other object.
+
+        Conform the object to the same index on all axes. Optional
+        filling logic, placing NA/NaN in locations having no value


Just say NaN

WillAyd · 2018-11-26T03:43:55Z

pandas/core/generic.py

        Returns
        -------
-        reindexed : same as input
+        Object with input data type, but with changed indices on each axis.


Replace everything before the comma with Same type as caller

WillAyd · 2018-11-26T03:44:34Z

pandas/core/generic.py

+
+        Examples
+        --------
+        >>> df_weather_station_1 = pd.DataFrame([[24.3, 75.7, 'high'],


I don't think the _weather_station bit adds any value; would be OK to just use df1, df2, etc...

datapythonista · 2018-11-26T16:40:40Z

@WillAyd made the changes, can you take a look?

WillAyd · 2018-11-26T23:12:25Z

Thanks @math-and-data and @datapythonista !

math-and-data added 2 commits September 20, 2018 00:53

DOC: Improve the docstring of DataFrame.reindex_like()

bab3a2b

DOC: Improve the docstring of DataFrame.reindex_like()

cd3ec62

DOC: Improve the docstring of DataFrame.reindex_like()

1ee2751

math-and-data changed the title ~~Docstring reindex_like~~ DOC: update the DataFrame.reindex_like docstring Sep 19, 2018

jorisvandenbossche added the Docs label Sep 20, 2018

jorisvandenbossche reviewed Sep 20, 2018

View reviewed changes

math-and-data added 4 commits September 23, 2018 22:51

docstring for .reindex_like

2cbf5dd

docstring .reindex_like: input params details

92c1d2f

docstring .reset_index, .set_index, .reindex, .reindex_like

5bcee6d

docstring set_index: PEP8 style check

2729193

Merge branch 'master' into docstring_reindex_like

4851b91

datapythonista reviewed Nov 3, 2018

View reviewed changes

math-and-data added 2 commits November 4, 2018 19:39

reindex: formatting

be1a774

reindex: formatting of docstring

7072e4a

datapythonista reviewed Nov 4, 2018

View reviewed changes

datapythonista and others added 6 commits November 4, 2018 21:09

Update pandas/core/generic.py

88e6e37

Co-Authored-By: math-and-data <math-and-data@users.noreply.github.com>

Update pandas/core/generic.py

38c5c94

Co-Authored-By: math-and-data <math-and-data@users.noreply.github.com>

reindex_like print example df

4518791

Merge branch 'docstring_reindex_like' of https://github.com/math-and-…

9d58d4e

…data/pandas into docstring_reindex_like

reindex_like params

363e8c0

reindex docstring: pep8 line length

4d59844

datapythonista approved these changes Nov 5, 2018

View reviewed changes

jorisvandenbossche reviewed Nov 5, 2018

View reviewed changes

Addressing code review comments

6bf4977

Merge remote-tracking branch 'upstream/master' into docstring_reindex…

2a838f2

…_like

WillAyd requested changes Nov 26, 2018

View reviewed changes

Addressing review comments

7df1f79

WillAyd approved these changes Nov 26, 2018

View reviewed changes

WillAyd merged commit ede0dae into pandas-dev:master Nov 26, 2018

jorisvandenbossche mentioned this pull request Jan 10, 2019

DEPR/API: disallow lists within list for set_index #24697

Closed

5 tasks

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

DOC: update the DataFrame.reindex_like docstring (pandas-dev#22775)

69e2f1d

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

DOC: update the DataFrame.reindex_like docstring (pandas-dev#22775)

92799a9

		@@ -3714,27 +3769,27 @@ def reindex(self, args, *kwargs):
		New labels / index to conform to. Preferably an Index object to

	Like calling ``.reindex(index=other.index,columns=other.columns,...)``.
	Like calling ``.reindex(index=other.index, columns=other.columns,...)``.

Uh oh!

Conversation

math-and-data commented Sep 19, 2018

Uh oh!

pep8speaks commented Sep 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comment last updated on November 04, 2018 at 21:16 Hours UTC

Uh oh!

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Sep 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

math-and-data commented Sep 23, 2018

Uh oh!

datapythonista left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datapythonista left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datapythonista left a comment

Choose a reason for hiding this comment

Uh oh!

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datapythonista commented Nov 20, 2018

Uh oh!

datapythonista commented Nov 21, 2018

Uh oh!

Choose a reason for hiding this comment

pep8speaks commented Sep 19, 2018 •

edited

Loading

codecov bot commented Sep 20, 2018 •

edited

Loading