DOC: Updated kurt docstring (for pandas sprint)

[WillAyd]

Assigned docstring for pandas documentation sprint

Please note that I also removed **kwargs from the call signature here as they were not being used, so I thought it would be strange to document. For what it's worth I started a discussion on the Google groups list to see if that's the best way to handle or if we wanted to go a different path

[jorisvandenbossche]

Thanks! Added some comments

[jorisvandenbossche]

I know this is copied from the DataFrame/Series one, but I personally don't find "kurtosis of normal == 0.0" very clear. I suppose it is about a normal distribution having a kurtosis of 0.0 ?

[jorisvandenbossche]

Again, I know this was already there ..:) But I also find this not very clear. What is the "input"? As I didn't provide anything to the function .. (it's rather the object calling rolling)

But not directly sure how to document this very well. Maybe just "Series/DataFrame" (depending on the calling object)

[WillAyd]

So when I did this originally I had:

Series or DataFrame
    Like-indexed object containing the result of the kurt operation

However, I ended up with the above because all of the other rolling / expanding functions used this in the _doc_template variable (linked below). We could either break that consistency or update the _doc_template variable to reflect the above (perhaps substituting kurt for %(name)) if we think that's a better generally applicable description

pandas/pandas/core/window.py

Line 51 in aedbd94

same type as input

[jorisvandenbossche]

I think we should break the consistency for now, with the idea that if all those docstrings get updated during the sprint, we should strive again for consistency with the new wording?

Or indeed already update the template, then it is changed for all as well.

In any case I think we should try to make the docstring that you are now editing, as good as possible.

[jorisvandenbossche]

period at and

[jorisvandenbossche]

I think by using dedent, you can actually keep this indented (so it doesn't look that 'off' inside the class)

(I am not even sure the dedent is needed, as there are many places where we don't use it, but that is something else)

[WillAyd]

Yep mistake on initial push. Subsequently indented in the next push

[jorisvandenbossche]

Can you put a small explanation? Very dummy saying what the example is doing

[datapythonista]

Really nice, added some comments with ideas.

[datapythonista]

I think it could make sense to add .skew too as statistically they are related, and usually appear together in literature. And technically speaking the mean and the variance could also belong here, as they are the first and second moments of a probability density (while skewness and kurtosis are the third and forth).

Also, it's probably worth to add pandas.Series.rolling. And not sure if in the "See Also" or somewhere else, but I think it would be very useful for the users to have a link from these functions to https://pandas.pydata.org/pandas-docs/stable/computation.html#window-functions

[WillAyd]

Yea I was thinking the same thing about pandas.Series.rolling and pandas.Series.expanding. Right now there is the _doc_template contained within the module that gives a generic See Also section, but my first attempt to override that didn't work out.

I'll look a little closer to see if there's a scalable way to have all of the functions here reference pandas.Series.rolling and the Series / DataFrame methods at a minimum, while allowing for additional "see also" to be specified per-method

[datapythonista]

Nice, did you find that the documentation about the "Notes" section made sense for this case?

[WillAyd]

I added this from familiarity with the NumPy standard, not necessarily the docs that you built (although they were very helpful on many other aspects - kudos)

[datapythonista]

Probably it's obvious enough, but may be having 6 values instead of 5, would give more the idea that after the 4th NaN all the values will be filled?

[datapythonista]

It's a somehow arbitrary comment, but to me it'd look better something like:
Series or DataFrame (same as the input) : some description

Or if the %(name)s above Series or DataFrame depending on in which method is being used? Then we can use it also here.

[codecov]

Codecov Report

Merging #19999 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #19999      +/-   ##
==========================================
+ Coverage   91.67%   91.67%   +<.01%     
==========================================
  Files         150      150              
  Lines       49096    49094       -2     
==========================================
  Hits        45007    45007              
+ Misses       4089     4087       -2

Flag	Coverage Δ
#multiple	90.05% <100%> (ø)	⬆️
#single	41.87% <100%> (-0.01%)	⬇️

Impacted Files	Coverage Δ
pandas/core/window.py	96.31% <100%> (-0.01%)	⬇️
pandas/util/testing.py	83.85% <0%> (+0.2%)	⬆️

---

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 e02f737...dd61858. Read the comment docs.

[WillAyd]

So based on both of your feedback I'm wondering if we should update the _doc_template in this module to look as follows:

_doc_template = """

Returns
-------
Series or DataFrame (matches input)
    Like-indexed object containing the result of function application

See also
--------
pandas.Series.%(name)s
pandas.DataFrame.%(name)s
%(other_see_alsos)
"""

This would ensure that we consistently always have pandas.Series.rolling and pandas.DataFrame.rolling as part of every function, but also give it the flexibility to add more references.

The downside here is at the moment I don't think that there's a way to add %(other_see_alsos) without explicitly wrapping all of the other methods in the module with a substitution that provides it. Alternately we could update the Substitution class to allow for optional keyword specification, but I'd worry about a lack of explicitness there.

Anyway wanted to get your feedback before going further - do you think the above template is a good idea and if so if it's worth me going and updating all of the @Substitution decorators in this module to contain other_see_also="" prior to the sprint?

[jorisvandenbossche]

@WillAyd I'm fine with both options (only updating this docstring here, or updating the template as well).

The question is a bit how well the template will scale if all docstrings have a custom extended summary, examples, notes etc, only to share the 'returns' and 'see also' section

[jreback]

thanks!

[jorisvandenbossche]

I still think the "matches input" is confusing. What is the input? I would think of input what I passed to the function, but that is not the case here.
Personally I would just leave it out. Or we should find a better wording (maybe "matches calling object", although I think that is also not very clear). Or explain it in more words in the explanation on the line below.

[WillAyd]

What do you think of "Returned object type is determined by the caller of the %(name)s calculation"

[jorisvandenbossche]

Yes, that sounds good

[jorisvandenbossche]

Is this to satisfy the doctests?
If so, we should try to find another solution, as I think this makes it more complex to read (compared to simply scipy.stats.kurtosis(arr[:-1], bias=False))

[WillAyd]

Fair enough. Let me mess around with iPython - I think we can use the %precision directive here behind the scenes

[jorisvandenbossche]

Hmm, the annoying thing here is that scipy prints 1.99999999999, as otherwise 1.200... would have worked (using an ellipsis to match anything on that line)

Yeah, not sure there is a better solution in this case

[WillAyd]

Yea I was hoping that setting np.print_options would help for SciPy but no luck. I kind of wonder if globally it wouldn't be preferable to just set this to 6 across all docstrings for consistency

[datapythonista]

+1 on setting it globally

[WillAyd]

Looked into it and I think that would unfortunately be pretty difficult for scalar values. np.set_printoptions may be worthwhile for arrays, but as far as this specific example is concerned the formatting of scalar values from what I can tell is left to interpreters like iPython. I looked at DocTests' documentation but didn't see anything there that would easily allow us to enforce that option during its testing

[jorisvandenbossche]

Maybe a Series is more natural? (as you don't have a column name)

[jorisvandenbossche]

Is the one with %(name)s substituted not the same as the two lines above? (below the substituted ones)

[WillAyd]

The substituted name in this case is either rolling or expanding - does that answer your question?

[jorisvandenbossche]

Ah, yes, that clarifies.
Another point, to avoid that the See also section becomes long, we could also put the related series/dataframe versions on a single line (like Series.kurtosis, DataFrame.kurtosis)

[WillAyd]

When you are worried about it becoming to long is that purely within the source or on the web as well? I believe that the HTML output styles this to display it all inline

[jorisvandenbossche]

Ah, didn't know the html output put them on a single line anyhow. Not ideal IMO, but that's numpydoc behaviour.

[jorisvandenbossche]

@jreback please wait a bit with merging the 'test' PRs for the sprint for now. Let's leave time to comment on them.

[jreback]

np

just wanted to clear backlog

i wont merge doc prs then u less u approve

[jorisvandenbossche]

i wont merge doc prs then u less u approve

It's also that for the test PRs now want to keep them open a bit so other organizers can look and review.

[jorisvandenbossche]

@WillAyd feel free to open a new PR for my remaining comments