-
-
Notifications
You must be signed in to change notification settings - Fork 19.4k
BUG: ewm*() interpretation of min_periods is off by one #7898
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
|
are there test for min_periods 1 and 0? thxs |
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.
Just removing gratuitous "a" from function names, and adding ewmvol.
|
OK, will add tests for min_periods 0 and 1. |
|
In the process of adding tests for min_periods = 0, 1, I noticed the following, which is really messing up the testing. I think all these functions should return I just noticed that |
|
I've updated the code to add tests for min_periods = 0, 1, 2, leaving the single-value behavior noted above unchanged. If want to, can address that in a separate issue/PR: #7900. |
doc/source/v0.15.0.txt
Outdated
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 the issue at the end
|
The Travis CI build passed. |
doc/source/v0.15.0.txt
Outdated
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.
change to issue and repush and lmk (don't need to wait for green)
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.
D'oh! Am away from computer now; will do later.
On Aug 1, 2014, at 6:40 PM, jreback notifications@github.com wrote:
In doc/source/v0.15.0.txt:
@@ -80,6 +80,11 @@ API changes
ewma(Series([1., None, 100.]), com=2.5, ignore_na=True) # pre-0.15.0 behavior
ewma(Series([1., None, 100.]), com=2.5, ignore_na=False) # default+- :func:
ewma, :func:ewmstd, :func:ewmvol, :func:ewmvar, :func:ewmcorr, and :func:ewmcov
- now set to
NaNthe firstmin_periods-1entries of the result (formin_periods>1).- Previously the first
min_periodsentries of the result were set toNaN.- The new behavior accords with the existing documentation. (:issues:
7884)
change to issue and repush and lmk (don't need to wait for green)—
Reply to this email directly or view it on GitHub.
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 re-pushed an update removing the spurious s.
BUG: ewm*() interpretation of min_periods is off by one
|
thanks! |
Closes #7884.
This addresses only the
ewm*()functions' off-by-one interpretation ofmin_periods.It doesn't address the inconsistency between the
ewm*()functions and therolling_*()functions in the meaning ofmin_periods.