BUG: object/EA reductions honour skipna=False (GH#4147, GH#18588)

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>

Author: jbrockmendel

doc/source/whatsnew/v3.1.0.rst

@@ -380,8 +380,12 @@ Timezones
 
 Numeric
 ^^^^^^^
-- Bug in :meth:`DataFrame.min`, :meth:`DataFrame.max`, :meth:`Series.min`, and :meth:`Series.max` on object-dtype data raising ``TypeError`` when missing values were present alongside values that do not support comparison with ``float`` (e.g. strings, :class:`datetime.date` objects, or mixed-timezone :class:`Timestamp` objects) (:issue:`4147`, :issue:`18588`, :issue:`24109`, :issue:`58707`, :issue:`61204`, :issue:`65500`)
-- Bug in :meth:`DataFrame.min`, :meth:`DataFrame.max`, :meth:`Series.min`, and :meth:`Series.max` on object-dtype data with ``skipna=False`` failing to propagate missing values, either raising ``TypeError`` or returning a value computed as though the missing values were absent; they now return a missing value like other dtypes do (:issue:`4147`)
+- Bug in :meth:`DataFrame.idxmin`, :meth:`DataFrame.idxmax`, :meth:`Series.idxmin`, :meth:`Series.idxmax`, :meth:`Series.argmin`, :meth:`Series.argmax`, :meth:`Index.argmin`, and :meth:`Index.argmax` on object-dtype data raising ``TypeError`` when missing values were present alongside values that do not support comparison with ``float`` (e.g. strings or :class:`datetime.date` objects); with ``skipna=False`` they now raise ``ValueError`` like other dtypes instead of ``TypeError`` (:issue:`4147`)
+- Bug in :meth:`DataFrame.min`, :meth:`DataFrame.max`, :meth:`Series.min`, :meth:`Series.max`, :meth:`Index.min`, and :meth:`Index.max` on object-dtype data raising ``TypeError`` when missing values were present alongside values that do not support comparison with ``float`` (e.g. strings, :class:`datetime.date` objects, or mixed-timezone :class:`Timestamp` objects) (:issue:`4147`, :issue:`18588`, :issue:`24109`, :issue:`58707`, :issue:`61204`, :issue:`65500`)
+- Bug in :meth:`DataFrame.min` and :meth:`DataFrame.max` with ``axis=1`` ignoring ``skipna=False`` for :class:`StringDtype`, :class:`IntervalDtype`, and other extension dtypes, returning a value where a missing value was expected (:issue:`18588`)
+- Bug in :meth:`DataFrame.min`, :meth:`DataFrame.max`, :meth:`Series.min`, and :meth:`Series.max` on object-dtype data with ``skipna=False`` failing to propagate missing values, either raising ``TypeError`` or returning a value computed as though the missing values were absent; they now return a missing value (:issue:`4147`)
+- Bug in :meth:`DataFrame.sum` and :meth:`DataFrame.prod` with ``axis=1`` ignoring ``skipna=False`` for extension dtypes, returning a value where a missing value was expected (:issue:`18588`)
+- Bug in :meth:`DataFrame.sum`, :meth:`DataFrame.prod`, :meth:`Series.sum`, and :meth:`Series.prod` on object-dtype data with ``skipna=False`` raising ``TypeError`` instead of returning a missing value (:issue:`4147`)
 - Fixed bug in :func:`read_excel` where having a column with mixture of numeric and boolean values will typecast the values based on the first appearance data type since 1==True and 0==False (:issue:`60088`)
 - Fixed bug in :meth:`DataFrame.idxmax` and :meth:`DataFrame.idxmin` returning incorrect row labels for nullable ``UInt64`` columns containing missing values alongside values above ``2**53`` (:issue:`64478`)
 - Fixed bug in :meth:`DataFrame.idxmax` and :meth:`DataFrame.idxmin` with ``axis=1`` and ``skipna=False`` returning incorrect column labels for extension array dtypes (e.g. :class:`BooleanDtype`, nullable integer/float, :class:`ArrowDtype`) (:issue:`56903`)
@@ -572,6 +576,8 @@ Groupby/resample/rolling
 - Bug in :meth:`.DataFrameGroupBy.apply` with ``as_index=False`` where applying on an empty :class:`DataFrame` returned inconsistent index metadata compared to non-empty results (:issue:`48135`)
 - Bug in :meth:`.DataFrameGroupBy.cumprod`, :meth:`.DataFrameGroupBy.cummin`, and :meth:`.DataFrameGroupBy.cummax` (and Series variants) returning ``Float64`` instead of preserving the nullable integer dtype (e.g. ``Int64``) when the group key contains ``NA`` (:issue:`65550`)
 - Bug in :meth:`.DataFrameGroupBy.idxmax`, :meth:`.DataFrameGroupBy.idxmin` (and :class:`Series` variants) with ``skipna=False`` returning incorrect results when the input contained no ``NA`` values (:issue:`56903`)
+- Bug in :meth:`.DataFrameGroupBy.min` and :meth:`.DataFrameGroupBy.max` (and :class:`Series` variants) ignoring ``skipna=False`` for object, :class:`StringDtype`, :class:`IntervalDtype`, and other extension dtypes, returning a value where a missing value was expected (:issue:`18588`)
+- Bug in :meth:`.DataFrameGroupBy.sum` and :meth:`.DataFrameGroupBy.prod` (and :class:`Series` variants) ignoring ``skipna=False`` for extension dtypes, and for object dtype with ``prod``, returning a value where a missing value was expected (:issue:`18588`)
 - Bug in :meth:`.GroupBy.any` and :meth:`.GroupBy.all` returning ``NaN`` with ``float64`` dtype for unobserved categorical groups on NumPy ``bool`` data instead of the boolean identity value with ``bool`` dtype (:issue:`65100`)
 - Bug in :meth:`.GroupBy.quantile` returning incorrect results for groups containing a non-null ``NaN`` value (e.g. from a pyarrow or masked float array), and returning the Unix epoch instead of ``NaT`` for all-``NaT`` datetime-like groups on some platforms (:issue:`64330`)
 - Bug in :meth:`.Resampler.agg` raising ``ValueError`` with a dict of aggregations when applied to a :meth:`DataFrame.groupby` with ``as_index=False`` (:issue:`52397`)

pandas/core/groupby/groupby.py

@@ -2876,6 +2876,11 @@ def sum(
                 skipna=skipna,
             )
         else:
+
+            def sum_compat(obj: NDFrameT):
+                # GH#18588: see min_compat below
+                return obj.sum(skipna=skipna)
+
             # If we are grouping on categoricals we want unobserved categories to
             # return zero, rather than the default of NaN which the reindexing in
             # _agg_general() returns. GH #31422
@@ -2884,7 +2889,7 @@ def sum(
                     numeric_only=numeric_only,
                     min_count=min_count,
                     alias="sum",
-                    npfunc=np.sum,
+                    npfunc=sum_compat,
                     skipna=skipna,
                 )
 
@@ -2965,12 +2970,17 @@ def prod(
         1   16   10
         2   30   72
         """
+
+        def prod_compat(obj: NDFrameT):
+            # GH#18588: see min_compat below
+            return obj.prod(skipna=skipna)
+
         return self._agg_general(
             numeric_only=numeric_only,
             min_count=min_count,
             skipna=skipna,
             alias="prod",
-            npfunc=np.prod,
+            npfunc=prod_compat,
         )
 
     @final
@@ -3082,12 +3092,19 @@ def min(
                 skipna=skipna,
             )
         else:
+
+            def min_compat(obj: NDFrameT):
+                # GH#18588: object/string dtypes have no cython group_min_max
+                # and reduce through this alt instead, so it has to apply
+                # skipna itself; np.min would always skip.
+                return obj.min(skipna=skipna)
+
             return self._agg_general(
                 numeric_only=numeric_only,
                 min_count=min_count,
                 skipna=skipna,
                 alias="min",
-                npfunc=np.min,
+                npfunc=min_compat,
             )
 
     @final
@@ -3199,12 +3216,17 @@ def max(
                 skipna=skipna,
             )
         else:
+
+            def max_compat(obj: NDFrameT):
+                # GH#18588: see min_compat above
+                return obj.max(skipna=skipna)
+
             return self._agg_general(
                 numeric_only=numeric_only,
                 min_count=min_count,
                 skipna=skipna,
                 alias="max",
-                npfunc=np.max,
+                npfunc=max_compat,
             )
 
     @final

pandas/core/nanops.py

@@ -663,7 +663,30 @@ def nansum(
     np.float64(3.0)
     """
     dtype = values.dtype
-    values, mask = _get_values(values, skipna, fill_value=0, mask=mask)
+    if dtype == object and not skipna:
+        if mask is None:
+            mask = isna(values)
+        if mask.any():
+            # GH#4147: an object NA that supports addition propagates on its
+            # own and carries more type information than a bare nan would
+            # (pd.NA, Decimal("NaN"), NaT), so prefer it.  Step in only when it
+            # does not, e.g. None, or values that cannot be added at all.
+            try:
+                the_sum = values.sum(axis, dtype=_get_dtype_max(dtype))
+            except TypeError:
+                pass
+            else:
+                return _maybe_null_out(
+                    the_sum, axis, mask, values.shape, min_count=min_count
+                )
+            if values.ndim == 1 or axis is None:
+                # error: Incompatible return value type (got "Union[Scalar,
+                # ndarray]", expected "Union[ndarray, float, NaTType]")
+                return _na_for_min_count(values, axis)  # type: ignore[return-value]
+            values = _blank_object_na_slices(values, mask, axis, 0)
+            min_count = _na_min_count(values, axis, min_count)
+    else:
+        values, mask = _get_values(values, skipna, fill_value=0, mask=mask)
     dtype_sum = _get_dtype_max(dtype)
     if dtype.kind == "f":
         # GH#43929 float16 sum overflows easily; upcast like numpy does
@@ -1131,6 +1154,102 @@ def nansem(
     return np.sqrt(var) / np.sqrt(count)
 
 
+def _blank_object_na_slices(
+    values: np.ndarray,
+    mask: npt.NDArray[np.bool_],
+    axis: AxisInt,
+    fill_value: Any,
+) -> np.ndarray:
+    """
+    Replace every reduction slice of an object-dtype array that holds an NA
+    with ``fill_value``.
+
+    For ``skipna=False`` those slices are nulled out afterwards, so their values
+    never reach the result and need not even support the operation being applied
+    (GH#4147).  A float NaN propagates through arithmetic and comparison on its
+    own; an object NA does not, and raises instead.
+
+    Only for reductions over an axis of a 2-D array.  When the reduction covers
+    the whole array the result is simply NA, and callers short-circuit instead.
+    """
+    any_na = np.expand_dims(mask.any(axis=axis), axis)
+    return np.where(any_na, fill_value, values)
+
+
+def _na_min_count(values: np.ndarray, axis: AxisInt, min_count: int) -> int:
+    """
+    ``min_count`` that nulls out any slice holding an NA, without weakening an
+    explicit larger ``min_count`` from the caller (GH#4147).
+    """
+    return max(min_count, values.shape[axis])
+
+
+def _fill_object_na(
+    values: np.ndarray,
+    mask: npt.NDArray[np.bool_],
+    axis: AxisInt | None,
+    fill_value: Any,
+) -> np.ndarray:
+    """
+    Replace NA entries in an object-dtype array with a valid value taken from
+    the same reduction slice.
+
+    GH#65500: the +/-inf fill used for other dtypes is not comparable with
+    arbitrary objects (Timestamps, strings, tuples) and raises.  Substituting a
+    value already present in the slice cannot change a min/max, and for
+    argmin/argmax a result landing on a filled position is corrected by
+    ``_maybe_fix_arg_at_na``.  Entirely-NA slices get ``fill_value``, since
+    their result is discarded by the caller either way.
+    """
+    if mask.all():
+        return np.full(values.shape, fill_value, dtype=object)
+    if values.ndim == 1 or axis is None:
+        fill = np.empty(1, dtype=object)
+        # wrap in an array so that e.g. tuple fill values
+        # are not broadcast by np.where
+        fill[0] = values[~mask][0]
+        return np.where(mask, fill, values)
+    indexer = np.expand_dims((~mask).argmax(axis=axis), axis)
+    fill = np.take_along_axis(values, indexer, axis=axis)
+    # argmax selects an NA entry for entirely-NA slices; fill
+    # those with +/-inf instead so the reduction does not warn
+    all_na = np.expand_dims(mask.all(axis=axis), axis)
+    fill = np.where(all_na, fill_value, fill)
+    return np.where(mask, fill, values)
+
+
+def _get_arg_values(
+    values: np.ndarray,
+    *,
+    fill_value_typ: str,
+    mask: npt.NDArray[np.bool_] | None,
+    skipna: bool,
+    axis: AxisInt | None = None,
+) -> tuple[np.ndarray, npt.NDArray[np.bool_] | None]:
+    """
+    Prepare values for nanargmin/nanargmax by filling NA entries so that the
+    comparison does not see them, and raise for ``skipna=False``.
+
+    Mirrors ``_get_values(values, True, ...)`` except for object dtype, where
+    the +/-inf fill is not comparable with arbitrary objects (GH#4147).  The
+    object fill is a same-slice value, which *can* win the comparison; a result
+    landing on it is corrected by ``_maybe_fix_arg_at_na``.
+    """
+    if values.dtype == object:
+        if mask is None:
+            mask = isna(values)
+        if not skipna and mask.any():
+            # _maybe_arg_null_out would raise this once the argmin/argmax is
+            # known, but the comparison must not run first: values in a slice
+            # holding an NA need not be comparable with each other (GH#4147)
+            raise ValueError("Encountered an NA value with skipna=False")
+        if mask.any():
+            fill_value = _get_fill_value(values.dtype, fill_value_typ=fill_value_typ)
+            values = _fill_object_na(values, mask, axis, fill_value)
+        return values, mask
+    return _get_values(values, True, fill_value_typ=fill_value_typ, mask=mask)
+
+
 def _nanminmax(meth, fill_value_typ):
     @bottleneck_switch(name=f"nan{meth}")
     @_datetimelike_compat
@@ -1148,35 +1267,21 @@ def reduction(
         min_count = 1
         if dtype == object:
             # GH#65500: _get_values' +/-inf fill isn't comparable with arbitrary
-            # objects (Timestamps, strings) and raises.  Fill NAs from the same
-            # slice instead (leaves min/max unchanged); all-NA slices keep the
-            # +/-inf fill since _maybe_null_out discards them anyway.
+            # objects (Timestamps, strings) and raises; see _fill_object_na.
             if mask is None:
                 mask = isna(values)
-            if not skipna:
-                # GH#4147: a float NaN propagates through the comparison on its
-                # own, but an object NA does not, so it used to be compared
-                # as-is.  Fill it like the skipna case so nothing raises, then
-                # null out every slice that held one to match float64/str/dt64.
-                min_count = values.size if axis is None else values.shape[axis]
             if mask.any():
                 fill_value = _get_fill_value(dtype, fill_value_typ=fill_value_typ)
-                if mask.all():
-                    values = np.full(values.shape, fill_value, dtype=object)
+                if skipna:
+                    values = _fill_object_na(values, mask, axis, fill_value)
                 elif values.ndim == 1 or axis is None:
-                    fill = np.empty(1, dtype=object)
-                    # wrap in an array so that e.g. tuple fill values
-                    # are not broadcast by np.where
-                    fill[0] = values[~mask][0]
-                    values = np.where(mask, fill, values)
+                    # GH#4147: an object NA does not propagate through the
+                    # comparison the way a float NaN does, so null out every
+                    # slice that holds one, matching float64/str/dt64.
+                    return _na_for_min_count(values, axis)
                 else:
-                    indexer = np.expand_dims((~mask).argmax(axis=axis), axis)
-                    fill = np.take_along_axis(values, indexer, axis=axis)
-                    # argmax selects an NA entry for entirely-NA slices; fill
-                    # those with +/-inf instead so the reduction does not warn
-                    all_na = np.expand_dims(mask.all(axis=axis), axis)
-                    fill = np.where(all_na, fill_value, fill)
-                    values = np.where(mask, fill, values)
+                    values = _blank_object_na_slices(values, mask, axis, fill_value)
+                    min_count = _na_min_count(values, axis, min_count)
         else:
             values, mask = _get_values(
                 values, skipna, fill_value_typ=fill_value_typ, mask=mask
@@ -1237,7 +1342,9 @@ def nanargmax(
     >>> nanops.nanargmax(arr, axis=1)
     array([2, 2, 1, 1])
     """
-    values, mask = _get_values(values, True, fill_value_typ="-inf", mask=mask)
+    values, mask = _get_arg_values(
+        values, fill_value_typ="-inf", mask=mask, skipna=skipna, axis=axis
+    )
     result = values.argmax(axis)
     # error: Argument 1 to "_maybe_fix_arg_at_na" has incompatible type "Any |
     # signedinteger[Any]"; expected "ndarray[Any, Any]"
@@ -1284,7 +1391,9 @@ def nanargmin(
     >>> nanops.nanargmin(arr, axis=1)
     array([0, 0, 1, 1])
     """
-    values, mask = _get_values(values, True, fill_value_typ="+inf", mask=mask)
+    values, mask = _get_arg_values(
+        values, fill_value_typ="+inf", mask=mask, skipna=skipna, axis=axis
+    )
     result = values.argmin(axis)
     # error: Argument 1 to "_maybe_fix_arg_at_na" has incompatible type "Any |
     # signedinteger[Any]"; expected "ndarray[Any, Any]"
@@ -1443,7 +1552,26 @@ def nanprod(
     """
     mask = _maybe_get_mask(values, skipna, mask)
 
-    if skipna and mask is not None:
+    if values.dtype == object and not skipna:
+        # GH#4147: see nansum
+        if mask is None:
+            mask = isna(values)
+        if mask.any():
+            try:
+                result = values.prod(axis)
+            except TypeError:
+                pass
+            else:
+                return _maybe_null_out(  # type: ignore[return-value]
+                    result, axis, mask, values.shape, min_count=min_count
+                )
+            if values.ndim == 1 or axis is None:
+                # error: Incompatible return value type (got "Union[Scalar,
+                # ndarray]", expected "float")
+                return _na_for_min_count(values, axis)  # type: ignore[return-value]
+            values = _blank_object_na_slices(values, mask, axis, 1)
+            min_count = _na_min_count(values, axis, min_count)
+    elif skipna and mask is not None:
         values = values.copy()
         values[mask] = 1
     result = values.prod(axis)
@@ -1459,10 +1587,11 @@ def _maybe_fix_arg_at_na(
     mask: npt.NDArray[np.bool_] | None,
     axis: AxisInt | None,
 ) -> np.ndarray:
-    # helper function for nanargmin/nanargmax: an argmin/argmax that landed on
-    # a masked position means the extremum equals the sentinel fill, so every
-    # unmasked value ties with the fill; the first unmasked position is the
-    # correct result (GH#64478)
+    # helper function for nanargmin/nanargmax.  Masked positions carry either
+    # the +/-inf sentinel or, for object dtype, the value at the first unmasked
+    # position (GH#4147).  Either way an argmin/argmax landing on a masked
+    # position means the fill is the extremum, so the first unmasked position
+    # holds it and is the correct result (GH#64478)
     if mask is None or not mask.any():
         return result
     if axis is None or mask.ndim == 1: