DOC: Improve docstring for pandas.Index.repeat

[alysivji]

String assigned to ChiPy (Chicago Python Users Group) chapter of the Worldwide Documentation Sprint

While Index.repeat does accept args and kwargs, it does not pass them through to the underlying numpy function.

The only additional parameter the numpy function takes is axis (numpy docs), which is not supported by pandas:

>>> pd.Index([1, 2, 3]).repeat(2, axis=2)
ValueError: the 'axis' parameter is not supported in the pandas implementation of repeat()

Should the *args and **kwargs be removed?

Thanks

[jorisvandenbossche]

Should the *args and **kwargs be removed?

No, in this case we need to keep them for compatibility with numpy (long story short: if you do np.repeat(idx), it will call idx.repeat but also pass the axis kwarg, which we catch with the **kwargs)

[jorisvandenbossche]

This was already like this, but now we are updating the docstring: I think it is better to change this to numpy.repeat (as the docstring of numpy.ndarray.repeat also refers to that one)

[alysivji]

Fixed!

[datapythonista]

Nice PR, added some comments.

[datapythonista]

I think it should be useful to have an extended summary explaining why this method is useful. I mean, in which real cases this method could be used. I'm not sure myself, I don't think I've ever used it, but I think it should be very valuable to let users quickly understand whether this was the method they were looking for or not.

Also, the extended summary could explain how they are repeated. In the examples it's shown that they are repeated like A, A, B, B and not A, B, A, B. And I think it'd be nice to also have a bit more information about it.

[alysivji]

@datapythonista how does this look? Definitely contrived, but I don't really see this function being that useful. Series.repeat on the other hand...

[jorisvandenbossche]

Hmm, as you say yourself it is rather contrived.
If we don't find a good example, I would rather leave it out than adding in one that is a bit strange.

The main reason this exists is because of numpy compatibility (Index was previously a numpy array subclass).
I think the method can be useful, but in very specific cases where you need something like that, too specific for the docstring. I think showing a generic example from which it is clear what it does (but without a why) is good enough in this case (like your first example).

[alysivji]

Sounds good. Will change and send another commit.

[datapythonista]

@jorisvandenbossche, It's just a personal opinion, but if I ever end up in the doc of this method, I'd love to find something like exactly what you said, The main reason this exists is because of numpy compatibility (Index was previously a numpy array subclass)..

May be it's just me, but if after reading the documentation I still don't get what a function is for, and in which cases it should be useful for me, I think the documentation is not doing a good job. If a function still exists for legacy/compatibility reasons, I personally find it very useful to let the users know.

[jorisvandenbossche]

May be it's just me, but if after reading the documentation I still don't get what a function is for, and in which cases it should be useful for me, I think the documentation is not doing a good job.

I think with the current example, it should be clear what it does, but I agree the why I would use it is less clear.

If a function still exists for legacy/compatibility reasons, I personally find it very useful to let the users know.

That's the orginal reason it is there, but nonetheless it has probably a use case (in contrast with some other methods/attributes like Series.imag which are really only there for historical reasons, but those are also not listed in api.rst).
I know I already used it (or numpy's couterpart, not sure), but it was probably a little part of a complex analysis flow and can't remember exactly why I used it. And anyhow without the full context, it might not make much sense or be difficult to understand.

I understand your point, and it would indeed be good to include a good example illustrating a real-world use case, but then we need to come up with one ..

[alysivji]

How does this docstring look? Anything I need to change?

[datapythonista]

Some information on why numpy.repeat is in the See Also, and differences among them if they exist would be useful.

Also, that's more an open question, but could make sense to list here something like Index.is_unique for example? Probably if we describe better real use-cases, we'll find other related methods.

[datapythonista]

If I'm not wrong, "See Also" should have capital "A"

[jorisvandenbossche]

Yes, that is correct (although that numpydoc will still recognize it correctly, as it tries to capitalize the words if they are not)

[datapythonista]

There is still some discussion on whether we want to document args and kwargs, but I think we should. If they are ignored, I think it'd be useful for users (and also developers) to know it, and why they are present. If they are being sent to numpy, that's also something worth mentioning.

[jreback]

these are for numpy compat (only a small set of methods are like this). see the validate_repeat below.

[jorisvandenbossche]

In this case the *kwargs are not ignored, but you get an informative error message if you try to pass any additional keyword (also in case of the numpy ones like axis). So therefore I would personally not document them.

[jreback]

I would agree that mentioned **kwargs are for numpy compat is not a bad idea

[jorisvandenbossche]

I thought it was only for the nice error message, but indeed it is also accepting (not raising) the argument when it is the default value (in this case axis=None).
In that case, it's fine for me as well to actually document this

[codecov]

Codecov Report

Merging #19985 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #19985      +/-   ##
==========================================
+ Coverage   91.68%   91.69%   +0.01%     
==========================================
  Files         150      150              
  Lines       49065    49112      +47     
==========================================
+ Hits        44984    45033      +49     
+ Misses       4081     4079       -2

Flag	Coverage Δ
#multiple	90.07% <ø> (+0.01%)	⬆️
#single	41.86% <ø> (-0.02%)	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/base.py	96.66% <ø> (ø)	⬆️
pandas/core/indexes/period.py	92.57% <0%> (-2.08%)	⬇️
pandas/plotting/_converter.py	65.07% <0%> (-1.88%)	⬇️
pandas/util/_test_decorators.py	91.89% <0%> (-0.54%)	⬇️
pandas/core/indexes/timedeltas.py	91.03% <0%> (-0.15%)	⬇️
pandas/core/indexes/datetimelike.py	96.7% <0%> (-0.13%)	⬇️
pandas/core/generic.py	95.84% <0%> (-0.05%)	⬇️
pandas/core/window.py	96.31% <0%> (-0.01%)	⬇️
pandas/core/indexes/datetimes.py	95.65% <0%> (-0.01%)	⬇️
pandas/plotting/_core.py	82.23% <0%> (ø)	⬆️
... and 6 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 607910b...8486969. Read the comment docs.

[jorisvandenbossche]

@alysivji For me this one looks good!
But let's leave it open for a little while longer to give other people the chance to comment.

[datapythonista]

As said in another thread, I'm more of the opinion that documenting *args and **kwargs would add value even to simply say that they are there for compatibility and shouldn't be used. But let's leave them out if the rest agree on that.

Anyway, looks really great, thank you for it!

[jorisvandenbossche]

So summary: we add kwargs to the Parameters section, with the message "Additional keywords have no effect but might be accepted for compatibility with numpy."

Feedback on the exact message?

[dukebody]

I'd say "Additional keywords have no effect but are be accepted for compatibility with numpy." Note the "are" vs "might be". I think the "axis" case is an edge one.

[jorisvandenbossche]

The reason I used "might be" instead of "are", is because only some keyword (and with some values) are ignored silently, others still raise as normal (in this case is actually is only axis=None that is ignored, any other keyword or other value for axis will raise an error).
But, that's also not directly clear from the "might be" :)

[datapythonista]

Looks good, just couple of things that are wrong in the script.

[datapythonista]

I think we have a bug in the validate_docstrings.py script, but I don't think we want to have type for kwargs as it's always a dict of {str: anything}. Also the name should be **kwargs, that's definitely a bug in the script, that has been reported before.

Not sure if I'm missing something, but why don't we have *args in the docstrings?

[alysivji]

Will fix to use **kwargs.

I think it's confusing to use args.

idx = pd.Index([1, 2]).repeat(2, 1) gives the following error:
ValueError: the 'axis' parameter is not supported in the pandas implementation of repeat().

You'd have to go digging into the numpy implementation to see what this whole axis thing is about.

[jorisvandenbossche]

+1 for using **kwargs instead of kwargs (sphinx does not seem to complain about it, that was what I first feared)

[dukebody]

I'm now confused @datapythonista . Should we use kwargs or **kwargs as the argument name in the docstring? Because in the mailing list you said kwargs is more common.

[jorisvandenbossche]

I'll answer on the mailing list. Let's go for **kwargs, and I am trying to fix the validation script: #20072

[datapythonista]

May be we could have the is_unique methods? Just an idea, not sure if it's so related...

[jorisvandenbossche]

Thanks for the updates, and for the nice docstring!