From 740adf8fbcecf0df4fbc05901953ffb01b1ffab2 Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Thu, 28 Apr 2022 15:36:08 -0500 Subject: [PATCH 01/12] BUG: GH46132 --- pandas/core/window/rolling.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index 5006db44849e0..9373e947757c1 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -658,7 +658,16 @@ def _numba_apply( return self._resolve_output(out, obj) def aggregate(self, func, *args, **kwargs): + _axis_modifed_flag = 0 + if self.axis == 1: + self.obj = self.obj.T + _axis_modifed_flag = 1 + self.axis = 0 result = ResamplerWindowApply(self, func, args=args, kwargs=kwargs).agg() + if _axis_modifed_flag == 1: + result = result.T + self.axis = 1 + self.obj = self.obj.T if result is None: return self.apply(func, raw=False, args=args, kwargs=kwargs) return result From 7a9d9099b887925032938cb5517b3f464fde0218 Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Thu, 28 Apr 2022 16:04:02 -0500 Subject: [PATCH 02/12] BUG: GH46132 rolling aggregate() with a list of functions along axis 1 raise ValueError --- pandas/core/window/rolling.py | 1 + pandas/tests/window/test_rolling.py | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index 9373e947757c1..fbd9a46356815 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -658,6 +658,7 @@ def _numba_apply( return self._resolve_output(out, obj) def aggregate(self, func, *args, **kwargs): + # GH46132 _axis_modifed_flag = 0 if self.axis == 1: self.obj = self.obj.T diff --git a/pandas/tests/window/test_rolling.py b/pandas/tests/window/test_rolling.py index 5f7709c62a1e2..35fbb1ca9428b 100644 --- a/pandas/tests/window/test_rolling.py +++ b/pandas/tests/window/test_rolling.py @@ -1849,3 +1849,23 @@ def test_rolling_var_same_value_count_logic(values, window, min_periods, expecte result_std = sr.rolling(window, min_periods=min_periods).std() tm.assert_series_equal(result_std, np.sqrt(expected)) tm.assert_series_equal(expected == 0, result_std == 0) + + +def test_rolling_agg_on_columns(): + # GH 46132 + + df = DataFrame({"a": [1, 3], "b": [2, 4]}) + res = df.rolling(window=2, axis=1, min_periods=1).aggregate([np.sum, np.mean]) + expected_val = np.array([[1, 3.0], + [1, 1.5], + [3, 7], + [3, 3.5]]) + + expected_index = MultiIndex.from_tuples([(0, 'sum'), + (0, 'mean'), + (1, 'sum'), + (1, 'mean')]) + + expected_frame = DataFrame(expected_val, index=expected_index, columns=["a", "b"]) + + tm.assert_frame_equal(expected_frame, res) From 2d11a3bb2607ea47106fe17b9aa8f3e05558810f Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Fri, 29 Apr 2022 05:52:04 +0000 Subject: [PATCH 03/12] Fixes from pre-commit [automated commit] --- pandas/tests/window/test_rolling.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/pandas/tests/window/test_rolling.py b/pandas/tests/window/test_rolling.py index 7a7bd3a96a04a..3e6b6fdc0a018 100644 --- a/pandas/tests/window/test_rolling.py +++ b/pandas/tests/window/test_rolling.py @@ -1856,15 +1856,11 @@ def test_rolling_agg_on_columns(): df = DataFrame({"a": [1, 3], "b": [2, 4]}) res = df.rolling(window=2, axis=1, min_periods=1).aggregate([np.sum, np.mean]) - expected_val = np.array([[1, 3.0], - [1, 1.5], - [3, 7], - [3, 3.5]]) - - expected_index = MultiIndex.from_tuples([(0, 'sum'), - (0, 'mean'), - (1, 'sum'), - (1, 'mean')]) + expected_val = np.array([[1, 3.0], [1, 1.5], [3, 7], [3, 3.5]]) + + expected_index = MultiIndex.from_tuples( + [(0, "sum"), (0, "mean"), (1, "sum"), (1, "mean")] + ) expected_frame = DataFrame(expected_val, index=expected_index, columns=["a", "b"]) From ca3e961c44b1b4754c93b3a457ee1529e5ad2db2 Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Fri, 29 Apr 2022 17:52:54 -0500 Subject: [PATCH 04/12] What's new entry --- doc/source/whatsnew/v1.5.0.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/source/whatsnew/v1.5.0.rst b/doc/source/whatsnew/v1.5.0.rst index c85a087835b80..4943fe9c0bc3b 100644 --- a/doc/source/whatsnew/v1.5.0.rst +++ b/doc/source/whatsnew/v1.5.0.rst @@ -635,6 +635,7 @@ Groupby/resample/rolling - Bug in :meth:`Rolling.var` and :meth:`Rolling.std` would give non-zero result with window of same values (:issue:`42064`) - Bug in :meth:`.Rolling.var` would segfault calculating weighted variance when window size was larger than data size (:issue:`46760`) - Bug in :meth:`Grouper.__repr__` where ``dropna`` was not included. Now it is (:issue:`46754`) +- Bug in :meth:`Rolling.aggregate` raise ValueError when ``func`` was a list of functions and ``axis=1`` (:issue:`46132`) Reshaping ^^^^^^^^^ From 32afa4324564ef0beafbc30eeb44328c49026c08 Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Sat, 30 Apr 2022 12:45:03 -0500 Subject: [PATCH 05/12] add try/finally and add test for the case if aggregate fails --- pandas/core/window/rolling.py | 17 ++++++++++------- pandas/tests/window/test_rolling.py | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index fbd9a46356815..e62e7a68dc8ae 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -659,18 +659,21 @@ def _numba_apply( def aggregate(self, func, *args, **kwargs): # GH46132 - _axis_modifed_flag = 0 + _axis_modifed_flag = False if self.axis == 1: self.obj = self.obj.T - _axis_modifed_flag = 1 + _axis_modifed_flag = True self.axis = 0 - result = ResamplerWindowApply(self, func, args=args, kwargs=kwargs).agg() - if _axis_modifed_flag == 1: - result = result.T - self.axis = 1 - self.obj = self.obj.T + try: + result = ResamplerWindowApply(self, func, args=args, kwargs=kwargs).agg() + finally: + if _axis_modifed_flag: + self.axis = 1 + self.obj = self.obj.T if result is None: return self.apply(func, raw=False, args=args, kwargs=kwargs) + if _axis_modifed_flag: + result = result.T return result agg = aggregate diff --git a/pandas/tests/window/test_rolling.py b/pandas/tests/window/test_rolling.py index 3e6b6fdc0a018..6eabd24972985 100644 --- a/pandas/tests/window/test_rolling.py +++ b/pandas/tests/window/test_rolling.py @@ -1876,3 +1876,18 @@ def test_rolling_mean_sum_floating_artifacts(): assert (result[-3:] == 0).all() result = r.sum() assert (result[-3:] == 0).all() + + +def test_rolling_agg_when_agg_fail(): + # GH 46132 + df = DataFrame({"a": [1, 3], "b": [2, 4]}) + win = df.rolling(window=2, axis=1, min_periods=1) + try: + win.aggregate([np.log, np.sqrt]) + except: + pass + tm.assert_frame_equal(win.obj, df) + # make sure if aggregate fails the attribute of Rolling/Window will not be change + assert win.axis == 1 + win.aggregate([np.mean, np.sum]) + assert win.axis == 1 From fecb9ea5ee3069ac511c377b8711a5ea9ca83910 Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Sat, 30 Apr 2022 12:55:53 -0500 Subject: [PATCH 06/12] fix PEP 8 issues --- pandas/tests/window/test_rolling.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/tests/window/test_rolling.py b/pandas/tests/window/test_rolling.py index 6eabd24972985..172e5a31edaf7 100644 --- a/pandas/tests/window/test_rolling.py +++ b/pandas/tests/window/test_rolling.py @@ -1884,7 +1884,7 @@ def test_rolling_agg_when_agg_fail(): win = df.rolling(window=2, axis=1, min_periods=1) try: win.aggregate([np.log, np.sqrt]) - except: + except ValueError: pass tm.assert_frame_equal(win.obj, df) # make sure if aggregate fails the attribute of Rolling/Window will not be change From fa2e1cbf75e988d47bde98fac5eed4aea569ae96 Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Sat, 30 Apr 2022 13:57:38 -0500 Subject: [PATCH 07/12] add comment --- pandas/core/window/rolling.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index e62e7a68dc8ae..e5b9434dda36c 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -660,6 +660,8 @@ def _numba_apply( def aggregate(self, func, *args, **kwargs): # GH46132 _axis_modifed_flag = False + # issue https://github.com/pandas-dev/pandas/issues/46132 + # modifing axis and transposing dataframe should not be needed once ReamplerWindow supports axis = 1 if self.axis == 1: self.obj = self.obj.T _axis_modifed_flag = True From 08fed948b6e0d142b05b8ce20731757274209604 Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Sat, 30 Apr 2022 14:18:35 -0500 Subject: [PATCH 08/12] fix issues --- pandas/core/window/rolling.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index e5b9434dda36c..1d8d0043e87ab 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -661,7 +661,8 @@ def aggregate(self, func, *args, **kwargs): # GH46132 _axis_modifed_flag = False # issue https://github.com/pandas-dev/pandas/issues/46132 - # modifing axis and transposing dataframe should not be needed once ReamplerWindow supports axis = 1 + # modifying axis and transposing dataframe should not be needed + # once ReamplerWindow supports axis = 1 if self.axis == 1: self.obj = self.obj.T _axis_modifed_flag = True From e34d1a568545b90fb9775f685cd084aad12ecb2c Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Sat, 30 Apr 2022 18:04:17 -0500 Subject: [PATCH 09/12] update test --- pandas/tests/window/test_rolling.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pandas/tests/window/test_rolling.py b/pandas/tests/window/test_rolling.py index 172e5a31edaf7..56f116b6aec38 100644 --- a/pandas/tests/window/test_rolling.py +++ b/pandas/tests/window/test_rolling.py @@ -1887,7 +1887,10 @@ def test_rolling_agg_when_agg_fail(): except ValueError: pass tm.assert_frame_equal(win.obj, df) - # make sure if aggregate fails the attribute of Rolling/Window will not be change + # make sure if aggregate fails the attribute of Rolling/Window will not be changed assert win.axis == 1 + # make sure the attribute of Rolling/Window will not be changed + # when aggregate runs successfully win.aggregate([np.mean, np.sum]) + tm.assert_frame_equal(win.obj, df) assert win.axis == 1 From 45185a93a8e7485086e086fcc9cc64bf85f38247 Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Sat, 30 Apr 2022 19:27:18 -0500 Subject: [PATCH 10/12] update comment --- pandas/core/window/rolling.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index 1d8d0043e87ab..4cae15e233c0e 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -660,7 +660,6 @@ def _numba_apply( def aggregate(self, func, *args, **kwargs): # GH46132 _axis_modifed_flag = False - # issue https://github.com/pandas-dev/pandas/issues/46132 # modifying axis and transposing dataframe should not be needed # once ReamplerWindow supports axis = 1 if self.axis == 1: From 8e21d87a4b8d51156c785e3bdffd099a154e6b71 Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Sun, 5 Jun 2022 15:42:52 -0500 Subject: [PATCH 11/12] cannot solve this issue by using _get_data_to_aggregate pattern or by assigning ResamplerWindowApply's attribute. --- pandas/core/window/rolling.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index 4cae15e233c0e..95bd7f5ae6b6d 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -659,23 +659,27 @@ def _numba_apply( def aggregate(self, func, *args, **kwargs): # GH46132 - _axis_modifed_flag = False # modifying axis and transposing dataframe should not be needed # once ReamplerWindow supports axis = 1 - if self.axis == 1: - self.obj = self.obj.T - _axis_modifed_flag = True - self.axis = 0 + + obj = self.obj + axis = self.axis + + self.obj = self.obj.T if self.axis == 1 else self.obj + self.axis = 0 + try: result = ResamplerWindowApply(self, func, args=args, kwargs=kwargs).agg() finally: - if _axis_modifed_flag: - self.axis = 1 - self.obj = self.obj.T + self.obj = obj + self.axis = axis + + if axis == 1: + result = result.T if result is not None else result + if result is None: return self.apply(func, raw=False, args=args, kwargs=kwargs) - if _axis_modifed_flag: - result = result.T + return result agg = aggregate From 2a933d9b0e40d792f65f6284ebce69d24821f483 Mon Sep 17 00:00:00 2001 From: xr-chen <826010519@qq.com> Date: Sun, 5 Jun 2022 15:54:02 -0500 Subject: [PATCH 12/12] fix pre-commit issue --- doc/source/whatsnew/v1.5.0.rst | 3 --- pandas/tests/window/test_rolling.py | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/source/whatsnew/v1.5.0.rst b/doc/source/whatsnew/v1.5.0.rst index 53678671fd149..0f361ca9a74a9 100644 --- a/doc/source/whatsnew/v1.5.0.rst +++ b/doc/source/whatsnew/v1.5.0.rst @@ -862,13 +862,10 @@ Groupby/resample/rolling - Bug in :meth:`Rolling.skew` and :meth:`Rolling.kurt` would give NaN with window of same values (:issue:`30993`) - Bug in :meth:`.Rolling.var` would segfault calculating weighted variance when window size was larger than data size (:issue:`46760`) - Bug in :meth:`Grouper.__repr__` where ``dropna`` was not included. Now it is (:issue:`46754`) -<<<<<<< HEAD - Bug in :meth:`Rolling.aggregate` raise ValueError when ``func`` was a list of functions and ``axis=1`` (:issue:`46132`) -======= - Bug in :meth:`DataFrame.rolling` gives ValueError when center=True, axis=1 and win_type is specified (:issue:`46135`) - Bug in :meth:`.DataFrameGroupBy.describe` and :meth:`.SeriesGroupBy.describe` produces inconsistent results for empty datasets (:issue:`41575`) - Bug in :meth:`DataFrame.resample` reduction methods when used with ``on`` would attempt to aggregate the provided column (:issue:`47079`) ->>>>>>> upstream/main Reshaping ^^^^^^^^^ diff --git a/pandas/tests/window/test_rolling.py b/pandas/tests/window/test_rolling.py index cf92146bd4a32..35bd4853df515 100644 --- a/pandas/tests/window/test_rolling.py +++ b/pandas/tests/window/test_rolling.py @@ -1894,6 +1894,8 @@ def test_rolling_agg_when_agg_fail(): win.aggregate([np.mean, np.sum]) tm.assert_frame_equal(win.obj, df) assert win.axis == 1 + + def test_rolling_skew_kurt_floating_artifacts(): # GH 42064 46431