-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
REGR: fix DataFrame sum and prod with min_count and numeric_only #41701
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
REGR: fix DataFrame sum and prod with min_count and numeric_only #41701
Conversation
@@ -9856,36 +9856,42 @@ def _get_data() -> DataFrame: | |||
|
|||
return out | |||
|
|||
assert numeric_only is None |
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.
this should still hold?
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.
oh i guess it should be assert numeric_only is None or min_count != 0
?
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.
correct we are only using the old code path when min_count is given until _mgr.reduce can handle min_count.
why do we need to add an assert?
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 needed, just took me a minute to understand what had changed
this overlaps with #41706 ? |
ok so this is an alt to #41711 ? |
#41711 and I reopen this directly against 1.2.x why is this not the preferred option? |
closing in favor of #41711 (comment) The change here was effectively only
when min_count and numeric_only is given, without touching any other library code |
it appears that the clean-up in #35899 to simplify _reduce may have been a bit premature, have reinstated a few lines of the removed code for backport purposes.
The longer term fix would probably be to support min_count in _mgr.reduce, #40143 (comment)