-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: expanding/rolling_skew/kurt() inconsistent with Series.skew/kurt() #8086
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
Comments
My 2c: there really shouldn't be two different implementations of skew/kurtosis functions. |
@jreback, afraid I'm not sure I follow your last comment. Do you mean the following? As a test for a fix, un-comment the following lines in
|
@seth-p I realize now why you commented these out |
The solution to this is to modify the behavior of |
I think so, assuming they are supposed to return unbiased estimates. Though to be honest I haven't examined them closely beyond observing the inconsistencies noted above. |
I was looking into this to remove the commented tests for the rolling versions. There are a bunch of tests making sure that the return value is 0, and the code itself explicitly sets things to 0. So even if it is a bug, it was clearly considered a feature when the code was written. Not sure whether changing the behavior can be happily done without breaking someone's code... |
relatd to this: #7928 I wouldn't return |
I think, ideally, the commented-out consistency tests (#8086 (comment)) should be there. Obviously some of the cases are Separately, I think should add to
|
Regarding |
@jreback, if need to divide by a denominator that is 0, then I think should return |
@seth-p yep that too! |
@jreback, no, at least in master, calculating unbiased ( |
@seth-p what I mean is that |
Ah, sorry, I misunderstood you. Yes, all the |
as far as the tests go, yes I think will have to change those tests and test for nan in those edge cases. |
In [133]: s.rolling(4).kurt()
Out[133]:
0 NaN
1 NaN
2 NaN
3 NaN
dtype: float64
In [134]: s.kurt()
Out[134]: 0 |
Note that for a constant series,
Series.skew()
returns0
, whilerolling/expanding_skew()
returnNaN
.While
rolling/expanding_kurt()
similarly returnNaN
for a constant series,Series.kurt()
produces an error.The text was updated successfully, but these errors were encountered: