-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DOC: Clarify where to the additional arguments for some win_types #34615
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
Conversation
DOC: Clarify where to the additional arguments for some win_types For example, std needs to specify when win_types is gaussian. However, std should be specified in the operation argument, not as one of the rolling arguments. This change is to clarify this point. Closes: pandas-dev#34593
pandas/core/window/rolling.py
Outdated
* ``gaussian`` (needs std in operation argument) | ||
* ``general_gaussian`` (needs power, width in operation argument) | ||
* ``slepian`` (needs width in operation argument) | ||
* ``exponential`` (needs tau in operation argument), center is set to None. |
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.
I'm wondering if it may be helpful to also add some additional explanation below (something like: "Certain window types require that a scale parameter be passed...") since this is a fairly confusing situation, and then call attention to the Gaussian case in the examples section. Thoughts @wchen928 @MarcoGorelli?
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.
How about we change to the following and add one more sentence before we list all the examples.
* ``exponential`` (needs tau in operation argument), center is set to None. | |
* ``exponential`` (need parameter: tau), center is set to None. |
Additional sentence: Certain window types require a scale parameter to be passed. Please see the third example below on how to add the additional parameters.
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.
Please see the third example below on how to add the additional parameters.
Sure, sounds good to me
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.
Great. I will wait for @dsaxton 's opinion and before editing it.
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.
Works for me too 👍
Edit: 711add5 First Commit Original issue: pandas-dev#34615
Hello @wchen928! 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 2020-06-17 20:58:13 UTC |
Edit: a3e38ac Second Commit What's new: I shortened line 935 so that each line is less than 88 characters. Original Issue: pandas-dev#34615
…ypes" This reverts commit 45be538.
…me win_types"" This reverts commit 05ed0eb.
Merged master to fix the CI failure. |
Thanks @wchen928 |
For example, std needs to specify when win_types is gaussian. However, std should be specified in the operation argument, not as one of the rolling arguments. This change is to clarify this point.
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff