Repository: pandas-dev/pandas
Pull request: TST: add test for rolling max with DatetimeIndex
- [x] closes #21096
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Problem statement:
regression in 0.23.0 on rolling max with DatetimeIndex.
Behaviour of .rolling() with datetimeindex appears to have changed or regressed between 0.22 & 0.23 versions of pandas.

See below for cut-down test case exhibiting behaviour.

Also see below for different output produced on same code on same data with 2 different pandas versions.

```python
import numpy as np
import pandas as pd


def main():
    print('pandas', pd.__version__)
    n = 10
    index = pd.date_range(
        start='2018-1-1 01:00:00',
        freq='1min',
        periods=n,
    )
    s = pd.Series(
        data=0,
        index=index,
    )
    s.iloc[1] = np.nan
    s.iloc[-1] = 2
    print(s)
    maxes = s.rolling(window=f'{n}min').max()
    print(maxes.value_counts(dropna=False))


if __name__ == '__main__':
    main()
```

ACTUAL OUTPUT
```
pandas 0.22.0      
2018-01-01 01:00:00    0.0             
2018-01-01 01:01:00    NaN             
2018-01-01 01:02:00    0.0             
2018-01-01 01:03:00    0.0             
2018-01-01 01:04:00    0.0             
2018-01-01 01:05:00    0.0             
2018-01-01 01:06:00    0.0             
2018-01-01 01:07:00    0.0             
2018-01-01 01:08:00    0.0             
2018-01-01 01:09:00    2.0             
Freq: T, dtype: float64                
0.0    9           
2.0    1           
dtype: int64       
pandas 0.23.0      
2018-01-01 01:00:00    0.0             
2018-01-01 01:01:00    NaN             
2018-01-01 01:02:00    0.0             
2018-01-01 01:03:00    0.0             
2018-01-01 01:04:00    0.0             
2018-01-01 01:05:00    0.0             
2018-01-01 01:06:00    0.0             
2018-01-01 01:07:00    0.0             
2018-01-01 01:08:00    0.0             
2018-01-01 01:09:00    2.0             
Freq: T, dtype: float64                
0.0    10          
dtype: int64       

```

Hints from issue discussion:
can you create the input programatically here (and update the top )
done @jreback , updated simpler test-case, completely in code in 1st comment.
```
$ git bisect bad 
39e7b6916b07982240bac87132848fb2665806a2 is the first bad commit
commit 39e7b6916b07982240bac87132848fb2665806a2
Author: Matt Kirk <matt@matthewkirk.com>
Date:   Wed Feb 14 18:13:19 2018 +0700

    Performance increase rolling min max (#19549)

:040000 040000 6543b93964c04b7f71c14a9992c8f70606e02c77 a7f4578db8843fdb0f9dbe02322584ced5628dd9 M      asv_bench
:040000 040000 3f68b43b26722b2f7a2874433e88965ca5d8bfda 2eaff46202589c76a8d85251cfdb052dd38660f1 M      doc
:040000 040000 e28eb2f13f13846afe64b82ce492a5a877b98be7 18784432eb0d9655cca3de34a02f5f514c44eaff M      pandas
:100755 100755 2332503e558ed7ee9d57e3b3153ae674c1d292ad c66979dd19ef039cd4b5172c65276d1340507631 M      setup.py
```
cc @hexgnu 
This looks fixed on master and could still use a regression test.

```
In [9]: pd.__version__
Out[9]: '0.24.0.dev0+911.g24ab22f75'

In [10]: s
Out[10]:
2018-01-01 01:00:00    0.0
2018-01-01 01:01:00    NaN
2018-01-01 01:02:00    0.0
2018-01-01 01:03:00    0.0
2018-01-01 01:04:00    0.0
2018-01-01 01:05:00    0.0
2018-01-01 01:06:00    0.0
2018-01-01 01:07:00    0.0
2018-01-01 01:08:00    0.0
2018-01-01 01:09:00    2.0
Freq: T, dtype: float64

In [11]: maxes = s.rolling(window=f'{n}min').max()

In [12]: maxes.value_counts(dropna=False)
Out[12]:
0.0    9
2.0    1
dtype: int64
```

Commit message: add test for rolling max with DatetimeIndex

Patch to review:
diff --git a/pandas/tests/window/test_moments.py b/pandas/tests/window/test_moments.py
index 6e4bc621d7f49..b3aef73b81335 100644
--- a/pandas/tests/window/test_moments.py
+++ b/pandas/tests/window/test_moments.py
@@ -1891,6 +1891,20 @@ def test_rolling_corr_with_zero_variance(self, window):
 
         assert s.rolling(window=window).corr(other=other).isna().all()
 
+    def test_rolling_max_datetimeindex(self):
+        # GH 21096
+        n = 10
+        index = pd.date_range(start="2018-1-1 01:00:00", freq="1min", periods=n)
+        s = Series(data=0, index=index)
+
+        s.iloc[1] = np.nan
+        s.iloc[-1] = 2
+        maxes = s.rolling(window=f"{n}min").max()
+        result = maxes.value_counts(dropna=False)
+        expected = Series(data={0.0: n - 1, 2.0: 1})
+
+        tm.assert_series_equal(result, expected)
+
     def _check_pairwise_moment(self, dispatch, name, **kwargs):
         def get_result(obj, obj2=None):
             return getattr(getattr(obj, dispatch)(**kwargs), name)(obj2)


Code review comments:
diff_hunk: @@ -1891,6 +1891,20 @@ def test_rolling_corr_with_zero_variance(self, window):
 
         assert s.rolling(window=window).corr(other=other).isna().all()
 
+    def test_rolling_max_datetimeindex(self):
path: pandas/tests/window/test_moments.py
text: @user1:
move to tests/window/test_timeseries_window.py and put near similar tests (this might be a duplicate)

@author:
Thanks for the feedback, @user1. Moved the test to `tests/window/test_timeseries_window.py`. This test is different by being the only one using minutes as frequency in the file. Other minor differences is that it's using Series as opposed to DataFrame and explicit definition of expected.

Having said that, I agree that there are very similar tests and I'm happy to remove it all together.

@user2:
@author if you would like to clean that up and parametrize frequencies / containers that would be *very* welcome