DOC/CLN: Index shared docs with appending actual docstrings

[jorisvandenbossche]

xref #19932 and #20016

This removed the _index_shared_docs dict for the Index class.
There were 4 methods that actually substituted values: take, get_indexer, get_indexer_non_unique, fillna

[jorisvandenbossche]

You can see in the second commit what actually needed to be changed in the Index docstrings.

For take and fillna: I don't think it was that much of a added value that it said the specific Index subclass (in many other docstrings we just say 'Index')
For get_indexer and get_indexer_non_unique the substituted values actually contained more information (eg target_klass was PeriodIndex or list of Periods). However, this was not always correct (eg for CategoricalIndex it does not only accept CategoricalIndex, but aslo object Index/Series/array/list) or consistent (eg for PeriodIndex we do not accept an object Index of Periods (but array or list is fine), while for CategoricalIndex we do).

If we want, I can keep a template for get_indexer / get_indexer_non_unique. But personally I don't think this is worth it (and I would rather try to make it more consistent)

[jreback]

Should clarify when should be using _shared_docs, I agree in Index subclasses we should prob try not to use it as much as its cumbersome. Though it is nice to have the correct return class rather than Index as the generic return value. Its more informative.

The problem here is that we generally do want to use shared kwargs. so its confusing that in some cases we don't and other cases we do. Should have a clear cut case here in order to NOT use them. The reason is in the future folks would just not use them, defeating the purpose of having good doc-strings. We should have a single well-defined way of doing this. Sure in some simple cases it is not needed, but then you have to decide when that is.

[jorisvandenbossche]

The problem here is that we generally do want to use shared kwargs. so its confusing that in some cases we don't and other cases we do.

With this PR it can be quite clear: for Index subclasses we don't use template substitution, for Series/Index (from IndexOpsMixin) or Series/DataFrame (from NDFrame) we do use them (for some of the methods).

Of course the question is: do we want to use template substitution for Index in the future as well? Because indeed, this PR prevents that.
(note that special methods from Index subclasses still have their own docstring if they are defined in the subclass, so it is only for the generic methods that the docstrings would be Index-subclass agnostic)

[jreback]

With this PR it can be quite clear: for Index subclasses we don't use template substitution, for Series/Index (from IndexOpsMixin) or Series/DataFrame (from NDFrame) we do use them (for some of the methods).

see that's the thing. Why would we do this? it just adds to the confusion. Index/Series are very very similar, so to do it in Series but not Index raises questions of why? and then there is the special case of IndexOpsMixin.

[jorisvandenbossche]

Why would we do this? it just adds to the confusion.

There is not much difference in practice:

@Subtitution(..)
@Appender(NDFrame.method.__doc__)

vs

@Appender(Index.method.__doc__)

Anyhow, even for NDFrame, is there is actually no substitution, we would only do Appender without Substitution, so you would get such a mixture in frame.py/series.py anyway (as you have this mixture today as well for cases that use % and other not).

[jorisvandenbossche]

IMO this makes it more explicit. Now often there is % _shared_docs_kwargs without that there is actually something to replace. For me, this creates confusion. So I would personally only do % _shared_docs_kwargs / @Substitution(_shared_docs_kwargs) when there is actually something to replace in the docstring.

[jreback]

@jorisvandenbossche the problem is a new contributor will have no which one to use. we need a clear unambiguous policy. it may be obvious. e.g. if the method is only defined in base.py then don't use a template / shared, otherwise do.

[TomAugspurger]

Thoughts on phrasing this as "Return a new index of the same type ..."? I suppose "of the values" implies that you're getting the same type of index back though.

[jreback]

generally we need language to assert that we are getting back the same type of Index after the op (which is almost always true), though there are exceptions. This is technically easier to have the doc-strings render like this, e.g. PeriodIndex is pretty explicty as a return type for example.

[jreback]

closing as stale. if you'd like to continue pls ping.

[pep8speaks]

Hello @jorisvandenbossche! Thanks for updating the PR.

• There are no PEP8 issues in the file pandas/core/indexes/base.py !

• There are no PEP8 issues in the file pandas/core/indexes/category.py !

• There are no PEP8 issues in the file pandas/core/indexes/datetimelike.py !

• There are no PEP8 issues in the file pandas/core/indexes/datetimes.py !

• There are no PEP8 issues in the file pandas/core/indexes/interval.py !

• There are no PEP8 issues in the file pandas/core/indexes/multi.py !

• There are no PEP8 issues in the file pandas/core/indexes/numeric.py !

• There are no PEP8 issues in the file pandas/core/indexes/period.py !

• There are no PEP8 issues in the file pandas/core/indexes/range.py !

• There are no PEP8 issues in the file pandas/core/indexes/timedeltas.py !

[jorisvandenbossche]

I will look into rebasing this next week

[datapythonista]

@jorisvandenbossche do you still want to rebase this?

[jreback]

closing as stale, but reopen if can continue