From 98c7183b92308cf9fa598fd17d83ec479aa956f4 Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Fri, 17 Apr 2020 09:34:20 +0300 Subject: [PATCH 1/8] enable count for BaseIndexer windows --- pandas/core/window/common.py | 1 + pandas/tests/window/test_base_indexer.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pandas/core/window/common.py b/pandas/core/window/common.py index 40f17126fa163..436585fe221dd 100644 --- a/pandas/core/window/common.py +++ b/pandas/core/window/common.py @@ -328,6 +328,7 @@ def func(arg, window, min_periods=None): def validate_baseindexer_support(func_name: Optional[str]) -> None: # GH 32865: These functions work correctly with a BaseIndexer subclass BASEINDEXER_WHITELIST = { + "count", "min", "max", "mean", diff --git a/pandas/tests/window/test_base_indexer.py b/pandas/tests/window/test_base_indexer.py index 43489e310bb93..93cc240a174c4 100644 --- a/pandas/tests/window/test_base_indexer.py +++ b/pandas/tests/window/test_base_indexer.py @@ -82,7 +82,7 @@ def get_window_bounds(self, num_values, min_periods, center, closed): df.rolling(indexer, win_type="boxcar") -@pytest.mark.parametrize("func", ["count", "skew", "cov", "corr"]) +@pytest.mark.parametrize("func", ["skew", "cov", "corr"]) def test_notimplemented_functions(func): # GH 32865 class CustomIndexer(BaseIndexer): From a812a8fc8138b798d6223a934e6079e7b24f59bf Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Fri, 17 Apr 2020 09:49:27 +0300 Subject: [PATCH 2/8] TST: add count to the test --- pandas/tests/window/test_base_indexer.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pandas/tests/window/test_base_indexer.py b/pandas/tests/window/test_base_indexer.py index 93cc240a174c4..1a3fe865d2a7a 100644 --- a/pandas/tests/window/test_base_indexer.py +++ b/pandas/tests/window/test_base_indexer.py @@ -99,6 +99,7 @@ def get_window_bounds(self, num_values, min_periods, center, closed): @pytest.mark.parametrize( "func,np_func,expected,np_kwargs", [ + ("count", len, [3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 2.0, np.nan], {},), ("min", np.min, [0.0, 1.0, 2.0, 3.0, 4.0, 6.0, 6.0, 7.0, 8.0, np.nan], {},), ( "max", From b540cf09a2923aa74c47e3887d45379e8be67808 Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Fri, 17 Apr 2020 10:08:44 +0300 Subject: [PATCH 3/8] ENH: actually get custom windows for count --- pandas/core/window/rolling.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index 3fdf81c4bb570..f2bec5ea24242 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -1939,7 +1939,9 @@ def aggregate(self, func, *args, **kwargs): def count(self): # different impl for freq counting - if self.is_freq_type: + # GH 32865. Also use custom rolling windows calculation + # when using a BaseIndexer subclass + if self.is_freq_type or isinstance(self.window, BaseIndexer): window_func = self._get_roll_func("roll_count") return self._apply(window_func, center=self.center, name="count") From 0c212b88b8adc11f0396ae85ff8d01eb676b6b8f Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Fri, 17 Apr 2020 10:13:16 +0300 Subject: [PATCH 4/8] remove validation from count BaseIndexer support validation is covered by the validation in _get_window_indexer --- pandas/core/window/rolling.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index f2bec5ea24242..f8a85e59054c7 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -1171,8 +1171,6 @@ class _Rolling_and_Expanding(_Rolling): ) def count(self): - if isinstance(self.window, BaseIndexer): - validate_baseindexer_support("count") blocks, obj = self._create_blocks() results = [] From 5877141ec38d358305ebb7bef32e2f416b7b7592 Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Fri, 17 Apr 2020 10:14:58 +0300 Subject: [PATCH 5/8] DOC: add whatsnew entry --- doc/source/whatsnew/v1.1.0.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index 2a641a37b46d8..50d97284b9023 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -174,8 +174,8 @@ Other API changes - Added :meth:`DataFrame.value_counts` (:issue:`5377`) - :meth:`Groupby.groups` now returns an abbreviated representation when called on large dataframes (:issue:`1135`) - ``loc`` lookups with an object-dtype :class:`Index` and an integer key will now raise ``KeyError`` instead of ``TypeError`` when key is missing (:issue:`31905`) -- Using a :func:`pandas.api.indexers.BaseIndexer` with ``count``, ``skew``, ``cov``, ``corr`` will now raise a ``NotImplementedError`` (:issue:`32865`) -- Using a :func:`pandas.api.indexers.BaseIndexer` with ``min``, ``max`` will now return correct results for any monotonic :func:`pandas.api.indexers.BaseIndexer` descendant (:issue:`32865`) +- Using a :func:`pandas.api.indexers.BaseIndexer` with ``skew``, ``cov``, ``corr`` will now raise a ``NotImplementedError`` (:issue:`32865`) +- Using a :func:`pandas.api.indexers.BaseIndexer` with ``count``, ``min``, ``max`` will now return correct results for any monotonic :func:`pandas.api.indexers.BaseIndexer` descendant (:issue:`32865`) - Added a :func:`pandas.api.indexers.FixedForwardWindowIndexer` class to support forward-looking windows during ``rolling`` operations. - From af41d6fe4fd72f47ec55c0c959b9be4cde2af669 Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Fri, 17 Apr 2020 10:35:06 +0300 Subject: [PATCH 6/8] DOC: clarify the comment --- pandas/core/window/rolling.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index f8a85e59054c7..410c17553a683 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -1937,8 +1937,8 @@ def aggregate(self, func, *args, **kwargs): def count(self): # different impl for freq counting - # GH 32865. Also use custom rolling windows calculation - # when using a BaseIndexer subclass + # GH 32865. Use a custom count function implementation + # when using a BaseIndexer subclass as a window if self.is_freq_type or isinstance(self.window, BaseIndexer): window_func = self._get_roll_func("roll_count") return self._apply(window_func, center=self.center, name="count") From f8a126a900609c4646e2809324f1b46ed9332436 Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Fri, 17 Apr 2020 18:17:45 +0300 Subject: [PATCH 7/8] add self.window class check to count --- pandas/core/window/rolling.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pandas/core/window/rolling.py b/pandas/core/window/rolling.py index 410c17553a683..a98a1ad19cb75 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -1171,6 +1171,7 @@ class _Rolling_and_Expanding(_Rolling): ) def count(self): + assert not isinstance(self.window, BaseIndexer) blocks, obj = self._create_blocks() results = [] From 12fc3b372b3a235c211a2506c7f744cf4ec535ed Mon Sep 17 00:00:00 2001 From: Alexander Kirko Date: Fri, 17 Apr 2020 18:20:19 +0300 Subject: [PATCH 8/8] DOC: add comment to assert --- 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 a98a1ad19cb75..62f470060b039 100644 --- a/pandas/core/window/rolling.py +++ b/pandas/core/window/rolling.py @@ -1171,6 +1171,8 @@ class _Rolling_and_Expanding(_Rolling): ) def count(self): + # GH 32865. Using count with custom BaseIndexer subclass + # implementations shouldn't end up here assert not isinstance(self.window, BaseIndexer) blocks, obj = self._create_blocks()