Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOC: Closed parameter not intuitively documented in DataFrame.rolling Part 2 The Window Strikes Back #60844

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

SebastianOuslis
Copy link
Contributor

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! In addition, can we fix some of what was introduced in #60832. It seems to me the closed argument does not render the way I believe was intended.

https://pandas.pydata.org/pandas-docs/dev/reference/api/pandas.core.groupby.DataFrameGroupBy.rolling.html

Sphinx needs a blank line to induce a line break in the resulting html.

image

In addition, I think the notation / language here can be tightened up.

If 'right', (First, Last] the last point in the window is included in the calculations.

In the above, (First, Last] is inserted mid-sentence without any reference. I'd suggest

If right, uses the window (first, last] meaning the last point is included in the calculations.

pandas/core/groupby/groupby.py Outdated Show resolved Hide resolved

If an integer, the fixed number of observations used for
each window.
If an integer, the delta between the start and end of each window.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about adding "The number of points in the window depends on the closed argument".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it, though I do think it would require a proper understanding of the closed argument to actually understand the implications on the size (i.e the example where the closed param is set to neither and can result in no window being calculated since the number of points is too few)

@SebastianOuslis
Copy link
Contributor Author

Thanks for the PR! In addition, can we fix some of what was introduced in #60832. It seems to me the closed argument does not render the way I believe was intended.

https://pandas.pydata.org/pandas-docs/dev/reference/api/pandas.core.groupby.DataFrameGroupBy.rolling.html

Sphinx needs a blank line to induce a line break in the resulting html.

image

In addition, I think the notation / language here can be tightened up.

If 'right', (First, Last] the last point in the window is included in the calculations.

In the above, (First, Last] is inserted mid-sentence without any reference. I'd suggest

If right, uses the window (first, last] meaning the last point is included in the calculations.

I agree with your comments and have updated the formatting as well. Thanks for pointing out the issue with the line break.

@mroeschke mroeschke added the Docs label Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: Closed parameter not intuitively documented in DataFrame.rolling
3 participants