CLN: removed unused parameter 'out' from Series.idxmin/idxmax
State: closed

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `git diff upstream/master | flake8 --diff`
- [ ] whatsnew entry

It seems misleading to have the `out` parameter in the function signature when it is not used:

```
pd.Series.argmin(self, axis=None, out=None, skipna=True)
pd.Series.argmax(self, axis=None, out=None, skipna=True)
```

(unlike the `numpy.argmin/argmax` methods where `out` can be used)


Labels:
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
## [Current coverage](https://codecov.io/github/pydata/pandas?ref=4bff73995bfd73291007980b78b30108ae32c964) is `88.64%`

> Merging **#12638** into **master** will not affect coverage as of [`4bff739`](https://codecov.io/github/pydata/pandas/commit/4bff73995bfd73291007980b78b30108ae32c964)

``` diff
@@            master   #12638   diff @@
=======================================
  Files          275      275       
  Stmts       133688   133688       
  Branches         0        0       
  Methods          0        0       
=======================================
+ Hit         118511   118512     +1
  Partial          0        0       
+ Missed       15177    15176     -1
```

> Review entire [Coverage Diff](https://codecov.io/github/pydata/pandas/compare/f71537ab2561ab5727008095e9685966619fa7b9...4bff73995bfd73291007980b78b30108ae32c964) as of [`4bff739`](https://codecov.io/github/pydata/pandas/commit/4bff73995bfd73291007980b78b30108ae32c964)
> 
> Powered by [Codecov](https://codecov.io). Updated on successful CI builds.

this is the same issue as being fixed in #12603. The underlying PR is almost there, so need to recast this similary.

cc @gfyoung 

now that #12603 is merged, this should be quite trivial.

@jreback one problem is that `numpy` actually passes both `axis` and `out` as positional arguments 
https://github.com/numpy/numpy/blob/master/numpy/core/fromnumeric.py#L1036 

``` python
return argmin(axis, out)
```

so it seems like we have two choices:
- keep the current function signature but make sure `out` is `None`, i.e.

``` python
def idxmin(self, axis=None, out=None, skipna=True):
    if out is not None:
        raise ValueError(msg)
```
- change it to

``` python
def idxmin(self, axis=None, *args, skipna=True):
     validate_args(args, min_length=0, max_length=1)
```

which would be better?

@jreback : Is there any reason why we're supporting an `axis` argument when there aren't any "axes" to a `Series` object, not to mention, it's not even used in the implementation?

@mortada : Your second option doesn't work because you can't put keywords after `*args`, but a more customized solution might be needed given that the keyword we want (`skipna`) comes after the keyword(s) that we  _don't_ want.  A suitable solution I think depends on how many keywords we can remove from the signature.

@mortada can you give a concrete example of where this actually fails now?

we should hide the `out` param, but since numpy is passing it positionally, prob have to interpret if its passed as the `axis` param.

@gfyoung it makes the signatures the same for the same methods in Series/DataFrame. there is consistency across methods in pandas, that's why most are already in `generic.py`.  Series only methods still have the `axis` param again for consistency.

E.g. `s.sum(0)` makes sense, so why not `s.idxmin(0)`

In fact these routines `idxmin/idxmax`, ideally we could consolidate them and put them in `generic.py`

@jreback : By passing in arguments like this, `idxmin` will interpret `numpy`'s `out` parameter as its `skipna` parameter.  It used to be that `skipna` would just default to `True` in these cases.

Since this issue is localized to `Series`, could we just create a method that validates that `skipna` is in the tuple (`None`, `True`, `False`) and assign it to be `True` if `skipna` is `None`?

```
Signature: pd.DataFrame.idxmin(self, axis=0, skipna=True)
Signature: pd.Series.idxmin(self, axis=None, out=None, skipna=True)
```

I would like to make the signture of `Series.idxmin/max` the same as `DataFrame.idxmin/max` w/o causing issues. IF its passing by position, then this is pretty easy. (just interpret `skipna`), are there cases where its NOT passed by position (but by kwargs)? if so, may need to add a `kwargs` handler as well (like we did for stat functions)

@jreback : On the `numpy` side in `fromnumeric.py`, it internally is by position and not keyword (again, why that is the case, I'm not sure).  In that case, we could go with handling `skipna` as I proposed above.

@gfyoung yeh something that that would work

@jreback , @mortada : this PR is being subsumed by my massive PR #12810.

@gfyoung makes sense, I'll close this one, thanks!