diff --git a/doc/source/whatsnew/v2.1.0.rst b/doc/source/whatsnew/v2.1.0.rst index f2bf446c3bb6d..ec28b7c2ecb11 100644 --- a/doc/source/whatsnew/v2.1.0.rst +++ b/doc/source/whatsnew/v2.1.0.rst @@ -370,6 +370,7 @@ Deprecations - Deprecated the ``method`` and ``limit`` keywords in :meth:`DataFrame.replace` and :meth:`Series.replace` (:issue:`33302`) - Deprecated the use of non-supported datetime64 and timedelta64 resolutions with :func:`pandas.array`. Supported resolutions are: "s", "ms", "us", "ns" resolutions (:issue:`53058`) - Deprecated values "pad", "ffill", "bfill", "backfill" for :meth:`Series.interpolate` and :meth:`DataFrame.interpolate`, use ``obj.ffill()`` or ``obj.bfill()`` instead (:issue:`53581`) +- Deprecated the behavior of :meth:`Index.argmax`, :meth:`Index.argmin`, :meth:`Series.argmax`, :meth:`Series.argmin` with either all-NAs and skipna=True or any-NAs and skipna=False returning -1; in a future version this will raise ``ValueError`` (:issue:`33941`, :issue:`33942`) - .. --------------------------------------------------------------------------- diff --git a/pandas/core/base.py b/pandas/core/base.py index e707a151fdb7f..9d6beff340cac 100644 --- a/pandas/core/base.py +++ b/pandas/core/base.py @@ -14,6 +14,7 @@ final, overload, ) +import warnings import numpy as np @@ -36,6 +37,7 @@ cache_readonly, doc, ) +from pandas.util._exceptions import find_stack_level from pandas.core.dtypes.cast import can_hold_element from pandas.core.dtypes.common import ( @@ -735,15 +737,29 @@ def argmax( if isinstance(delegate, ExtensionArray): if not skipna and delegate.isna().any(): + warnings.warn( + f"The behavior of {type(self).__name__}.argmax/argmin " + "with skipna=False and NAs, or with all-NAs is deprecated. " + "In a future version this will raise ValueError.", + FutureWarning, + stacklevel=find_stack_level(), + ) return -1 else: return delegate.argmax() else: + result = nanops.nanargmax(delegate, skipna=skipna) + if result == -1: + warnings.warn( + f"The behavior of {type(self).__name__}.argmax/argmin " + "with skipna=False and NAs, or with all-NAs is deprecated. " + "In a future version this will raise ValueError.", + FutureWarning, + stacklevel=find_stack_level(), + ) # error: Incompatible return value type (got "Union[int, ndarray]", expected # "int") - return nanops.nanargmax( # type: ignore[return-value] - delegate, skipna=skipna - ) + return result # type: ignore[return-value] @doc(argmax, op="min", oppose="max", value="smallest") def argmin( @@ -755,15 +771,29 @@ def argmin( if isinstance(delegate, ExtensionArray): if not skipna and delegate.isna().any(): + warnings.warn( + f"The behavior of {type(self).__name__}.argmax/argmin " + "with skipna=False and NAs, or with all-NAs is deprecated. " + "In a future version this will raise ValueError.", + FutureWarning, + stacklevel=find_stack_level(), + ) return -1 else: return delegate.argmin() else: + result = nanops.nanargmin(delegate, skipna=skipna) + if result == -1: + warnings.warn( + f"The behavior of {type(self).__name__}.argmax/argmin " + "with skipna=False and NAs, or with all-NAs is deprecated. " + "In a future version this will raise ValueError.", + FutureWarning, + stacklevel=find_stack_level(), + ) # error: Incompatible return value type (got "Union[int, ndarray]", expected # "int") - return nanops.nanargmin( # type: ignore[return-value] - delegate, skipna=skipna - ) + return result # type: ignore[return-value] def tolist(self): """ diff --git a/pandas/core/indexes/base.py b/pandas/core/indexes/base.py index 21de473170e31..83eab58e3844a 100644 --- a/pandas/core/indexes/base.py +++ b/pandas/core/indexes/base.py @@ -7271,6 +7271,13 @@ def argmin(self, axis=None, skipna: bool = True, *args, **kwargs) -> int: # Take advantage of cache mask = self._isnan if not skipna or mask.all(): + warnings.warn( + f"The behavior of {type(self).__name__}.argmax/argmin " + "with skipna=False and NAs, or with all-NAs is deprecated. " + "In a future version this will raise ValueError.", + FutureWarning, + stacklevel=find_stack_level(), + ) return -1 return super().argmin(skipna=skipna) @@ -7283,6 +7290,13 @@ def argmax(self, axis=None, skipna: bool = True, *args, **kwargs) -> int: # Take advantage of cache mask = self._isnan if not skipna or mask.all(): + warnings.warn( + f"The behavior of {type(self).__name__}.argmax/argmin " + "with skipna=False and NAs, or with all-NAs is deprecated. " + "In a future version this will raise ValueError.", + FutureWarning, + stacklevel=find_stack_level(), + ) return -1 return super().argmax(skipna=skipna) diff --git a/pandas/core/series.py b/pandas/core/series.py index b6e6d9b5d70c5..b7445bf158b90 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -2530,7 +2530,12 @@ def idxmin(self, axis: Axis = 0, skipna: bool = True, *args, **kwargs) -> Hashab nan """ axis = self._get_axis_number(axis) - i = self.argmin(axis, skipna, *args, **kwargs) + with warnings.catch_warnings(): + # TODO(3.0): this catching/filtering can be removed + # ignore warning produced by argmin since we will issue a different + # warning for idxmin + warnings.simplefilter("ignore") + i = self.argmin(axis, skipna, *args, **kwargs) if i == -1: # GH#43587 give correct NA value for Index. return self.index._na_value @@ -2601,7 +2606,12 @@ def idxmax(self, axis: Axis = 0, skipna: bool = True, *args, **kwargs) -> Hashab nan """ axis = self._get_axis_number(axis) - i = self.argmax(axis, skipna, *args, **kwargs) + with warnings.catch_warnings(): + # TODO(3.0): this catching/filtering can be removed + # ignore warning produced by argmax since we will issue a different + # warning for argmax + warnings.simplefilter("ignore") + i = self.argmax(axis, skipna, *args, **kwargs) if i == -1: # GH#43587 give correct NA value for Index. return self.index._na_value diff --git a/pandas/tests/extension/base/methods.py b/pandas/tests/extension/base/methods.py index 9c699f01944e5..bc79db3d18ad7 100644 --- a/pandas/tests/extension/base/methods.py +++ b/pandas/tests/extension/base/methods.py @@ -160,8 +160,13 @@ def test_argreduce_series( self, data_missing_for_sorting, op_name, skipna, expected ): # data_missing_for_sorting -> [B, NA, A] with A < B and NA missing. + warn = None + if op_name.startswith("arg") and expected == -1: + warn = FutureWarning + msg = "The behavior of Series.argmax/argmin" ser = pd.Series(data_missing_for_sorting) - result = getattr(ser, op_name)(skipna=skipna) + with tm.assert_produces_warning(warn, match=msg): + result = getattr(ser, op_name)(skipna=skipna) tm.assert_almost_equal(result, expected) def test_argmax_argmin_no_skipna_notimplemented(self, data_missing_for_sorting): diff --git a/pandas/tests/reductions/test_reductions.py b/pandas/tests/reductions/test_reductions.py index a15dbb8bcac94..2f5deb7b2bbd5 100644 --- a/pandas/tests/reductions/test_reductions.py +++ b/pandas/tests/reductions/test_reductions.py @@ -113,10 +113,17 @@ def test_nanargminmax(self, opname, index_or_series): # GH#7261 klass = index_or_series arg_op = "arg" + opname if klass is Index else "idx" + opname + warn = FutureWarning if klass is Index else None obj = klass([NaT, datetime(2011, 11, 1)]) assert getattr(obj, arg_op)() == 1 - result = getattr(obj, arg_op)(skipna=False) + + msg = ( + "The behavior of (DatetimeIndex|Series).argmax/argmin with " + "skipna=False and NAs" + ) + with tm.assert_produces_warning(warn, match=msg): + result = getattr(obj, arg_op)(skipna=False) if klass is Series: assert np.isnan(result) else: @@ -125,7 +132,8 @@ def test_nanargminmax(self, opname, index_or_series): obj = klass([NaT, datetime(2011, 11, 1), NaT]) # check DatetimeIndex non-monotonic path assert getattr(obj, arg_op)() == 1 - result = getattr(obj, arg_op)(skipna=False) + with tm.assert_produces_warning(warn, match=msg): + result = getattr(obj, arg_op)(skipna=False) if klass is Series: assert np.isnan(result) else: @@ -155,26 +163,40 @@ def test_argminmax(self): obj = Index([np.nan, 1, np.nan, 2]) assert obj.argmin() == 1 assert obj.argmax() == 3 - assert obj.argmin(skipna=False) == -1 - assert obj.argmax(skipna=False) == -1 + msg = "The behavior of Index.argmax/argmin with skipna=False and NAs" + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmin(skipna=False) == -1 + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmax(skipna=False) == -1 obj = Index([np.nan]) - assert obj.argmin() == -1 - assert obj.argmax() == -1 - assert obj.argmin(skipna=False) == -1 - assert obj.argmax(skipna=False) == -1 + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmin() == -1 + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmax() == -1 + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmin(skipna=False) == -1 + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmax(skipna=False) == -1 + msg = "The behavior of DatetimeIndex.argmax/argmin with skipna=False and NAs" obj = Index([NaT, datetime(2011, 11, 1), datetime(2011, 11, 2), NaT]) assert obj.argmin() == 1 assert obj.argmax() == 2 - assert obj.argmin(skipna=False) == -1 - assert obj.argmax(skipna=False) == -1 + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmin(skipna=False) == -1 + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmax(skipna=False) == -1 obj = Index([NaT]) - assert obj.argmin() == -1 - assert obj.argmax() == -1 - assert obj.argmin(skipna=False) == -1 - assert obj.argmax(skipna=False) == -1 + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmin() == -1 + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmax() == -1 + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmin(skipna=False) == -1 + with tm.assert_produces_warning(FutureWarning, match=msg): + assert obj.argmax(skipna=False) == -1 @pytest.mark.parametrize("op, expected_col", [["max", "a"], ["min", "b"]]) def test_same_tz_min_max_axis_1(self, op, expected_col):