ENH: Rolling window with step size (GH-15354)
State: closed

- [x] closes #15354
- [x] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [x] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [ ] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.


Labels:
name: Enhancement
name: Window
description: rolling, ewma, expanding

Comments:
This is quite a large PR because the introduction of a step argument requires changes in multiple places where rolling window functionality is implemented or exposed, which in turn requires adding a bunch of tests to cover these changes. The general rule is that adding `step=step` to a (possibly group-by) rolling window is equivalent to adding `[::step]` to the original (possibly per-group) result. The step-argument-related code is designed to optimize the computation, i.e., to avoid computing an output that would immediately be filtered out by `[::step]`.

While the code for this PR took some time to figure out, almost all of the changes seem reasonable to me in hindsight. Nevertheless, one thing I'm worried about is the interaction of this PR with [#42915](https://github.com/pandas-dev/pandas/issues/42915). Specifically, the [test_rolling_corr_cov_other_diff_size_as_groups](https://github.com/pandas-dev/pandas/pull/45735/commits/ff6a89885dcc92c948c27bb03d71ab8a8789d85e#diff-e338c43cbd06b849f3d6a1b97cd787a48770c616b627f33eb20f67a6fc56b116R182) test case ran into trouble and I could only get it to work with the following changes:
1. [Decapsulating the per-group step-filtering](https://github.com/pandas-dev/pandas/pull/45735/commits/ff6a89885dcc92c948c27bb03d71ab8a8789d85e#diff-e338c43cbd06b849f3d6a1b97cd787a48770c616b627f33eb20f67a6fc56b116R188-R190)
2. Adding a [pairwise-result adjustment function](https://github.com/pandas-dev/pandas/pull/45735/commits/ff6a89885dcc92c948c27bb03d71ab8a8789d85e#diff-376f031d24a9c167d8ea06982971b613702f7dd122e168c3a8b333009a811ed4R809) that is implemented differently between [rolling-group-by window](https://github.com/pandas-dev/pandas/pull/45735/commits/ff6a89885dcc92c948c27bb03d71ab8a8789d85e#diff-376f031d24a9c167d8ea06982971b613702f7dd122e168c3a8b333009a811ed4R2706-R2723) and [other (expanding- or emw-) windows](https://github.com/pandas-dev/pandas/pull/45735/commits/ff6a89885dcc92c948c27bb03d71ab8a8789d85e#diff-376f031d24a9c167d8ea06982971b613702f7dd122e168c3a8b333009a811ed4R778-R784). I tried multiple ways to encapsulate the step-filtering, but they all ran into issues (e.g., reindexing issues related to [prep_binary](https://github.com/rtpsw/pandas/blob/ff6a89885dcc92c948c27bb03d71ab8a8789d85e/pandas/core/window/common.py#L168) in `flex_binary_moments`). I gave up on getting it to work without decapsulation after a few tries and decided to put in this PR for review. Because I was able to get the similar [test_rolling_corr_cov_other_same_size_as_groups](https://github.com/pandas-dev/pandas/pull/45735/commits/ff6a89885dcc92c948c27bb03d71ab8a8789d85e#diff-e338c43cbd06b849f3d6a1b97cd787a48770c616b627f33eb20f67a6fc56b116R154) test case (let alone the many other test cases in this PR) to work out without resorting to decapsulation, I suspect something weird is going on with this interaction, specifically something that has to do with the [#42915 rolling-window branching logic](https://github.com/rtpsw/pandas/blob/ff6a89885dcc92c948c27bb03d71ab8a8789d85e/pandas/core/window/rolling.py#L801-L842), but I couldn't as yet point my finger at what exactly. Please review this part carefully.
@mroeschke: I'd like to try to clean up the errors in this PR first. After that I don't mind canceling it and just using it as a reference as needed.

What do you have in mind for `test_step.py`? As I was working on testing this code, I found that adding the step parameter to existing tests was catching up unexpected errors left and right, so I'd feel confident about adding but not replacing existing tests in this PR.

IIRC, I did not add a step parameter to `ewm.method` and `expanding.method`. In any case, I do not have a strong opinion about raising `NotImplementedError` for them. As for `cov` and `corr`, I believe what passes the tests, as modified with the step argument and `[::step]` pattern, is most likely the behavior users would expect; so, if there is no desire to release them as standard, perhaps only as experimental?
> What do you have in mind for test_step.py? As I was working on testing this code, I found that adding the step parameter to existing tests was catching up unexpected errors left and right, so I'd feel confident about adding but not replacing existing tests in this PR.

Ah okay sure then this is a good argument to keep step in the existing tests.


> perhaps only as experimental

I slightly prefer having dedicated behavior than experimental per-se i.e. I would rather raise a `NotImplementedError`, flesh out the intended behavior in an issue, and then add.

But if through your investigation you are confident about the intended behavior with corr/cov, then I'll defer to you. 

While I'm reasonably confident about the expected behavior of most rolling window step-operations as discussed in [#15354](https://github.com/pandas-dev/pandas/issues/15354), I'd suggest posting there about the `[::step]` pattern and see if anyone objects.

As for `corr` and `cov`, I'm not sure what behavior users would expect with a step-parameter even after reading through #[42915](https://github.com/pandas-dev/pandas/issues/42915). Here I'd vote for `NotImplementedError` and open a discussion about this.
AFAICS, the build failure is benign and this PR is cleaned up of errors. I'll work on starting a separate PR that advances in manageable chunks.
I'm closing this PR and will open a replacement one soon.
Replaced by [#45765](https://github.com/pandas-dev/pandas/pull/45765).