CLN: Replace Appender and Substitution with simpler doc decorator
State: closed

- [X] closes #30933 
- [x] tests added / passed
- [X] passes `black pandas`
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

A new decorator to handle docstring formatting. There is also an update for an existing case to show how it works.

Labels:
name: Docs
name: Clean

Comments:
@pandas-dev/pandas-core I think this should simplify significantly the reuse of docstrings. Thoughts?
Can you a bit more specific about what exactly the proposal is? 

Looking at the code, it seems to combine the Appender and Substitution decorators into a single decorator (certainly a good idea!), but for the rest works more or less the same?
> Can you a bit more specific about what exactly the proposal is?
> 
> Looking at the code, it seems to combine the Appender and Substitution decorators into a single decorator (certainly a good idea!), but for the rest works more or less the same?

Yes, I think you have summed up this proposal very well. The idea of this is to create a simple solution to re-use docstring. It combined `Appender` and `Substitution` into one decorator and retain most of their features. 

One additional improvement from this approach: it can take docstring (unrendered) as a template too. This gives us the ability to put docstring under function directly, instead of saving them as a global variable somewhere else. This might help us manage docstring more easily and more conveniently.
> One additional improvement from this approach: it can take docstring (unrendered) as a template too. This gives us the ability to put docstring under function directly, instead of saving them as a global variable somewhere else.

That's something we already started doing (but only a bit) with the current Appender as well (see https://dev.pandas.io/docs/development/contributing_docstring.html#sharing-docstrings, and eg https://github.com/pandas-dev/pandas/blob/master/pandas/core/series.py#L787 in practice), if I understand you correctly. Anyway, I also think this is better than the `shared_docs` variables, so if this decorator makes that even a bit more simple, all the better! 
@HH-MWB some other relevant discussion: https://github.com/pandas-dev/pandas/issues/19932 (I had a PR doing part of that, but never finished https://github.com/pandas-dev/pandas/pull/20022)
@HH-MWB since I think we all like this, I think you can add:
- Couple more cases as @WillAyd said
- Unit tests for the decorator
- Documentation about the new decorator

Good point about @jorisvandenbossche regarding `@doc(pandas.Series.head.__doc__)` vs `@doc(pandas.Series.head)`. I have a small preference for not having to write `__doc__`, and make the code a bit cleaner when using the decorator, but happy with both options.

Thanks a lot for working on this, I think it'll be a significant improvement. We're using `Appender` 312 times. :)
> Documentation about the new decorator

We already document the sharing of docstrings: https://dev.pandas.io/docs/development/contributing_docstring.html#sharing-docstrings. So should we just update that documentation already to show this method? 

> Good point about @jorisvandenbossche regarding `@doc(pandas.Series.head.__doc__)` vs `@doc(pandas.Series.head)`. I have a small preference for not having to write `__doc__`

To be clear, I am also in favor of not having to write `__doc__`. I mainly wanted to point out that not having to put the docstring in a shared variable was already possible (and done). 
> We're using Appender 312 times.

We could consider using something like [this](https://github.com/bashtage/arch/blob/master/arch/utility/array.py#L90) to automatically inherit docstrings.  (Though id rather use `__init_subclass_` than a metaclass)
Do you have an example to see how this looks like when applied? 
> To be clear, I am also in favor of not having to write `__doc__`. I mainly wanted to point out that not having to put the docstring in a shared variable was already possible (and done).

Sorry for didn't make it clear. I totally agree with you that we could use `func.__doc__` to avoid put docstring in a shared variable. 

However, in some very edge cases, I still think it going to be hard to avoid shared variables using `Appender` and `Substitution` only. For example, in [functions of "register a custom accessor"](https://github.com/pandas-dev/pandas/blob/master/pandas/core/accessor.py#L213-L316), it might be very difficult to remove `_doc` from a shared variable. If we try to put `_doc` under a function, the template will become a rendered string after `@Substitution`, which makes other functions hard to get the original docstring template.
> However, in some very edge cases, I still think it going to be hard to avoid shared variables using Appender and Substitution only. 

Yeah, I also didn't want to say we should not use it at all. Unfortunately, it's not only some edge cases where we will still need to use a shared variable. I think basically every case where there is no base class method that is always overridden (such as the `register_..` free functions you mentioned), we will still need to work with a variable that holds the docstring. 

While starting to use this better decorator throughout the code base, we should also try to clean up the cases where we are using a shared docstring with substitution where actually nothing is being substituted / or where it is never substituted. The first case leads the unneeded complexity, while the second leads to left-over template variables in the docstring (see https://github.com/pandas-dev/pandas/issues/19932 for some example from some time ago)
(sorry wrong button)
May I get some help here? I was confused about this mypy failure. The error message shows:
```
pandas/core/series.py:4105: error: Cannot determine type of 'fillna'
```

My guess is [commit c57c51f](https://github.com/pandas-dev/pandas/pull/31060/commits/c57c51f947aa07206bbb74fa58d31d147e092bae) causes this failure. I applied almost the same changes in pandas/core/series.py and pandas/core/frame.py, but one passed and one failed. 

Thanks!

@simonjayhawkins do you mind having a look at the mypy problem? 
> @simonjayhawkins do you mind having a look at the mypy problem?

looks like it may be a mypy import issue. the following produces no errors...

```diff
diff --git a/pandas/core/series.py b/pandas/core/series.py
index 8b74ec4f5..9fc18a774 100644
--- a/pandas/core/series.py
+++ b/pandas/core/series.py
@@ -70,6 +70,7 @@ from pandas.core.construction import (
     is_empty_data,
     sanitize_array,
 )
+from pandas.core.generic import NDFrame
 from pandas.core.indexers import maybe_convert_indices
 from pandas.core.indexes.accessors import CombinedDatetimelikeProperties
 from pandas.core.indexes.api import (
@@ -4102,7 +4103,7 @@ Name: Max Speed, dtype: float64
             errors=errors,
         )
 
-    @doc(generic.NDFrame.fillna, **_shared_doc_kwargs)
+    @doc(NDFrame.fillna, **_shared_doc_kwargs)
     def fillna(
         self,
         value=None,
```
> > @simonjayhawkins do you mind having a look at the mypy problem?
> 
> looks like it may be a mypy import issue. the following produces no errors...

It works! Thank you for your help. @simonjayhawkins @datapythonista 
> Really nice, looks good to me.

@datapythonista Thanks! Any further adjustment you would like to see? 
It's all good, just need someone else from the team to have a look. 
> lgtm. so plan would be to replace Appender / Substitution with this entirely in a follow up(s) right?

Yes. If everyone agrees with this update, I will be happy to keep working on this and replace `Appender` / `Substitution` with this entirely.
@HH-MWB can you merge master one more time? Should resolve CI failures

@jreback any thoughts?
Thanks @HH-MWB 
Looking forward to follow ups
> Looking forward to follow ups

@WillAyd Thank you too. I have submit a new issue for replacing with `doc` decorator [here](https://github.com/pandas-dev/pandas/issues/31942).


Also, @datapythonista @simonjayhawkins @jorisvandenbossche thanks for your help too.