Skip to content

Conversation

rhshadrach
Copy link
Member

@rhshadrach rhshadrach commented Aug 2, 2025

@rhshadrach rhshadrach added Groupby API - Consistency Internal Consistency of API/Behavior Reduction Operations sum, mean, min, max, etc. labels Aug 2, 2025
@rhshadrach rhshadrach added this to the 3.0 milestone Aug 2, 2025
@rhshadrach rhshadrach requested a review from jbrockmendel August 2, 2025 18:55
@@ -504,7 +504,7 @@ Renamed the following offset aliases (:issue:`57986`):

Other Removals
^^^^^^^^^^^^^^
- :class:`.DataFrameGroupBy.idxmin`, :class:`.DataFrameGroupBy.idxmax`, :class:`.SeriesGroupBy.idxmin`, and :class:`.SeriesGroupBy.idxmax` will now raise a ``ValueError`` when used with ``skipna=False`` and an NA value is encountered (:issue:`10694`)
- :class:`.DataFrameGroupBy.idxmin`, :class:`.DataFrameGroupBy.idxmax`, :class:`.SeriesGroupBy.idxmin`, and :class:`.SeriesGroupBy.idxmax` will now raise a ``ValueError`` when a group has all NA values, or when used with ``skipna=False`` and any NA value is encountered (:issue:`10694`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does 10694 cover both of these cases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - added the linked issue in the OP. Thanks!

@@ -283,8 +283,6 @@ def test_idxmin_idxmax_extremes_skipna(skipna, how, float_numpy_dtype):
np.nan,
max_value,
np.nan,
np.nan,
np.nan,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is bc there's an all-NA group so itll now raise?

Copy link
Member Author

@rhshadrach rhshadrach Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and the case of testing for raising is done elsewhere.

@jbrockmendel
Copy link
Member

Couple of small questions, assuming no surprises on those: LGTM

@rhshadrach rhshadrach requested a review from mroeschke August 5, 2025 18:37
@mroeschke mroeschke merged commit eb489f2 into pandas-dev:main Aug 5, 2025
43 checks passed
@mroeschke
Copy link
Member

Thanks @rhshadrach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API - Consistency Internal Consistency of API/Behavior Groupby Reduction Operations sum, mean, min, max, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: groupby.idxmin/idxmax will all NA values
3 participants