Fix rolling skew/kurt for low variance windows#63121
Conversation
| # it could be treated as zero, here we follow the original | ||
| # skew/kurt behaviour to check | ||
| # m2 <= ((float64_machine_eps * mean) ** 2) * observations | ||
| elif m2 <= m2_cutoff: |
There was a problem hiding this comment.
With the new implementation for skewness, it shouldn't have much of a problem with numerical instability against degenerate distributions, which is the main reason for this branch. Can you check if it's possible to remove this branch or to just compare against 0?
There was a problem hiding this comment.
Indeed works without this, so removed the branch.
Co-authored-by: Álvaro Kothe <kothe65@gmail.com>
Alvaro-Kothe
left a comment
Alvaro-Kothe
left a comment
There was a problem hiding this comment.
Looks good. Just add an entry in doc/source/whatsnew/v3.0.0.rst
Alvaro-Kothe
left a comment
Alvaro-Kothe
left a comment
There was a problem hiding this comment.
LGTM. Just a nit in the test.
|
|
||
|
|
||
| @pytest.mark.parametrize("sp_func, roll_func", [["kurtosis", "kurt"], ["skew", "skew"]]) | ||
| def test_low_variance_series(sp_func, roll_func): |
There was a problem hiding this comment.
Just add a comment linking to the issue for future contributors.
| def test_low_variance_series(sp_func, roll_func): | |
| def test_low_variance_series(sp_func, roll_func): | |
| # GH#62946 |
|
Could someone with write permissions to review this? maybe @mroeschke |
|
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
jbrockmendel
added
Window
|
@Alvaro-Kothe did your reductions PR close this? |
|
The issue was closed by #64366. |
3 participants
Rolling.skewandRolling.kurtreturn NaN for low-variance windows #62946doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature. Will add laterAGENTS.md. Does not apply