DOC: fix PR02 errors in docstrings - pandas.core.groupby.SeriesGroupBy.idxmax,  pandas.core.groupby.SeriesGroupBy.idxmin
State: closed

All PR02 Errors resolved in the following cases:

1. scripts/validate_docstrings.py --format=actions --errors=PR02 pandas.core.groupby.SeriesGroupBy.idxmax 
2. scripts/validate_docstrings.py --format=actions --errors=PR02 pandas.core.groupby.SeriesGroupBy.idxmin 

OUTPUT:

1. scripts/validate_docstrings.py --format=actions --errors=PR02 pandas.core.groupby.SeriesGroupBy.idxmax 
```
################################################################################
################################## Validation ##################################
################################################################################

Docstring for "pandas.core.groupby.SeriesGroupBy.idxmax" correct. :)
```

2. scripts/validate_docstrings.py --format=actions --errors=PR02 pandas.core.groupby.SeriesGroupBy.idxmin 
```
################################################################################
################################## Validation ##################################
################################################################################

Docstring for "pandas.core.groupby.SeriesGroupBy.idxmin" correct. :)
```


- [x] xref https://github.com/pandas-dev/pandas/issues/57111
- [x] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [x] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [ ] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [ ] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.


Labels:
name: Docs
name: Groupby

Comments:
What's the reason to stop reusing the docstring here? Seems like the parameters are the same, no?
@datapythonista the reason I removed  `@doc(Series.idxmin.__doc__)` and  `@doc(Series.idxmax.__doc__)` is that `Series.idxmin` and `Series.idxmax`  have extra parameters `*args` and `**kwargs` that were causing the PR02 errors when the same docstring was used for `SeriesGroupBy.idxmin` and `SeriesGroupBy.idxmax` (as these do not use `*args` and `**kwargs`). 

I am open to other suggestions for resolving the PR02 errors if you know of other ways I can resolve this. 
@mroeschke can you take a look at the examples I added? 
> the reason I removed `@doc(Series.idxmin.__doc__)` and `@doc(Series.idxmax.__doc__)` is that `Series.idxmin` and `Series.idxmax` have extra parameters `*args` and `**kwargs`

I see. Sorry, I compared the one changed here with the one in `DataFrame` and they have the same signature. I see it's the one with `Series` that has a different signature.

The way we've been dealing with this is using variables for the parts that change. In this case you could create a new variable `extra_args` which is empty in one case, and contains the docstring part of `*args` and `**kwargs` when needed. It's difficult to tell at what point reusing docstrings make things easier or more difficult, but for this case it may make sense if we are already reusing the docstrings.
IMO I am generally in favor of moving away from dynamic docstring. Especially since docstrings are a good avenue for new contributions, I think the overhead of `doc/Appender/Substitution` can make fixing docstrings more onerous. Also templating can get in the way of automatic docstring formatting that ruff now provides: https://github.com/pandas-dev/pandas/pull/56863
> IMO I am generally in favor of moving away from dynamic docstring

+1
> It's difficult to tell at what point reusing docstrings make things easier or more difficult, but for this case it may make sense if we are already reusing the docstrings.

You would also have to do something about the different examples, as now the groupby version has groupby-specific examples.
@datapythonista considering the input above, is the effort to refactor this (main docstring + additional args + examples) worth undertaking at this time, or do we want to revisit that in a separate conversation outside of the scope of this PR? 

There are other similar PR02 errors that could be resolved with either approach. 
I was just pointing out at how we've been doing things so far. And as I said, I really don't know when reusing docstrings simplify things, and when it makes them more complex, it's probably more a personal preference than something we can agree with, and I don't have a strong opinion. So, I'm surely happy to let you decide how to move forward here.
@jordan-d-murphy - when you merge main, you can remove any references to `axis` in your docstring as this argument has been removed.
Thank you all so much for the guidance and feedback. For now, I'll refrain from refactoring the docstring into the dynamic approach. I've merged main back into this branch to resolve conflicts, and the pipeline is running again. I also removed the `axis` references. Can someone take another look once the pipeline finishes and the checks are green?
@rhshadrach in this PR I've removed axis for `SeriesGroupBy.idxmin` and `SeriesGroupBy.idxmax`

I see `Series.idxmin` and `Series.idxmax` also have references to `axis`, should I remove those as well? I can update this PR with that change or open a separate PR for that fix if it's needed. 
@rhshadrach can you give another look at this when you get time? I've addressed the changes you requested 
Thanks @jordan-d-murphy!