DOC: clarify default window in rolling method #18177
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18177 +/- ##
==========================================
+ Coverage 91.4% 91.41% +<.01%
==========================================
Files 163 163
Lines 50068 50068
==========================================
+ Hits 45764 45768 +4
+ Misses 4304 4300 -4
Continue to review full report at Codecov.
|
pandas/core/window.py
Outdated
| @@ -503,6 +503,7 @@ class Window(_Window): | |||
| * ``general_gaussian`` (needs power, width) | |||
| * ``slepian`` (needs width). | |||
|
|
|||
| If ``win_type=None`` a boxcar window is used. | |||
There was a problem hiding this comment.
If we add this, I think we should explain what a boxcar window is (as now in the default case the user does not need to worry about it, and those assuming you just get a uniform hard-cut window are correct)
There was a problem hiding this comment.
Come to think of it, none of the other windows are explained either! Are the windows used custom pandas ones, or are they just the normal numpy ones? And if they're pandas ones do they have their own bit of documentation to link to?
There was a problem hiding this comment.
these are all scipy windows
certainly take links to that documentation (in doc string and in computation.rst)
|
I've added a link to the scipy docs - I presumed this is where all the windows come from? |
pandas/core/window.py
Outdated
| @@ -503,6 +503,9 @@ class Window(_Window): | |||
| * ``general_gaussian`` (needs power, width) | |||
| * ``slepian`` (needs width). | |||
|
|
|||
| If ``win_type=None`` a boxcar window is used. To learn more about different | |||
There was a problem hiding this comment.
this is not correct, the default is in fact win_type=None, which means your window is specified by the the window parameters; only if you actually specify a win_type do you get a specific weighted window.
There was a problem hiding this comment.
Fundamentally doing a rolling window calculation involves choosing a window, so what window does passing win_type=None choose? I thought it was a boxcar?
Is it valid to specify both the window parameters and the window type, or can only one be provided?
There was a problem hiding this comment.
most operations are an u weighted window
only if you specify win_type do you get a special weighted window (which also has only sum/mean) methods (this is not very common usage, not using win_type is by far the most common)
i think the docs in computation are pretty clear in this
certainly could add more / better examples in the doc string
There was a problem hiding this comment.
Right, so if you don't specify win_type you get an un-weighted window, which is a boxcar window, which is what my addition says.
I disagree that the docs are pretty clear, and I think this helps clear them up. (I also think it doesn't confuse anything, and I'm 99% sure what I've written is correct!)
There was a problem hiding this comment.
Yes, the current default uniform unweighted window is apparently a boxcar window (which I only learned by looking up what a boxcar window is).
I think that I am not alone in that, therefore I would change the explanation slightly to first explain what the default window is, and then only to say that this is called a boxcar window (people should certainly not need to follow a link to know what the meaning of the default is IMO)
There was a problem hiding this comment.
Ah, I think I understand the confusion now - I don't have time now, but will try and revise later.
|
Is this a bit better? |
|
seems ok to me, @jorisvandenbossche |
jorisvandenbossche
changed the title
|
@dstansby Thanks a lot! |
(cherry picked from commit c40c8f8)
(cherry picked from commit c40c8f8)
git diff upstream/master -u -- "*.py" | flake8 --diff