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
Implement numeric_only
for skew
and kurtosis
#10258
Conversation
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.
Nice. Make sure to test upstream
as well.
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 @phofl. Just one small comment
@@ -1426,6 +1426,36 @@ def test_reductions_frame_dtypes_numeric_only(func): | |||
) | |||
|
|||
|
|||
@pytest.mark.parametrize("func", ["skew", "kurtosis"]) | |||
def test_skew_kurt_numeric_only_false(func): |
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.
Can we add an assert_eq
to make sure the result from dask matches pandas?
Also, is numeric_only=True
tested elsewhere?
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.
Both of them are tested somewhere else, that's why I added only the False case here.
I would rather raise. We should have added a deprecation earlier, that's true, but at least right now we have the chance to catch up with pandas' behavior. Supporting diverging behaviors and following one step behind feels like extra work for not much benefit. Just my 2c. |
That's what we are doing, so all good |
pre-commit run --all-files
cc @jrbourbeau
Not sure what the policy Is, but this changes behaviour for pandas 2.0, with this pr
raises like pandas does, but without this pr it drops the dt column. I guess? we would prefer not raising here without deprecating first since we did not show any deprecation warnings?
Nothing changes with pandas < 2.0.