API: .argmax should be positional, not an alias for idxmax

[TomAugspurger]

In #6214 it was reported that argmax changed (I think since Series didn't subclass ndarray anymore). It's sometimes useful to get the index position of the max, so it'd be nice if .argmax did that, leaving idxmax to always be label-based.

In [1]: import pandas as pd
pd.Ser
In [2]: s = pd.Series([1, 2, 3, 0], index=['a', 'b', 'c', 'd'])

In [3]: s.idxmax()
Out[3]: 'c'

In [4]: s.argmax()
Out[4]: 'c'

I would expect Out[4] to be 2. The current workaround is np.argmax(x), which is OK, but doesn't generalize to DataFrames well.

We could deprecate .argmax in 0.21, pointing users to .idxmax. Then later we can change .argmax to be always be positional.

---

Update: We've deprecated .argmax/min. Need to change the behavior next.

[lphk92]

I am working on this at the SciPy 2017 sprints

[rgommers]

np.argmax(some_series) does the correct thing right now (and did in the past, and will remain correct). But it gives:

\site-packages\numpy\core\fromnumeric.py:52: FutureWarning: '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.
  return getattr(obj, method)(*args, **kwds)

The deprecation warning appears to be for np.argmax which is confusing. So two suggestions:

• amend the message to say pd.Series.argmax is ...
• don't use the word "deprecated" - Series.argmax is not deprecated, it will just change behavior (hence FutureWarning). Deprecation normally means that it will be removed.

And a possible more involved change for this orpossibly for future related warnings: check whether the deprecated method is called by numpy's _wrapfunc rather than by the user directly, and avoid giving a warning if that is the case and the user code is and remains correct.

[jorisvandenbossche]

np.argmax(some_series) does the correct thing right now (and did in the past, and will remain correct)

I don't think it does? Eg

In [8]: np.argmax(pd.Series([1, 2, 3], index=['a', 'b', 'c']))
/home/joris/miniconda3/envs/dev/lib/python3.5/site-packages/numpy/core/fromnumeric.py:57: FutureWarning: '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 row.
  return getattr(obj, method)(*args, **kwds)
Out[8]: 'c'

In [9]: np.argmax(pd.Series([1, 2, 3], index=['a', 'b', 'c']).values)
Out[9]: 2

So it gives 'c', while it should give 2.

But I certainly agree that it is confusing that it seems that np.argmax is deprecated (not sure if we can solve this apart from the wording, as this has to do with python stacklevel printing), and that the wording can be improved (+1 on your two suggestions).

check whether the deprecated method is called by numpy's _wrapfunc rather than by the user directly, and avoid giving a warning if that is the case and the user code is and remains correct.

Is that possible? (how to do it exactly?)
Because that would certainly be nice, also in some other cases. For example in #21930 (comment) where we deprecated Series.compress (to remove it eventually) which also raises a warning for np.compress(s) (in this case that was actually OK, as it will not work anymore in the future, but I can imagine similar cases).

[jorisvandenbossche]

I suppose it is easy to do with inspecting the stack. Is it guaranteed to always be _wrapfunc , or should we rather check in general if the call is coming from the numpy module?

[jorisvandenbossche]

@rgommers see #22310 for edit to the deprecation message

[rgommers]

So it gives 'c', while it should give 2.

You're right. My code does:

x = pd.Series([1, 2, 3], index=['a', 'b', 'c'])
x[np.argmax(x)]

I think this is normal though - the majority of users probably won't look at the actual value of the index.

Is it guaranteed to always be _wrapfunc, or should we rather check in general if the call is coming from the numpy module?

_wrapfunc probably won't change, but it's private so no hard guarantee. These are the functions that use _wrapfunc:

• take
• reshape
• choose
• repeat
• swapaxes
• transpose
• argpartition
• argsort
• argmax
• argmin
• searchsorted
• nonzero
• compress
• clip
• cumsum
• cumprod
• round

So that's not all - there are functions like np.sum that implement the logic themselves:

if type(a) is not mu.ndarray:
    try:
        sum = a.sum
    except AttributeError:
        pass
    else:
        return sum(axis=axis, dtype=dtype, out=out, **kwargs)

Indeed checking that the caller is a numpy object is probably best. But having thought about it, there's probably some way to break that, e.g. by a user wrapping a call to pandas in np.vectorize. Probably best to ignore my suggestion, not worth the trouble.

@rgommers see #22310 for edit to the deprecation message

Thanks

[jorisvandenbossche]

You're right. My code does:

       x = pd.Series([1, 2, 3], index=['a', 'b', 'c'])
       x[np.argmax(x)]

I think this is normal though - the majority of users probably won't look at the actual value of the index.

That's a good point. In case you have a non-numerical index like in the small example above, the x[np.argmax(x)] will keep working in the future (when argmax will return a positional integer), but with numerical indexes, this will not be guaranteed to give the same result. So therefore, if you have this deprecation message, you are still encouraged to switch to idxmax.
Which of course makes it more difficult to write code that works for both a Series and numpy 1D-array, but as long as [] mixes positional (numpy-like) and label-based indexing, there is no way around this complexity.

[wesm]

I just ran into this problem when working through errata for Python for Data Analysis. Can argmax/argmin return the positional values in 0.24.0?

[jorisvandenbossche]

Generally, our rule is to keep deprecations two additional feature releases (after the first release having the warning), and only remove/change it in the release after that. For argmax/argmin, that would mean to only change it in 0.25/1.0.

@wesm the reason you ask is because you actually want to get the positional value? (and don't want to resort to np.argmin(np.array(values)) that would only be needed for one additional release?

[wesm]

Correct, the positional value. That's what it did before the cited refactoring caused index labels to be returned

[jbrockmendel]

@TomAugspurger is this closed by #30113 and #16955?

[jorisvandenbossche]

Yes, the argmax method and np.argmax now work positionally.