CLN: Enforce deprecation of argmin/max and idxmin/max with NA values #57971
Conversation
rhshadrach
added
Missing-data
|
Thanks @rhshadrach |
| raise ValueError("Encountered all NA values") | ||
| elif not skipna and isna(delegate).any(): | ||
| raise ValueError("Encountered an NA value with skipna=False") | ||
|
|
There was a problem hiding this comment.
huh IIRC the idea with the deprecation was that eventually we'd be able to make this method just return self.array.argmax(skipna=skipna)
There was a problem hiding this comment.
Thanks - makes sense. Right now the various arrays raise NotImplementedError. I think changing this to a ValueError will make this doable. Will followup.
One other mistake I've realized here (thanks to #58013) - I thought it would be more helpful to raise "all NA values" even when skipna=False if all values are NA, but that requires doing an extra O(n) check. I plan to change the message to "Encountered an NA value with skipna=False" even when all values are NA for perf.
There was a problem hiding this comment.
i might be confusing this with argsort; there is a TODO(3.0) comment in Series.argsort about using self.array.argsort directly
…andas-dev#57971) * CLN: Enforce deprecation of argmin/max and idxmin/max with NA values * Docstrings
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.Ref: #33941, #51276
This is complicated by #57745 - we still need a proper deprecation for groupby's idxmin/idxmax. For DataFrame with EAs and axis=1, we use groupby's implementation. So I'm leaving that deprecation in place for now, and we can enforce it after groupby's is deprecated and ready to be enforced.