From 71bb72b140bf864401388d796e190a8ea31977f7 Mon Sep 17 00:00:00 2001 From: phofl Date: Sat, 3 Oct 2020 01:32:59 +0200 Subject: [PATCH 1/3] Raise ValueError with nan in timeaware windows --- doc/source/whatsnew/v1.2.0.rst | 1 + pandas/core/window/rolling.py | 19 +++++++++++-------- pandas/tests/window/test_grouper.py | 17 +++++++++++++++++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/doc/source/whatsnew/v1.2.0.rst b/doc/source/whatsnew/v1.2.0.rst index 016e8d90e7d21..d8d486e6ff177 100644 --- a/doc/source/whatsnew/v1.2.0.rst +++ b/doc/source/whatsnew/v1.2.0.rst @@ -404,6 +404,7 @@ Groupby/resample/rolling - Bug in :meth:`DataFrame.groupby` does not always maintain column index name for ``any``, ``all``, ``bfill``, ``ffill``, ``shift`` (:issue:`29764`) - Bug in :meth:`DataFrameGroupBy.apply` raising error with ``np.nan`` group(s) when ``dropna=False`` (:issue:`35889`) - Bug in :meth:`Rolling.sum()` returned wrong values when dtypes where mixed between float and integer and axis was equal to one (:issue:`20649`, :issue:`35596`) +- Bug in :meth:`DataFrameGroupBy.rolling` returned wrong values with timeaware window containing ``NaN``. Raises ``ValueError`` because windows are not monotonic now (:issue:`34617`) Reshaping ^^^^^^^^^ diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index 6ab42dda865e7..0d57be6fb00e5 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -1984,10 +1984,13 @@ def _validate_monotonic(self): Validate monotonic (increasing or decreasing). """ if not (self._on.is_monotonic_increasing or self._on.is_monotonic_decreasing): - formatted = self.on - if self.on is None: - formatted = "index" - raise ValueError(f"{formatted} must be monotonic") + self._raise_monotonic_error() + + def _raise_monotonic_error(self): + formatted = self.on + if self.on is None: + formatted = "index" + raise ValueError(f"{formatted} must be monotonic") def _validate_freq(self): """ @@ -2301,8 +2304,8 @@ def _gotitem(self, key, ndim, subset=None): def _validate_monotonic(self): """ Validate that on is monotonic; - we don't care for groupby.rolling - because we have already validated at a higher - level. + in this case we have to check only for nans, because + monotonicy was already validated at a higher level. """ - pass + if self._on.hasnans: + self._raise_monotonic_error() diff --git a/pandas/tests/window/test_grouper.py b/pandas/tests/window/test_grouper.py index 0eebd657e97b7..02f63d01d0854 100644 --- a/pandas/tests/window/test_grouper.py +++ b/pandas/tests/window/test_grouper.py @@ -416,3 +416,20 @@ def test_groupby_rolling_empty_frame(self): result = expected.groupby(["s1", "s2"]).rolling(window=1).sum() expected.index = pd.MultiIndex.from_tuples([], names=["s1", "s2", None]) tm.assert_frame_equal(result, expected) + + @pytest.mark.parametrize( + ("rollings", "key"), [({"on": "a"}, "a"), ({"on": None}, "index")] + ) + def test_groupby_rolling_nans_in_index(self, rollings, key): + # GH: 34617 + df = pd.DataFrame( + { + "a": pd.to_datetime(["2020-06-01 12:00", "2020-06-01 14:00", np.nan]), + "b": [1, 2, 3], + "c": [1, 1, 1], + } + ) + if key == "index": + df.set_index("a", inplace=True) + with pytest.raises(ValueError, match=f"{key} must be monotonic"): + df.groupby("c").rolling("60min", **rollings) From 0ab0c55cfeb2d549cdf105ab28943c313e06b69f Mon Sep 17 00:00:00 2001 From: phofl Date: Sat, 3 Oct 2020 04:23:39 +0200 Subject: [PATCH 2/3] Remove inplace from test --- pandas/tests/window/test_grouper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/tests/window/test_grouper.py b/pandas/tests/window/test_grouper.py index 262215ca2a627..f8feff3f83b89 100644 --- a/pandas/tests/window/test_grouper.py +++ b/pandas/tests/window/test_grouper.py @@ -442,6 +442,6 @@ def test_groupby_rolling_nans_in_index(self, rollings, key): } ) if key == "index": - df.set_index("a", inplace=True) + df = df.set_index("a") with pytest.raises(ValueError, match=f"{key} must be monotonic"): df.groupby("c").rolling("60min", **rollings) From 6936af34f124862b3cb580e4134da84b3aaa04ef Mon Sep 17 00:00:00 2001 From: phofl Date: Wed, 7 Oct 2020 10:09:32 +0200 Subject: [PATCH 3/3] Fix Pep 8 --- pandas/tests/window/test_grouper.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/tests/window/test_grouper.py b/pandas/tests/window/test_grouper.py index 9f0d9d69502ae..f1fb70b5b860a 100644 --- a/pandas/tests/window/test_grouper.py +++ b/pandas/tests/window/test_grouper.py @@ -458,7 +458,6 @@ def test_groupby_rolling_string_index(self): ).set_index(["group", "index"]) tm.assert_frame_equal(result, expected) - @pytest.mark.parametrize( ("rollings", "key"), [({"on": "a"}, "a"), ({"on": None}, "index")] )