Various docstring fixes #28744

ChiefMilesEdgeworth · 2019-10-02T04:34:18Z

xref Fix PR02 issues in docstrings #27976
tests added / passed - Didn't run all tests, only those concerning the issue (./scripts/validate_docstrings.py pandas.Series.drop --errors=PR02)
passes black pandas - Only docstrings were edited, so there were no major changes
passes git diff upstream/master -u -- "*.py" | flake8 --diff
~~whatsnew entry~~ - Not needed, only docs changed
General fixes to the documentation in different places. I discovered that the root of many of the conflicts lies in decorators. We might need to re-evaluate how we're getting the signature of functions in the validation script.

First time submitting a PR! Let me know if I need to do anything, or if there's anything I should do differently next time :)

Simple error fixes for idxmin and idxmax regarding their docstrings

Fixed errors on swaplevel, rename, and drop methods

Fixed methods mean, std, var, and corr in ewm.py. Fixed methods sum, apply, max, std, var, and quantile in rolling.py

Fixed functions table, scatter_matrix, radviz, bootstrap_plot, parallel_coordinates, lag_plot, and autocorrelation_plot. Noticed that functions with the @deprecate_kwarg decorator tend to not behave correctly with the tests.

Fixed the docstring for transpose, eval, swaplevel, and melt in frame.py

Just missed the one line.

Caught small whitespace issues.

Fixed errors shown by automated tests (PR04 and GL03)

angelaambroz · 2019-10-02T15:22:50Z

Oh, uh - I guess this PR re-does what mine did - plus additional stuff. No sense merging both, so looks like this one might be a superset of mine. So much for my first PR on pandas... 😞

ChiefMilesEdgeworth · 2019-10-02T15:26:08Z

Sorry, I didn't see yours until after I had committed the overlapping changes. There's still more than a few that look like simple formatting errors if you want to go after those. I'm going to start looking into the issues with decorators. Don't feel disheartened, my work wasn't any more valuable than yours.

…

On Wed, Oct 2, 2019 at 11:23 AM Angela Ambroz ***@***.***> wrote: Oh, uh - I guess this PR re-does what mine did <#28742> - plus additional stuff. No sense merging both, so looks like this one might be a superset of mine. So much for my first PR on pandas... 😞 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#28744?email_source=notifications&email_token=ACT76UTP2DHUISSB4XZSH7LQMS4HVA5CNFSM4I4RUVZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAFEO3I#issuecomment-537544557>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACT76US5D7W4EIHUZ5TPTU3QMS4HVANCNFSM4I4RUVZA> .

-- Sincerely, *Nathan Abel* Kettering University - Computer Science 989-780-3050

Fixed issues shown by automated tests with error PR03.

angelaambroz · 2019-10-02T15:33:17Z

Thanks for the follow-up. Yeah, it's hard to find unclaimed work on these projects. (I've been looking here and on scikit-learn.) I'll take a look at the remaining stuff. 👍

ChiefMilesEdgeworth · 2019-10-02T16:43:23Z

I'm not sure why the Travis CI build failed. Could I get a pointer on how to fix it, or was the code not at fault?

datapythonista

Thanks a lot for working on all those. Added some comments, all the args and kwargs should be merged, unless they really require different descriptions.

datapythonista · 2019-10-02T23:07:19Z

pandas/core/frame.py

+        *args
+            Additional arguments have no effect but might be accepted for
+            compatibility with numpy.
+        **kwargs


The script is wrong in this case, please revert

datapythonista · 2019-10-02T23:07:29Z

pandas/core/frame.py

@@ -3237,7 +3237,7 @@ def eval(self, expr, inplace=False, **kwargs):
            If the expression contains an assignment, whether to perform the
            operation inplace and mutate the existing DataFrame. Otherwise,
            a new DataFrame is returned.
-        kwargs : dict
+        **kwargs : dict


please remove the type

datapythonista · 2019-10-02T23:08:09Z

pandas/core/series.py

+        *args
+            Additional arguments have no effect but might be accepted
+            for compatability with NumPy
+        **kwargs


merge those please

datapythonista · 2019-10-02T23:09:24Z

pandas/core/frame.py

@@ -5205,8 +5205,10 @@ def swaplevel(self, i=-2, j=-1, axis=0):

        Parameters
        ----------
-        i, j : int, str (can be mixed)
-            Level of index to be swapped. Can pass level name as string.
+        i : int, str (can be mixed)


int or str, not sure what the can be mixed means here, but shouldn't be in the type (can be explained in the description)

datapythonista · 2019-10-02T23:09:45Z

pandas/core/series.py

+        *args
+            Additional arguments have no effect but might be accepted
+            for compatibility with NumPy.
+        **kwargs


please merge args and kwargs

datapythonista · 2019-10-02T23:11:11Z

pandas/core/window/ewm.py

        **kwargs
-           Keyword arguments to be passed into func.
+            Keyword arguments to be passed into func.


merge args and kwargs

datapythonista · 2019-10-02T23:11:34Z

pandas/core/window/ewm.py

+            output will be a MultiIndex DataFrame in the case of DataFrame
+            inputs. In the case of missing elements, only complete pairwise
+            observations will be used.
+        **kwargs : dict of {str : Any}


remove the type

datapythonista · 2019-10-02T23:11:49Z

pandas/core/window/ewm.py

        """
+


remove this blank line, this is probably breaking the CI

datapythonista · 2019-10-02T23:12:36Z

pandas/core/window/rolling.py

-    *args, **kwargs
+    args : tuple of Any
+        Arguments and keyword arguments to be passed into func.
+    kwargs : dict of {str : any}


remove the types, aren't those *args instead of args?

datapythonista · 2019-10-02T23:13:08Z

pandas/plotting/_misc.py

@@ -398,7 +398,7 @@ def lag_plot(series, lag=1, ax=None, **kwds):
    series : Time series
    lag : lag of the scatter plot, default 1
    ax : Matplotlib axis object, optional
-    kwds : Matplotlib scatter method keyword arguments, optional
+    **kwds : Matplotlib scatter method keyword arguments, optional


move this to the description, no type for star params

Put args and kwargs back on the same line, since that's how the documentation should be. The validation script needs to be changed to catch this.

…rious_docstring_fixes

pep8speaks · 2019-10-03T00:42:03Z

Hello @Nabel0721! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-11-08 03:21:11 UTC

Should've checked it before commiting. Two very small fixes

ChiefMilesEdgeworth · 2019-10-03T00:45:04Z

I'm going to start looking into fixing the validation script so it catches more of these properly, since the correct format for the docs is to have args and kwargs in the same line.

datapythonista · 2019-10-03T01:36:32Z

Can't look now, but I think there is an issue for it.

ChiefMilesEdgeworth · 2019-10-03T01:55:26Z

I think this (#20298) is it. If you'd like I can comment on there with this issue. There's another part I'd like to fix as well, which I commented about in #27976.

datapythonista · 2019-10-03T02:04:07Z

Sure, will be great to see both fixed.

ChiefMilesEdgeworth · 2019-10-04T04:39:45Z

This is ready to be merged, I have the changes to the validation script in a separate pull request, since it addresses a different problem.

datapythonista

Some comments, other than that looks good

datapythonista · 2019-10-04T10:39:06Z

pandas/core/window/ewm.py

+        *args
+            Arguments to be passed into func.
+        **kwargs
+            Keyword arguments to be passed into func.


Better revert this please.

Any reason to not resolve this and mark as resolved? I think your proposed change is incorrect, the original is more accurate.

No idea why I missed these. I'll get them right away.

Actually, it looks like I just forgot to mark this as resolved. It should be fixed in the last commit.

I think it's not reverted.

Can you revert please

For some reason it's showing up as reverted on my machine, but not here. I'll just do the edits on Github.

pandas/core/series.py

pandas/plotting/_misc.py

Merged *args and **kwargs, removed a duplicate documentation, and removed the type for **kwargs.

ChiefMilesEdgeworth · 2019-10-04T16:12:05Z

Issues are fixed. Thanks for working through this with me!

datapythonista

couple of small things, but looks good besides that

datapythonista · 2019-10-07T02:18:07Z

pandas/core/window/ewm.py

+        *args
+            Arguments to be passed into func.
+        **kwargs
+            Keyword arguments to be passed into func.


Any reason to not resolve this and mark as resolved? I think your proposed change is incorrect, the original is more accurate.

datapythonista · 2019-10-07T02:19:08Z

pandas/core/series.py

-            Level of index to be swapped. Can pass level name as string.
+        i : int, str
+            Level of first index to be swapped. Can pass level name as string.
+        j : int, str


int or str in both cases

and you can also merge them back

datapythonista · 2019-10-07T02:20:12Z

pandas/core/frame.py

+        i : int or str
+            Level of first index to be swapped. Can pass level name as string.
+        j : int or str
+            Level of second index to be swapped. Can pass level name as string.


I'd merge those, I think the validation script should accept this, as proposed in the PR where you update it.

Right now the proposed script does not validate any variables on the same line, just *args and **kwargs. I left it that way in case we decide we don't want people to just throw a bunch of variables on the same line. If you think that'd be fine, then I can rewrite the script update to allow for that, and change these here.

I think the PR allowing having those together was merged, right? Can you have i, j instead please

Updating to most recent version

datapythonista · 2019-10-14T03:11:36Z

pandas/core/frame.py

+        i : int or str
+            Level of first index to be swapped. Can pass level name as string.
+        j : int or str
+            Level of second index to be swapped. Can pass level name as string.


I think the PR allowing having those together was merged, right? Can you have i, j instead please

datapythonista · 2019-10-14T03:11:51Z

pandas/core/series.py

-            Level of index to be swapped. Can pass level name as string.
+        i : int, str
+            Level of first index to be swapped. Can pass level name as string.
+        j : int, str


datapythonista · 2019-10-14T03:12:27Z

pandas/core/window/ewm.py

+        *args
+            Arguments to be passed into func.
+        **kwargs
+            Keyword arguments to be passed into func.


I think it's not reverted.

datapythonista · 2019-10-14T03:13:11Z

pandas/plotting/_misc.py

@@ -398,7 +398,8 @@ def lag_plot(series, lag=1, ax=None, **kwds):
    series : Time series
    lag : lag of the scatter plot, default 1
    ax : Matplotlib axis object, optional
-    kwds : Matplotlib scatter method keyword arguments, optional
+    **kwds
+        Matplotlib scatter method keyword arguments, optional


Can you remove the optional, and end with period.

Moved a couple of index parameters onto the same line, and redid one parameter description

…_fixes

datapythonista

Nice fixes, few comments

pandas/core/frame.py

datapythonista · 2019-10-16T00:27:30Z

pandas/core/frame.py

-        i, j : int, str (can be mixed)
-            Level of index to be swapped. Can pass level name as string.
+        i, j: int or str
+            Levels indecies to be swapped. Can pass level name as string.


typo, indices

datapythonista · 2019-10-16T00:28:03Z

pandas/core/series.py

-        i, j : int, str (can be mixed)
-            Level of index to be swapped. Can pass level name as string.
+        i, j : int, str
+            Level of the indecies to be swapped. Can pass level name as string.


datapythonista · 2019-10-16T00:29:53Z

pandas/core/window/ewm.py

+        *args
+            Arguments to be passed into func.
+        **kwargs
+            Keyword arguments to be passed into func.


Can you revert please

Added a space between a parameter and the colon.

Fixed typos on documentation

datapythonista

Almost there, couple of comments.

pandas/core/frame.py

pandas/core/window/ewm.py

datapythonista · 2019-10-24T04:46:32Z

@Nabel0721 do you have time to commit the last changes?

And next time it'd be better if you leave checked the checkbox to let maintainers commit to your branch, and we could do it ourselves.

Thanks!

ChiefMilesEdgeworth · 2019-10-24T11:37:39Z

I got really busy unexpectedly, so I don't think I'll have time for a couple more weeks. I do think I have the "allow edits from maintainers" checkbox on, so I'm not sure why it's showing up as off on your end. If you are able to make edits though, feel free.

WillAyd · 2019-11-08T00:05:03Z

Nice PR but sounds like it is stale (or at least will be for the foreseeable future).

@Nabel0721 ping if you'd like to pick this back up and can reopen

ChiefMilesEdgeworth · 2019-11-08T03:14:08Z

Yeah, my apologies. I’m not going to be able to get to this for a while. I’ll be sure to ping if there’s an update.

On Thu, Nov 7, 2019 at 7:05 PM William Ayd ***@***.***> wrote: Closed #28744 <#28744>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#28744?email_source=notifications&email_token=ACT76UWBBBVR7LTRSKCNXVDQSSUN5A5CNFSM4I4RUVZKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOUXDNNGY#event-2781271707>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACT76UTH7RIBWJPLBE77W6TQSSUN5ANCNFSM4I4RUVZA> .

-- Sincerely, *Nathan Abel* Kettering University - Computer Science 989-780-3050

datapythonista · 2019-11-08T03:18:16Z

@Nabel0721 the only thing pending is literally clicking on the Commit suggestions buttons in this comment: #28744 (review)

We can't do it ourselves (github issue, or you didn't allow maintainers to edit this PR). If you can spend one minute doing that, we can merge this changes.

I didn't know I could do this Co-Authored-By: Marc Garcia <garcia.marc@gmail.com>

Small change to wording Co-Authored-By: Marc Garcia <garcia.marc@gmail.com>

ChiefMilesEdgeworth · 2019-11-08T03:21:30Z

Sorry, I didn't know I could do that. I would have done it a while ago if I had known.

ChiefMilesEdgeworth · 2019-11-08T03:23:26Z

It must be a github issue that you can't do edits though. I checked again and the allow edits checkbox is still checked. Sorry for how long this has taken to finish up, but thank you for being patient.

datapythonista

lgtm, thanks a lot @Nabel0721 for fixing all this. Hope to see you back when you have more free time!

ChiefMilesEdgeworth · 2019-11-08T03:27:33Z

For sure. It's always nice to get to work on python code.

WillAyd · 2019-11-08T16:41:18Z

Great thanks @Nabel0721 and @datapythonista

ChiefMilesEdgeworth added 9 commits October 1, 2019 18:04

DOC: Fixed PR02 docstring error in pandas Series idxmin and idxmax

a3e4315

Simple error fixes for idxmin and idxmax regarding their docstrings

DOC: Fixed other PR02 docstring errors in pandas Series class

d1f1ba0

Fixed errors on swaplevel, rename, and drop methods

DOC: Fixed other PR02 docstring errors in pandas rolling and ewm files

7fc0e35

Fixed methods mean, std, var, and corr in ewm.py. Fixed methods sum, apply, max, std, var, and quantile in rolling.py

DOC: Fixed other PR02 docstring errors in pandas frame.py file

51bdc5d

Fixed the docstring for transpose, eval, swaplevel, and melt in frame.py

Ran Black

ffe849f

Just missed the one line.

Flake8 Changes

f3b9540

Caught small whitespace issues.

Merge branch 'master' into Various_docstring_fixes

d5c0394

Test fixes

bedf748

Fixed errors shown by automated tests (PR04 and GL03)

DOC: Test fixes part 2

4d7a386

Fixed issues shown by automated tests with error PR03.

Merge branch 'master' into Various_docstring_fixes

2658caf

datapythonista reviewed Oct 2, 2019

View reviewed changes

datapythonista added the Docs label Oct 2, 2019

ChiefMilesEdgeworth added 2 commits October 2, 2019 20:41

DOC: Update args and kwargs to same line, other small changes

532b53c

Put args and kwargs back on the same line, since that's how the documentation should be. The validation script needs to be changed to catch this.

Merge remote-tracking branch 'origin/Various_docstring_fixes' into Va…

2f48f5a

…rious_docstring_fixes

DOC: Missed whitespace

2df56a2

Should've checked it before commiting. Two very small fixes

datapythonista reviewed Oct 4, 2019

View reviewed changes

DOC: Fixing overlooked issues

ded2c0c

Merged *args and **kwargs, removed a duplicate documentation, and removed the type for **kwargs.

datapythonista reviewed Oct 7, 2019

View reviewed changes

ChiefMilesEdgeworth added 2 commits October 11, 2019 18:14

Merge remote-tracking branch 'upstream/master'

d687909

Updating to most recent version

Merge branch 'master' into Various_docstring_fixes

ec3b412

datapythonista reviewed Oct 14, 2019

View reviewed changes

ChiefMilesEdgeworth added 3 commits October 15, 2019 16:41

DOC: Revert back to multiple parameters on one line

494b997

Moved a couple of index parameters onto the same line, and redid one parameter description

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

2c723a7

…_fixes

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

0507336

…_fixes

datapythonista requested changes Oct 16, 2019

View reviewed changes

ChiefMilesEdgeworth added 2 commits October 15, 2019 22:10

DOC: Spacing fixed

f631cb4

Added a space between a parameter and the colon.

DOC: Fixed typos

16fd88a

Fixed typos on documentation

datapythonista reviewed Oct 16, 2019

View reviewed changes

pandas/core/frame.py Outdated Show resolved Hide resolved

pandas/core/window/ewm.py Outdated Show resolved Hide resolved

WillAyd closed this Nov 8, 2019

datapythonista reopened this Nov 8, 2019

ChiefMilesEdgeworth and others added 2 commits November 7, 2019 22:20

Update pandas/core/frame.py

a5cbc8c

I didn't know I could do this Co-Authored-By: Marc Garcia <garcia.marc@gmail.com>

Update pandas/core/window/ewm.py

2a105c2

Small change to wording Co-Authored-By: Marc Garcia <garcia.marc@gmail.com>

datapythonista approved these changes Nov 8, 2019

View reviewed changes

WillAyd approved these changes Nov 8, 2019

View reviewed changes

WillAyd added this to the 1.0 milestone Nov 8, 2019

WillAyd merged commit 69c61c5 into pandas-dev:master Nov 8, 2019

Reksbril pushed a commit to Reksbril/pandas that referenced this pull request Nov 18, 2019

Various docstring fixes (pandas-dev#28744)

a115335

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

Various docstring fixes (pandas-dev#28744)

54859f8

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

Various docstring fixes (pandas-dev#28744)

6aa3918

Various docstring fixes #28744

Various docstring fixes #28744

Conversation

ChiefMilesEdgeworth commented Oct 2, 2019 • edited by datapythonista Loading

angelaambroz commented Oct 2, 2019

ChiefMilesEdgeworth commented Oct 2, 2019 via email

angelaambroz commented Oct 2, 2019

ChiefMilesEdgeworth commented Oct 2, 2019

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

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

pep8speaks commented Oct 3, 2019 • edited Loading

Comment last updated at 2019-11-08 03:21:11 UTC

ChiefMilesEdgeworth commented Oct 3, 2019

datapythonista commented Oct 3, 2019

ChiefMilesEdgeworth commented Oct 3, 2019

datapythonista commented Oct 3, 2019

ChiefMilesEdgeworth commented Oct 4, 2019

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

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

ChiefMilesEdgeworth commented Oct 4, 2019

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

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

datapythonista left a comment

Choose a reason for hiding this comment

datapythonista commented Oct 24, 2019

ChiefMilesEdgeworth commented Oct 24, 2019

WillAyd commented Nov 8, 2019

ChiefMilesEdgeworth commented Nov 8, 2019 via email

datapythonista commented Nov 8, 2019

ChiefMilesEdgeworth commented Nov 8, 2019

ChiefMilesEdgeworth commented Nov 8, 2019

datapythonista left a comment

Choose a reason for hiding this comment

ChiefMilesEdgeworth commented Nov 8, 2019

WillAyd commented Nov 8, 2019

ChiefMilesEdgeworth commented Oct 2, 2019 •

edited by datapythonista

Loading

pep8speaks commented Oct 3, 2019 •

edited

Loading