BUG: object and extension-dtype reductions honour skipna=False (GH-4147) - #66508
Open
jbrockmendel wants to merge 4 commits into
Open
BUG: object and extension-dtype reductions honour skipna=False (GH-4147)#66508jbrockmendel wants to merge 4 commits into
jbrockmendel wants to merge 4 commits into
Conversation
Adds regression tests for the family of duplicate reports fixed by GH#65526: GH#4147, GH#18588, GH#24109, GH#58707 and GH#61204. Existing coverage was Series-only, so the 2D take_along_axis branch of _nanminmax that GH#18588 and GH#61204 report was untested. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…#4147) _nanminmax gated its NA fill on `dtype == object and skipna`. With skipna=False the _get_values fallback never builds a mask for object dtype, so NAs were fed into the comparison as-is: a float NaN propagates through a comparison on its own, but an object NA does not. min/max either raised TypeError or returned a value computed as though the NA were absent. De-gate the object branch so the fill always runs, then null out any slice that held an NA via _maybe_null_out(min_count=<slice length>), matching float64, str and datetime64. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Accumulated review fixes across the object-dtype reduction work: idxmin/idxmax object fill, the groupby _agg_py_fallback skipna closures, nansum/nanprod, the whole-array short-circuit, and the accompanying tests and whatsnew entries. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # doc/source/whatsnew/v3.1.0.rst # pandas/tests/frame/test_reductions.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on GH-66507 (test-only, closes the five duplicate reports) — please review that
one first; this PR's diff assumes it. Supersedes GH-66471, which mixed the two and has
been closed.
GH-66507 covers the
skipna=Truehalf of object-dtypemin/max, already fixed onmain by GH-65526. Writing those tests turned up four further defects in the same code
path, all of which are fixed here. References GH-4147 and GH-18588 for context; the
closing keywords live on GH-66507.
1.
skipna=Falseon object dtype_nanminmaxgated the NA fill ondtype == object and skipna. Withskipna=Falsethe_get_valuesfallback never builds a mask for object, so NAs went into the comparisonas-is. A float NaN propagates through a comparison on its own; an object NA does not:
Both now return a missing value. A reduction that covers the whole array short-circuits
to NA; a per-axis 2-D reduction blanks the NA-holding slices and nulls them through
_maybe_null_out. Values in a nulled slice are never compared, so they need not even becomparable with each other —
Series([1, "a", None], dtype=object).min(skipna=False)isNA rather than a
TypeError, while the NA-freeSeries([1, "a"])still raises.2.
idxmin/idxmaxon object dtypenanargmin/nanargmaxkept the±inffill, so they still raised on exactly the datamin/maxnow handles:They now share
_nanminmax's same-slice fill._maybe_fix_arg_at_naneeds no change:if an argmin/argmax lands on a masked position, the fill must be at least as extreme as
every valid value, so it is the extremum, and it sits at the first unmasked position —
exactly what
(~mask).argmax()returns. Verified by brute force against an NA-droppedreference rather than by argument alone. With
skipna=Falsethey now raise the sameValueErrorother dtypes do instead ofTypeError.3.
sum/prodon object dtypeSame defect in
nansum/nanprod: an object NA does not propagate through+/*theway a float NaN does, so
Series([2, None], dtype=object).sum(skipna=False)raised.Note the asymmetry with min/max: NAs that do support the operation (
pd.NA,Decimal("NaN"),NaT) already propagate correctly and carry more type informationthan a bare nan, so those are left alone — the natural reduction is attempted first and
the fill only steps in when it raises.
Series([1, pd.NA, 3], dtype=object).sum(skipna=False)stays
<NA>.4.
skipna=Falsedropped by the groupby fallbackDtypes with no Cython aggregation fall back to
_agg_py_fallback→agg_series(ser, alt)with
alt=np.min, which has no way to receiveskipnaand silently dropped it:npfuncis now a closure that appliesskipna. The same fallback backsDataFrame.min(axis=1)on string columns, so that is fixed by the same change. Reach ismin/maxfor object/StringDtype/IntervalDtype/other EAs,prodfor object andEAs, and
sumfor EAs only — objectsumhas a Cython implementation and alreadyhonoured
skipna.Verification
Beyond the suite: brute-force fuzz against an independent NA-dropped reference; a
systematic mutation sweep over every new source line (every surviving mutation closed and
re-checked); and a differential run of ~63k reduction calls against main with every
difference classified as intended-and-documented or investigated.
Not addressed
mean/std/var/semon object withskipna=Falsestill raiseTypeError. Samedefect, but pre-existing and untouched here; worth a follow-up. Object string
sumwith
skipna=Truealso still raises, on theskipna=Truepath this PR does not modify.