-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
DOC: reverse rolling window (#38627) #41842
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: reverse rolling window (#38627) #41842
Conversation
File is copied from @vangorade last changes in pull request (pandas-dev#39091): pandas-dev#39091 Fixed typo to pass pre-commit codespell hook.
From pull request (pandas-dev#39091): pandas-dev#39091 - remove noqa - change line back to single quotes - rename df1 to reversed_df
From pull request (pandas-dev#39091): pandas-dev#39091 - remove repeated df initialization in code example - rename df2 to reversed_df
…andas-dev#38627). Move code example as suggested in pull request (pandas-dev#39091): pandas-dev#39091 Compiled document to check it looked fine by running: python make.py --single user_guide/window.rst
There was a problem hiding this 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 @mdhsieh! Some initial comments
doc/source/user_guide/window.rst
Outdated
s = pd.Series(range(5), index=pd.date_range('2020-01-01', periods=5, freq='1D')) | ||
s.rolling(window='2D').sum() | ||
s = pd.Series(range(5), index=pd.date_range("2020-01-01", periods=5, freq="1D")) | ||
s.rolling(window="2D").sum() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there are still some nonessential changes
doc/source/user_guide/window.rst
Outdated
df = pd.DataFrame({'A': ['a', 'b', 'a', 'b', 'a'], 'B': range(5)}) | ||
df.groupby('A').expanding().sum() | ||
df = pd.DataFrame({"A": ["a", "b", "a", "b", "a"], "B": range(5)}) | ||
df.groupby("A").expanding().sum() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here and other places
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, changed them back
doc/source/user_guide/window.rst
Outdated
passed then computes the statistic for each pair of columns, returning a :class:`DataFrame` with a | ||
:class:`MultiIndex` whose values are the dates in question (see :ref:`the next section | ||
passed then computes the statistic for each pair of columns, returning a | ||
``MultiIndexed DataFrame`` whose ``index`` are the dates in question (see :ref:`the next section |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change looks suspect, think you might need to merge master if you didn't add this change
doc/source/user_guide/window.rst
Outdated
@@ -592,7 +601,7 @@ The following formula is used to compute exponentially weighted mean with an inp | |||
|
|||
.. math:: | |||
|
|||
y_t = \frac{\sum_{i=0}^t 0.5^\frac{t_{t} - t_{i}}{\lambda} x_{t-i}}{\sum_{i=0}^t 0.5^\frac{t_{t} - t_{i}}{\lambda}}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment w respect to merging master as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you check the diff, looks unchanged. You might need to fetch first, see https://pandas.pydata.org/docs/development/contributing.html#working-with-the-code for a workflow on updating a pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I added the changes from master branch file since fetching and merging didn't change those lines..
Unecessary changes.
Description was not modified after fetching and merging, so pasted from master branch: https://github.com/pandas-dev/pandas/blob/master/doc/source/user_guide/window.rst
Since fetching and merging didn't modify this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you just add the new section and save cleanups for another PR (so just add content)?
It still appears that things changed that were not supposed to in the PR.
Yeah, I can do that. |
To only add reverse rolling window section and avoid any unwanted changes.
Forgot to add.
I realized I accidentally deleted part of a code example.
|
Updated, does this work now? |
doc/source/user_guide/window.rst
Outdated
reversed_df = df[::-1].rolling("2s").sum()[::-1] | ||
reversed_df | ||
|
||
Or we can also do it using :meth:`api.indexers.FixedForwardWindowIndexer` which basically creates window boundaries for fixed-length windows that include the current row. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is exactly the same as the one above this new section?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right the FixedForwardWindowIndexer
example is the same, my bad. Deleted it.
doc/source/user_guide/window.rst
Outdated
@@ -299,6 +299,38 @@ forward-looking rolling window, and we can use it as follows: | |||
indexer = FixedForwardWindowIndexer(window_size=2) | |||
df.rolling(indexer, min_periods=1).sum() | |||
|
|||
Reverse rolling window |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this example has the same result as the FixedForwardWindowIndexer
you added in this section, you can just continue in the section above and show that it's equivalent to the code block you added here (i.e. don't need the new section or first sentence)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, sure.
FixedForwaredWindowIndexer example is same as the one above the added section.
Modify sentence slightly to show this example should output same result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor comment otherwise LGTM
doc/source/user_guide/window.rst
Outdated
@@ -299,6 +299,24 @@ forward-looking rolling window, and we can use it as follows: | |||
indexer = FixedForwardWindowIndexer(window_size=2) | |||
df.rolling(indexer, min_periods=1).sum() | |||
|
|||
We can also achieve this by using slicing in python, applying rolling aggregation, and then flipping the result as shown in example below: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Don't need in python
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, done.
Coding is already in Python.
Thanks @mdhsieh. CI failure unrelated |
* DOC: Add examples to do reversed rolling window (pandas-dev#38627). File is copied from @vangorade last changes in pull request (pandas-dev#39091): pandas-dev#39091 Fixed typo to pass pre-commit codespell hook. * DOC: Add some suggested changes. From pull request (pandas-dev#39091): pandas-dev#39091 - remove noqa - change line back to single quotes - rename df1 to reversed_df * DOC: Add reference to FixedForwardWindowIndexer. * DOC: Add more suggested changes. From pull request (pandas-dev#39091): pandas-dev#39091 - remove repeated df initialization in code example - rename df2 to reversed_df * DOC: Add suggested changes to reverse rolling window code examples (pandas-dev#38627). Move code example as suggested in pull request (pandas-dev#39091): pandas-dev#39091 Compiled document to check it looked fine by running: python make.py --single user_guide/window.rst * DOC: Revert more double quotes to single quotes. Unecessary changes. * DOC: Change binary window description to match master branch. Description was not modified after fetching and merging, so pasted from master branch: https://github.com/pandas-dev/pandas/blob/master/doc/source/user_guide/window.rst * DOC: Change exponentially weighted mean formula to one in master branch. Since fetching and merging didn't modify this line. * DOC: Replace window.rst with file from pandas-dev master branch. To only add reverse rolling window section and avoid any unwanted changes. * DOC: Add section with reverse rolling window example. * DOC: Add section markup. Forgot to add. * DOC: Delete the same code example. FixedForwaredWindowIndexer example is same as the one above the added section. * DOC: Remove section and first sentence. Modify sentence slightly to show this example should output same result. * DOC: Remove unecessary words. Coding is already in Python.
This adds the suggested changes from closed pull request (#38627):
https://github.com/pandas-dev/pandas/pull/39091/files/80819f65402e46caa29ff7192b34d43f91c1a0ee
After updated commit.
remove noqa
change line back to single quotes
rename
df1
toreversed_df
add reference to
FixedForwardWindowIndexer
remove repeated
df
initialization in code examplerename
df2
toreversed_df
move code example
closes DOC: example for doing a reversed rolling window #38627
tests added / passed
Ensure all linting tests pass, see here for how to run them
whatsnew entry