DOC: Fix errors in pandas.Series.argmin
State: closed

- [x] closes https://github.com/pandanistas/pandanistas_sprint_jakarta2020/issues/19
- [ ] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

output of `python scripts/validate_docstrings.py pandas.Series.argmin`:
```
################################################################################
################################## Validation ##################################
################################################################################
```



Labels:
name: Docs

Comments:
Thanks @farhanreynaldo can you merge master. failing CI should be fixed.
@datapythonista should we be sharing the bulk of the docstring with Series.argmax, xref #32019
> @datapythonista should we be sharing the bulk of the docstring with Series.argmax, xref #32019

Yes, I think that makes sense. @farhanreynaldo the idea here is instead of repeating the same in both docstrings, to reuse from one to the other. We have a `@doc` decorator to help with that. The general idea if I'm not wrong of the exact syntax is:

```python
@doc(operation='maximum')
def argmax():
    """
    This is computing the {operation}.
    """
    pass

@doc(argmax, operation='minimum')
def argmin():
    pass
```

The `@doc` decorator was recently introduced in #31060, the diff of that PR can help you understand. If you have any question, or you have any idea for improvement given this use case, feel free to ping @HH-MWB 
> 
> 
> > @datapythonista should we be sharing the bulk of the docstring with Series.argmax, xref #32019
> 
> Yes, I think that makes sense. @farhanreynaldo the idea here is instead of repeating the same in both docstrings, to reuse from one to the other. We have a `@doc` decorator to help with that. The general idea if I'm not wrong of the exact syntax is:
> 
> ```python
> @doc(operation='maximum')
> def argmax():
>     """
>     This is computing the {operation}.
>     """
>     pass
> 
> @doc(argmax, operation='minimum')
> def argmin():
>     pass
> ```
> 
> The `@doc` decorator was recently introduced in #31060, the diff of that PR can help you understand. If you have any question, or you have any idea for improvement given this use case, feel free to ping @HH-MWB

Sure, let me figure it out first.
@datapythonista should I do another pull request regarding the changes for `pandas.Series.argmax` doc?
> @datapythonista should I do another pull request regarding the changes for `pandas.Series.argmax` doc?

I think a single PR with the changes to reuse the docstring is what makes more sense. Whether you reuse this PR, or you close this an open a new one doesn't make a difference for us.
> 
> 
> Thanks @farhanreynaldo for the updates.
> 
> a couple of ci failues.. you'll need to run black, https://pandas.io/docs/development/contributing.html#python-pep8-black
> the also the doc decorator parameters should be strings.

Sure, I will run `black pandas`
@farhanreynaldo can you merge master to resolve conflicts
@farhanreynaldo can you run black to fix ci. ping on green.
> 
> 
> Great job @farhanreynaldo
> 
> For the examples, we would usually have both the minimum and the maximum in both examples, which may be easier and it makes sense. And even for the see also section we often have both (even if one is self-referencing the current page).
> 
> But looks good like this too.

Yeah, I think it would be nice to put both examples in the documentation, but then we should add another parameter for `example values` in doc arguments. Is that okay?
> 
> 
> @farhanreynaldo can you run black to fix ci. ping on green.

@simonjayhawkins all green now.
The idea is not to add another parameter but to remove the ones you have now for the examples. If you have the max first and the min later you don't need any variable.
> 
> 
> The idea is not to add another parameter but to remove the ones you have now for the examples. If you have the max first and the min later you don't need any variable.

Oh I see, we could reduce the parameter. Is this what you are looking for?
```
See Also
--------
Series.arg{op} : Return position of the {op}imum value.
numpy.ndarray.arg{op} : Equivalent method for numpy arrays.
Series.arg{oppose} : Similar method, but returning the {oppose}imum.
Series.idxmax : Return index label of the maximum values.
Series.idxmin : Return index label of the minimum values.
```

and for the examples
```
Examples
--------
Consider dataset containing cereal calories

>>> s = pd.Series({{'Corn Flakes': 100.0, 'Almond Delight': 110.0,
...                'Cinnamon Toast Crunch': 120.0, 'Cocoa Puff': 110.0}})
>>> s
Corn Flakes              100.0
Almond Delight           110.0
Cinnamon Toast Crunch    120.0
Cocoa Puff               110.0
dtype: float64

>>> s.argmax()
2

>>> s.argmin()
0

The maximum cereal calories is the third element and 
the minimum cereal calories is the first element,
since series is zero-indexed. 
```
Exactly, that's what I meant. 
And for see also you could also repeat argmin and argmax and avoid the variables. 
> 
> 
> And for see also you could also repeat argmin and argmax and avoid the variables.

Could we keep the variables in the `See Also` section so the order of the reference consistent with its function? 
e.g `pandas.Series.argmin` documentation would have its function appear first in the `See also` section
I'm ok with that. There is a trade off between keeping things simple in the code, and having each docstring with its own links, examples... In general I think it's better to keep things simpler for us if the docstrings are useful, but not really important in this case, I think any option is good. 
> 
> 
> I'm ok with that. There is a trade off between keeping things simple in the code, and having each docstring with its own links, examples... In general I think it's better to keep things simpler for us if the docstrings are useful, but not really important in this case, I think any option is good.

I think I'm just gonna follow your lead.
Thanks for the fixes. The error in the CI is caused by a new version of Matplotlib being released, and will be fixed after #32444 is merged.
> 
> 
> Thanks for the fixes. The error in the CI is caused by a new version of Matplotlib being released, and will be fixed after #32444 is merged.

Thank you for the input throughout this pull request, @datapythonista and @simonjayhawkins!
If you can merge master, CI should be green now.
@simonjayhawkins do you want to have another look? this should be ready now
I think we should merge this. @farhanreynaldo looks like GitHub deployed some changes last night, were they consider (in the git history) that the person merging the PR is the author, and not you :(

See #32457 for the full discussion.
Actually I'm okay with that
Thanks for working on this @farhanreynaldo