-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DOC: Fix docstring of window.Expanding/Rolling.apply and document *args and **kwargs #24184
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
DOC: Fix docstring of window.Expanding/Rolling.apply and document *args and **kwargs #24184
Conversation
…and pandas.core.window.Rolling.apply The following 'errors' were fixed: - Summary does not start with a capital letter - Unknown parameters {\*args and \*\*kwargs are passed to the function} - Parameter "func" description should finish with "." - Parameter "raw" description should start with a capital letter - Parameter "\*args and \*\*kwargs are passed to the function" has no type - Parameter "\*args and \*\*kwargs are passed to the function" description should finish with "." - No Returns section found The following 'warning' was addressed: - See Also section not found
Hello @LJArendse! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on January 07, 2019 at 11:30 Hours UTC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides the comment, you have PEP8 issues in your code (tabs), it's probably worth using a validator in your editor.
Also, if instead of describing what you did, you can just copy the output of the validation script, that's more useful to review.
Codecov Report
@@ Coverage Diff @@
## master #24184 +/- ##
===========================================
- Coverage 92.21% 43.03% -49.18%
===========================================
Files 162 162
Lines 51723 51723
===========================================
- Hits 47695 22261 -25434
- Misses 4028 29462 +25434
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #24184 +/- ##
==========================================
- Coverage 92.38% 92.37% -0.02%
==========================================
Files 166 166
Lines 52478 52382 -96
==========================================
- Hits 48483 48389 -94
+ Misses 3995 3993 -2
Continue to review full report at Codecov.
|
And if you can please provide the output of the validation script in the description, as mentioned before, that would be helpful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
For next time, it's useful if you also copy the rendered docstring in the description, not only the list of errors. As the docstring is splitted in different variables, and some values are not explicit (like %(name)s
, it's some times easier to understand the rendered docstring.
@datapythonista
|
can you run I think the problem it's indeed removing the indentation |
…y'] docstring, and Remove @appender(_doc_template) decorator for rolling apply and expanding apply
Fixed Summary contains heading whitespace for pandas.core.window.EWM.mean pandas.core.window.EWM.mean
pandas.core.window.Expanding.apply
pandas.core.window.Rolling.apply
|
@datapythonista looks like the fixes have passed the checks :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @LJArendse
Yea I think that should work more universally. If you can try and check tenderer output would be preferable if it works
…Sent from my iPhone
On Dec 11, 2018, at 12:34 PM, LJ ***@***.***> wrote:
@LJArendse commented on this pull request.
In pandas/core/window.py:
>
- \*args and \*\*kwargs are passed to the function""")
+ Returns
Hey @WillAyd,
I did it in the _shared_docs['apply'] because the @appender(_doc_template) decorator on the window.Expanding/Rolling.apply methods is what threw the original validation error No Returns section found. I was also not sure whether my changes, for window.Expanding/Rolling.apply, are relevant for the other functions wrapped with @appender(_doc_template).
I see your point though, I am happy to update the _doc_template
From:
_doc_template = """
Returns
-------
same type as input
See Also
--------
Series.%(name)s
DataFrame.%(name)s
"""
To:
_doc_template = """
Returns
-------
Series or DataFrame
Return type is determined by the caller.
See Also
--------
Series.%(name)s : Series %(name)s.
DataFrame.%(name)s : DataFrame %(name)s.
"""
. . . if you agree with my modification to _doc_template ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@WillAyd I agree, updated the _doc_template as discussed. |
what is the status of this? anything pending? @LJArendse @WillAyd |
Hi @datapythonista Additional to this, from what I understand, I will be investigating the best way to add I will only be able to work on this after the 2nd of Jan. |
…indow-rolling-apply-and-expanding-apply
In the progress of adding the *args and **kwargs parameters to the following:
|
lgtm, let us know when you're done. And feel free to leave some of the docstring fixes to other PRs, in general it's better to have smaller PRs, than a large one. @WillAyd if you can take a look. |
Done. I am busy uploading the before and after validation output for each updated docstring. |
Expanding.applyBefore:
After:
|
Rolling.applyBefore:
After:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks @LJArendse
@WillAyd merge when you're happy
The following functions had I can upload the full before and after validation output if you want (I did not want to upload for each as it will make this thread extra long - unless you are okay with that?). |
The following functions had I can upload the full before and after validation output if you want (I did not want to upload for each as it will make the thread extra long - unless you are okay with that?). |
Thanks @LJArendse |
This pull request will
Fix docstring formatting for
pandas.core.window.Expanding.apply
andpandas.core.window.Rolling.apply
Document
*args
and**kwargs
parameters:The following functions had
*args
and**kwargs
added to their docstring:Rolling.max
Expanding.max
EWM.mean
EWM.std
EWM.var
The following functions had
**kwargs
added to their docstring:Rolling.skew
Expanding.skew
Rolling.cov
Expanding.cov
EWM.cov
EWM.corr