Repository: pandas-dev/pandas
Pull request: BUG: Fix empty closed window issue with rolling min and max
- [x] closes #26005 (#26940 and #26910 solves other issues presented in #26005)
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Problem statement:
offset-based rolling window, multiple issues with closed='left'
#### Code Sample

```python
# Case 1: single row
df1 = pd.DataFrame({'B': [0]}, index=[pd.Timestamp('20130101 09:00:00')])
df1.rolling('1s', closed='left').median()  # <- raises 'MemoryError: skiplist_init failed'

# Case 2: multiple rows, but entries separated by a larger time than the specified window
df2 = pd.DataFrame({'B': [0, 1]}, index=[pd.Timestamp('20130101 09:00:00'), pd.Timestamp('20130101 09:00:02')])
df2.rolling('1s', closed='left').median() # <- raises 'MemoryError: skiplist_init failed'
df2.rolling('1s', closed='left').max() # <- no error, but second entry seems incorrect

# Case 3: as long as at least one row has other entries in its window, it runs without 
# an exception but the values are suspect
df3 = pd.DataFrame({'B': [1, 2, 3]}, index=[pd.Timestamp('20130101 09:00:00'), pd.Timestamp('20130101 09:00:02'), pd.Timestamp('20130101 09:00:03')])
df3.rolling('1s', closed='left').median() # <- no exception, but the values seem incorrect
df3.rolling('1s', closed='left').max() # ditto
df3.rolling('2s', closed='left').median() # ditto (note longer window)
```
#### Problem description

Obviously, the exception cases are a big problem and should be addressed. The other cases laid out here seem to give unexpected results that are inconsistent with other aggregations (such as mean and sum) that do seem to be operating correctly. Note that using closed='right' or closed='both' does seem to give results consistent with my expectations, while using closed='neither' yields similar problems as closed='left'. (So, it would seem that the common factor here is whether or not the input rows are included in their own rolling windows.)

#### Expected Output

Case 1: since there are no other entries in the input row's window, I would expect that the median aggregation return NaN. (This would be consistent with mean, max, etc. for this case.)

```python
                      B
2013-01-01 09:00:00 NaN
```

Case 2: since neither input row should have any other entries in their windows, I would expect that the median and max results should all be NaN. (This would be consistent with what the mean aggregation returns for this case.)
```python
                     B
2013-01-01 09:00:00  NaN
2013-01-01 09:00:02  NaN
```

Case 3a and 3b (1s window): since neither of the first two input rows should have any other entries in their windows, I would expect that their median and max results should all be NaN. since the last row does have an entry in its window (the second row) I would expect that both the median and max should be 2.0. (This would be consistent with what the mean aggregation returns for this case.)
```python
                     B
2013-01-01 09:00:00  NaN
2013-01-01 09:00:02  NaN
2013-01-01 09:00:03  2.0
```
Case 3c (2s window): since the first row should have no entries in its window, I would expect the first output row to be NaN. the second row will have the first entry in its window, so I would expect its output to be 1.0. similarly, the last row will have the second entry in its window and I would expect its output to be 2.0.
```python
                     B
2013-01-01 09:00:00  NaN
2013-01-01 09:00:02  1.0
2013-01-01 09:00:03  2.0
```


#### Output of ``pd.show_versions()``

<details>

INSTALLED VERSIONS
------------------
commit: None
python: 2.7.14.final.0
python-bits: 64
OS: Linux
OS-release: 4.4.0-17134-Microsoft
machine: x86_64
processor: x86_64
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: None.None
pandas: 0.24.2
pytest: None
pip: 19.0.1
setuptools: 40.6.3
Cython: None
numpy: 1.16.0
scipy: 1.1.0
pyarrow: None
xarray: None
IPython: None
sphinx: None
patsy: None
dateutil: 2.7.5
pytz: 2018.9
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: 2.0.2
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml.etree: 4.3.0
bs4: None
html5lib: None
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: 2.10
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None
gcsfs: None
</details>

Hints from issue discussion:
Thanks for the report. Might be an off by one error with closed='left' for these. Investigation and PRs would certainly be welcome

Commit message: Add whatsnew

Patch to review:
diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst
index 82e093bc2bd49..e2554b0da148d 100644
--- a/doc/source/whatsnew/v0.25.0.rst
+++ b/doc/source/whatsnew/v0.25.0.rst
@@ -841,6 +841,7 @@ Groupby/resample/rolling
 - Bug in :meth:`pandas.core.groupby.GroupBy.agg` where incorrect results are returned for uint64 columns. (:issue:`26310`)
 - Bug in :meth:`pandas.core.window.Rolling.median` and :meth:`pandas.core.window.Rolling.quantile` where MemoryError is raised with empty window (:issue:`26005`)
 - Bug in :meth:`pandas.core.window.Rolling.median` and :meth:`pandas.core.window.Rolling.quantile` where incorrect results are returned with ``closed='left'`` and ``closed='neither'`` (:issue:`26005`)
+- Bug in :meth:`pandas.core.window.Rolling.max` and :meth:`pandas.core.window.Rolling.min` where incorrect results are returned with an empty variable window`` (:issue:`26005`)
 
 Reshaping
 ^^^^^^^^^
diff --git a/pandas/_libs/window.pyx b/pandas/_libs/window.pyx
index df86f395d6097..6203577e450d9 100644
--- a/pandas/_libs/window.pyx
+++ b/pandas/_libs/window.pyx
@@ -1313,9 +1313,11 @@ cdef _roll_min_max_variable(ndarray[numeric] values,
 
         # if right is open then the first window is empty
         close_offset = 0 if endi[0] > starti[0] else 1
+        # first window's size
+        curr_win_size = endi[0] - starti[0]
 
         for i in range(endi[0], endi[N-1]):
-            if not Q.empty():
+            if not Q.empty() and curr_win_size > 0:
                 output[i-1+close_offset] = calc_mm(
                     minp, nobs, values[Q.front()])
             else:
@@ -1344,7 +1346,7 @@ cdef _roll_min_max_variable(ndarray[numeric] values,
             Q.push_back(i)
             W.push_back(i)
 
-        if not Q.empty():
+        if not Q.empty() and curr_win_size > 0:
             output[N-1] = calc_mm(minp, nobs, values[Q.front()])
         else:
             output[N-1] = NaN
diff --git a/pandas/tests/test_window.py b/pandas/tests/test_window.py
index 4dfdd1c96728b..639d5bcfefa3f 100644
--- a/pandas/tests/test_window.py
+++ b/pandas/tests/test_window.py
@@ -520,6 +520,19 @@ def test_closed(self):
         with pytest.raises(ValueError):
             df.rolling(window=3, closed='neither')
 
+    @pytest.mark.parametrize("closed", ["neither", "left"])
+    @pytest.mark.parametrize(
+        "func", ["std", "mean", "median", "sum", "max", "min", "var"])
+    def test_closed_empty(self, closed, func):
+        # GH 26005
+        ser = pd.Series(data=np.arange(5),
+                        index=pd.date_range("2000", periods=5, freq="2D"))
+        roll = ser.rolling("1D", closed=closed)
+
+        result = getattr(roll, func)()
+        expected = pd.Series([np.nan] * 5, index=ser.index)
+        tm.assert_series_equal(result, expected)
+
     @pytest.mark.parametrize("func", ['min', 'max'])
     def test_closed_one_entry(self, func):
         # GH24718


Code review comments:
diff_hunk: @@ -520,6 +520,19 @@ def test_closed(self):
         with pytest.raises(ValueError):
             df.rolling(window=3, closed='neither')
 
+    @pytest.mark.parametrize("closed", ["neither", "left"])
+    @pytest.mark.parametrize(
+        "func", ["std", "mean", "median", "sum", "max", "min", "var"])
+    def test_closed_empty(self, closed, func):
path: pandas/tests/test_window.py
text: @user1:
we have a fixture for these - look in pandas/conftest

@author:
I guess it is `all_numeric_reductions` but `prod` function fails since it is not implemented for `Rolling`. I will add `kurt` and `skew` to the list  

@user1:
you may need to define a separate fixture to avoid some issues

@author:
Done