Deprecating Series.argmin and Series.argmax (pandas-dev#16830)

Added statements about correcting behavior in future commit

Add reference to github ticket

Fixing placement of github comment

Made test code more explicit

Fixing unrelated tests that are also throwing warnings

Updating whatsnew to give more detail about deprecation

Fixing whatsnew and breaking out tests to catch warnings

Additional comments and more concise whatsnew

Updating deprecate decorator to support custom message

DOC: Update docstrings, depr message, and whatsnew

Author: Lucas Kushner

doc/source/whatsnew/v0.21.0.txt

@@ -439,12 +439,35 @@ Other API Changes
 
 Deprecations
 ~~~~~~~~~~~~
-- :func:`read_excel()` has deprecated ``sheetname`` in favor of ``sheet_name`` for consistency with ``.to_excel()`` (:issue:`10559`).
 
+- :func:`read_excel()` has deprecated ``sheetname`` in favor of ``sheet_name`` for consistency with ``.to_excel()`` (:issue:`10559`).
 - ``pd.options.html.border`` has been deprecated in favor of ``pd.options.display.html.border`` (:issue:`15793`).
 
 - :func:`SeriesGroupBy.nth` has deprecated ``True`` in favor of ``'all'`` for its kwarg ``dropna`` (:issue:`11038`).
 
+Series.argmax and Series.argmin
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+- The behavior of :func:`Series.argmax` has been deprecated in favor of :func:`Series.idxmax` (:issue:`16830`)
+- The behavior of :func:`Series.argmin` has been deprecated in favor of :func:`Series.idxmin` (:issue:`16830`)
+
+For compatibility with NumPy arrays, ``pd.Series`` implements ``argmax`` and
+``argmin``. Since pandas 0.13 :func:`Series.argmax` and
+:func:`Series.argmin` returned the *label* of the maximum and minimum,
+rather than the *position*.
+
+Since returning the positional ``argmax`` and ``argmin`` can be useful,
+we've deprecated the current behavior of :func:`Series.argmax` and
+:func:`Series.argmin`. Using either of these will emit a ``FutureWarning``.
+
+If you were using ``Series.argmin`` and ``Series.argmax``, please switch to using
+``idxmin`` and ``idxmax``. In the future, ``Series.argmin`` will return the
+*position* of the minimum. Likewise for ``Series.argmax``.
+
+Until this functionality is implemented in the future, we recommend using
+``Series.values.argmin()`` and ``Series.values.argmax()`` for returning
+the positional ``argmin`` and ``argmax``.
+
 .. _whatsnew_0210.prior_deprecations:
 
 Removal of prior version deprecations/changes

pandas/core/series.py

@@ -68,7 +68,8 @@
 import pandas.core.common as com
 import pandas.core.nanops as nanops
 import pandas.io.formats.format as fmt
-from pandas.util._decorators import Appender, deprecate_kwarg, Substitution
+from pandas.util._decorators import (
+    Appender, deprecate, deprecate_kwarg, Substitution)
 from pandas.util._validators import validate_bool_kwarg
 
 from pandas._libs import index as libindex, tslib as libts, lib, iNaT
@@ -1271,7 +1272,7 @@ def duplicated(self, keep='first'):
 
     def idxmin(self, axis=None, skipna=True, *args, **kwargs):
         """
-        Index of first occurrence of minimum of values.
+        Index *label* of the first occurrence of minimum of values.
 
         Parameters
         ----------
@@ -1284,7 +1285,9 @@ def idxmin(self, axis=None, skipna=True, *args, **kwargs):
 
         Notes
         -----
-        This method is the Series version of ``ndarray.argmin``.
+        This method is the Series version of ``ndarray.argmin``. This method
+        returns the label of the minimum, while ``ndarray.argmin`` returns
+        the position. To get the position, use ``series.values.argmin()``.
 
         See Also
         --------
@@ -1299,7 +1302,7 @@ def idxmin(self, axis=None, skipna=True, *args, **kwargs):
 
     def idxmax(self, axis=None, skipna=True, *args, **kwargs):
         """
-        Index of first occurrence of maximum of values.
+        Index *label* of the first occurrence of maximum of values.
 
         Parameters
         ----------
@@ -1312,7 +1315,9 @@ def idxmax(self, axis=None, skipna=True, *args, **kwargs):
 
         Notes
         -----
-        This method is the Series version of ``ndarray.argmax``.
+        This method is the Series version of ``ndarray.argmax``. This method
+        returns the label of the maximum, while ``ndarray.argmax`` returns
+        the position. To get the position, use ``series.values.argmax()``.
 
         See Also
         --------
@@ -1326,8 +1331,18 @@ def idxmax(self, axis=None, skipna=True, *args, **kwargs):
         return self.index[i]
 
     # ndarray compat
-    argmin = idxmin
-    argmax = idxmax
+    argmin = deprecate('argmin', idxmin,
+                       msg="'argmin' is deprecated. Use 'idxmin' instead. "
+                           "The behavior of 'argmin' will be corrected to "
+                           "return the positional minimum in the future. "
+                           "Use 'series.values.argmin' to get the position of "
+                           "the minimum now.")
+    argmax = deprecate('argmax', idxmax,
+                       msg="'argmax' is deprecated. Use 'idxmax' instead. "
+                           "The behavior of 'argmax' will be corrected to "
+                           "return the positional maximum in the future. "
+                           "Use 'series.values.argmax' to get the position of "
+                           "the maximum now.")
 
     def round(self, decimals=0, *args, **kwargs):
         """

pandas/tests/series/test_analytics.py

@@ -1242,16 +1242,31 @@ def test_idxmin(self):
         result = s.idxmin()
         assert result == 1
 
-    def test_numpy_argmin(self):
-        # argmin is aliased to idxmin
+    def test_numpy_argmin_deprecated(self):
+        # See gh-16830
         data = np.random.randint(0, 11, size=10)
-        result = np.argmin(Series(data))
-        assert result == np.argmin(data)
+
+        s = Series(data)
+        with tm.assert_produces_warning(FutureWarning, check_stacklevel=False):
+            # The deprecation of Series.argmin also causes a deprecation
+            # warning when calling np.argmin. This behavior is temporary
+            # until the implemention of Series.argmin is corrected.
+            result = np.argmin(s)
+            expected = np.argmin(data)
+            assert result == expected
+
+        with tm.assert_produces_warning(FutureWarning):
+            # argmin is aliased to idxmin
+            result = s.argmin()
+            expected = s.idxmin()
+            assert result == expected
 
         if not _np_version_under1p10:
-            msg = "the 'out' parameter is not supported"
-            tm.assert_raises_regex(ValueError, msg, np.argmin,
-                                   Series(data), out=data)
+            with tm.assert_produces_warning(FutureWarning,
+                                            check_stacklevel=False):
+                msg = "the 'out' parameter is not supported"
+                tm.assert_raises_regex(ValueError, msg, np.argmin,
+                                       s, out=data)
 
     def test_idxmax(self):
         # test idxmax
@@ -1297,17 +1312,31 @@ def test_idxmax(self):
         result = s.idxmin()
         assert result == 1.1
 
-    def test_numpy_argmax(self):
-
-        # argmax is aliased to idxmax
+    def test_numpy_argmax_deprecated(self):
+        # See gh-16830
         data = np.random.randint(0, 11, size=10)
-        result = np.argmax(Series(data))
-        assert result == np.argmax(data)
+
+        s = Series(data)
+        with tm.assert_produces_warning(FutureWarning, check_stacklevel=False):
+            # The deprecation of Series.argmax also causes a deprecation
+            # warning when calling np.argmax. This behavior is temporary
+            # until the implemention of Series.argmax is corrected.
+            result = np.argmax(s)
+            expected = np.argmax(data)
+            assert result == expected
+
+        with tm.assert_produces_warning(FutureWarning):
+            # argmax is aliased to idxmax
+            result = s.argmax()
+            expected = s.idxmax()
+            assert result == expected
 
         if not _np_version_under1p10:
-            msg = "the 'out' parameter is not supported"
-            tm.assert_raises_regex(ValueError, msg, np.argmax,
-                                   Series(data), out=data)
+            with tm.assert_produces_warning(FutureWarning,
+                                            check_stacklevel=False):
+                msg = "the 'out' parameter is not supported"
+                tm.assert_raises_regex(ValueError, msg, np.argmax,
+                                       s, out=data)
 
     def test_ptp(self):
         N = 1000

pandas/tests/series/test_api.py

@@ -345,7 +345,7 @@ def test_ndarray_compat(self):
                          index=date_range('1/1/2000', periods=1000))
 
         def f(x):
-            return x[x.argmax()]
+            return x[x.idxmax()]
 
         result = tsdf.apply(f)
         expected = tsdf.max()

pandas/tests/sparse/test_series.py

@@ -1379,11 +1379,21 @@ def test_numpy_func_call(self):
         # numpy passes in 'axis=None' or `axis=-1'
         funcs = ['sum', 'cumsum', 'var', 'mean',
                  'prod', 'cumprod', 'std', 'argsort',
-                 'argmin', 'argmax', 'min', 'max']
+                 'min', 'max']
         for func in funcs:
             for series in ('bseries', 'zbseries'):
                 getattr(np, func)(getattr(self, series))
 
+    def test_deprecated_numpy_func_call(self):
+        # NOTE: These should be add to the 'test_numpy_func_call' test above
+        # once the behavior of argmin/argmax is corrected.
+        funcs = ['argmin', 'argmax']
+        for func in funcs:
+            for series in ('bseries', 'zbseries'):
+                with tm.assert_produces_warning(FutureWarning,
+                                                check_stacklevel=False):
+                    getattr(np, func)(getattr(self, series))
+
 
 @pytest.mark.parametrize(
     'datetime_type', (np.datetime64,

pandas/util/_decorators.py

@@ -7,7 +7,7 @@
 
 
 def deprecate(name, alternative, alt_name=None, klass=None,
-              stacklevel=2):
+              stacklevel=2, msg=None):
     """
     Return a new function that emits a deprecation warning on use.
 
@@ -21,14 +21,16 @@ def deprecate(name, alternative, alt_name=None, klass=None,
         Name to use in preference of alternative.__name__
     klass : Warning, default FutureWarning
     stacklevel : int, default 2
+    msg : str
+          The message to display in the warning.
+          Default is '{name} is deprecated. Use {alt_name} instead.'
     """
 
     alt_name = alt_name or alternative.__name__
     klass = klass or FutureWarning
+    msg = msg or "{} is deprecated. Use {} instead".format(name, alt_name)
 
     def wrapper(*args, **kwargs):
-        msg = "{name} is deprecated. Use {alt_name} instead".format(
-            name=name, alt_name=alt_name)
         warnings.warn(msg, klass, stacklevel=stacklevel)
         return alternative(*args, **kwargs)
     return wrapper