-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DOC: explain EWM #34910
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: explain EWM #34910
Conversation
Hello @KrishnaSai2020! 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-20 20:06:07 UTC |
@charlesdong1991 believe this is done please have a look when you can. |
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, this looks much cleaner now! only one nitpick
Co-authored-by: Kaiqi Dong <[email protected]>
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.
hey, it's very close.
I think you could search pandas.core.window.ewm
in pandas, and there are some other usages of it, probably in some rst files, otherwise doc validation won't pass.
An easier walkaround would be keeping the file name unchanged as ewm.py
, the main purpose of this PR i think is to make EMW
class name more intuitive, while not too big deal to have ewm.py
in dev, but you might still need to change a few places to make CI pass.
thanks @KrishnaSai2020 |
@jreback np and @charlesdong1991 thanks for your help! |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff