-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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: Updating Series.autocorr docstring #22787
DOC: Updating Series.autocorr docstring #22787
Conversation
Hello @HubertKl! Thanks for submitting the PR.
|
pandas/core/series.py
Outdated
float | ||
The Pearson correlation between self and self.shift(lag). | ||
|
||
See also |
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.
Does the "A" in also need to be capitalized?
pandas/core/series.py
Outdated
See also | ||
-------- | ||
Series.corr : Compute the correlation between two Series. | ||
|
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.
Add Series.shift, DataFrame.corr and DataFrame.corrwith
pandas/core/series.py
Outdated
|
||
Notes | ||
----- | ||
return Nan if the computation of the autocorr is not possible. |
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.
Perhaps say something like if the correlation is not defined. Nan should be written as
`NaN`
pandas/core/series.py
Outdated
|
||
**Warning** | ||
|
||
If the pearson correlation between self and self.shift(lag) cannot be |
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.
Rather than "self" and "self.shift(lag)", how about "If the Pearson correlation between the two points is not defined, then ``NaN`` is returned.
Codecov Report
@@ Coverage Diff @@
## master #22787 +/- ##
==========================================
+ Coverage 92.18% 92.18% +<.01%
==========================================
Files 169 169
Lines 50810 50820 +10
==========================================
+ Hits 46839 46850 +11
+ Misses 3971 3970 -1
Continue to review full report at Codecov.
|
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.
Not sure if Pearson should be capitalized.
Also, I'd personally remove the "Warning" header in the examples, I think it gives more emphasis than needed.
But looks good to me. Good improvement. Thanks @HubertKl
Thanks @TomAugspurger and @datapythonista for the constructive comments. I taken all of those into account in the last version. |
do you mind replacing |
Unless I made a mistake, I think that I replaced Pearson by pearson in the Third commit. Edit: sorry I read it too quickly you suggested the other way arond, I got it. Sure! |
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, thank you @HubertKl for the changes! Will merge on green.
Thanks @HubertKl! |
git diff upstream/master -u -- "*.py" | flake8 --diff