From 088ca14b4dcb95a949a5947c59000aac73895871 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 14:03:18 -0400 Subject: [PATCH 01/31] WIP --- pandas/_libs/groupby.pyx | 28 +++++++++++++++++++++++++--- pandas/core/groupby/groupby.py | 20 +++++++++++++++++--- 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/pandas/_libs/groupby.pyx b/pandas/_libs/groupby.pyx index 48ee01c809efd..8090ee560603a 100644 --- a/pandas/_libs/groupby.pyx +++ b/pandas/_libs/groupby.pyx @@ -391,9 +391,10 @@ def group_fillna_indexer(ndarray[int64_t] out, ndarray[intp_t] labels, def group_any_all(uint8_t[::1] out, const uint8_t[::1] values, const intp_t[:] labels, - const uint8_t[::1] mask, + uint8_t[::1] mask, str val_test, - bint skipna) -> None: + bint skipna, + uint8_t[::1] output_mask = None) -> None: """ Aggregated boolean values to show truthfulness of group elements. @@ -422,6 +423,9 @@ def group_any_all(uint8_t[::1] out, Py_ssize_t i, N = len(labels) intp_t lab uint8_t flag_val + bint use_kleene_logic + + use_kleene_logic = output_mask is not None if val_test == 'all': # Because the 'all' value of an empty iterable in Python is True we can @@ -437,16 +441,34 @@ def group_any_all(uint8_t[::1] out, raise ValueError("'bool_func' must be either 'any' or 'all'!") out[:] = 1 - flag_val + if use_kleene_logic: + output_mask[:] = 1 - skipna with nogil: for i in range(N): lab = labels[i] - if lab < 0 or (skipna and mask[i]): + if lab < 0 or (mask[i] and skipna): continue + if use_kleene_logic: + if not mask[i]: + + + if use_kleene_logic and not mask[i]: + output_mask[lab] = 0 + + if use_kleene_logic and mask[i]: + output_mask[lab] = 1 + + # Either 'any' and True or 'all' and False if values[i] == flag_val: out[lab] = flag_val + # In both of the above cases, the value + # is known regardless of any missing values + if use_kleene_logic: + output_mask[lab] = 0 + # ---------------------------------------------------------------------- # group_add, group_prod, group_var, group_mean, group_ohlc diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index e226f771d5b9f..00b06ab23299e 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -30,6 +30,10 @@ class providing the base-class of operations. Union, ) +from pandas.core.arrays.masked import BaseMaskedArray +from pandas.core.arrays.boolean import BooleanArray + + import numpy as np from pandas._config.config import option_context @@ -1413,16 +1417,21 @@ def _bool_agg(self, val_test, skipna): Shared func to call any / all Cython GroupBy implementations. """ - def objs_to_bool(vals: np.ndarray) -> tuple[np.ndarray, type]: + def objs_to_bool(vals: ArrayLike) -> tuple[np.ndarray, type]: if is_object_dtype(vals): vals = np.array([bool(x) for x in vals]) + elif isinstance(vals, BaseMaskedArray): + vals = vals._data.astype(bool) else: vals = vals.astype(bool) return vals.view(np.uint8), bool - def result_to_bool(result: np.ndarray, inference: type) -> np.ndarray: - return result.astype(inference, copy=False) + def result_to_bool(result: np.ndarray, inference: type, output_mask: np.ndarray | None = None) -> ArrayLike: + if output_mask is None: + return result.astype(inference, copy=False) + else: + return BooleanArray(result, output_mask) return self._get_cythonized_result( "group_any_all", @@ -2735,6 +2744,11 @@ def _get_cythonized_result( mask = isna(values).view(np.uint8) func = partial(func, mask) + if how == "group_any_all" and isinstance(values, BaseMaskedArray): + output_mask = np.zeros(result_sz, dtype=np.uint8) + func = partial(func, output_mask=output_mask) + post_processing = partial(post_processing, output_mask=output_mask) + if needs_ngroups: func = partial(func, ngroups) From 2554921a4fe6e12f4a2773ad7c25ae522077ee73 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 15:02:50 -0400 Subject: [PATCH 02/31] TSTS: Consolidate groupby any, all --- .../tests/groupby/aggregate/test_aggregate.py | 16 ---- pandas/tests/groupby/test_any_all.py | 92 +++++++++++++++++++ pandas/tests/groupby/test_function.py | 43 --------- pandas/tests/groupby/test_groupby.py | 11 --- 4 files changed, 92 insertions(+), 70 deletions(-) create mode 100644 pandas/tests/groupby/test_any_all.py diff --git a/pandas/tests/groupby/aggregate/test_aggregate.py b/pandas/tests/groupby/aggregate/test_aggregate.py index fc0b4d86e81bf..145f2643465ef 100644 --- a/pandas/tests/groupby/aggregate/test_aggregate.py +++ b/pandas/tests/groupby/aggregate/test_aggregate.py @@ -402,22 +402,6 @@ def test_multi_function_flexible_mix(df): grouped.aggregate(d) -def test_groupby_agg_coercing_bools(): - # issue 14873 - dat = DataFrame({"a": [1, 1, 2, 2], "b": [0, 1, 2, 3], "c": [None, None, 1, 1]}) - gp = dat.groupby("a") - - index = Index([1, 2], name="a") - - result = gp["b"].aggregate(lambda x: (x != 0).all()) - expected = Series([False, True], index=index, name="b") - tm.assert_series_equal(result, expected) - - result = gp["c"].aggregate(lambda x: x.isnull().all()) - expected = Series([True, False], index=index, name="c") - tm.assert_series_equal(result, expected) - - @pytest.mark.parametrize( "op", [ diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py new file mode 100644 index 0000000000000..579909281f18e --- /dev/null +++ b/pandas/tests/groupby/test_any_all.py @@ -0,0 +1,92 @@ +import builtins +from io import StringIO + +import numpy as np +import pytest + +from pandas.errors import UnsupportedFunctionCall + +import pandas as pd +from pandas import ( + DataFrame, + Index, + MultiIndex, + Series, + Timestamp, + date_range, + isna, +) +import pandas._testing as tm +import pandas.core.nanops as nanops +from pandas.util import _test_decorators as td + + +@pytest.mark.parametrize("agg_func", ["any", "all"]) +@pytest.mark.parametrize("skipna", [True, False]) +@pytest.mark.parametrize( + "vals", + [ + ["foo", "bar", "baz"], + ["foo", "", ""], + ["", "", ""], + [1, 2, 3], + [1, 0, 0], + [0, 0, 0], + [1.0, 2.0, 3.0], + [1.0, 0.0, 0.0], + [0.0, 0.0, 0.0], + [True, True, True], + [True, False, False], + [False, False, False], + [np.nan, np.nan, np.nan], + ], +) +def test_groupby_bool_aggs(agg_func, skipna, vals): + df = DataFrame({"key": ["a"] * 3 + ["b"] * 3, "val": vals * 2}) + + # Figure out expectation using Python builtin + exp = getattr(builtins, agg_func)(vals) + + # edge case for missing data with skipna and 'any' + if skipna and all(isna(vals)) and agg_func == "any": + exp = False + + exp_df = DataFrame([exp] * 2, columns=["val"], index=Index(["a", "b"], name="key")) + result = getattr(df.groupby("key"), agg_func)(skipna=skipna) + tm.assert_frame_equal(result, exp_df) + + +def test_any(gb): + expected = DataFrame( + [[True, True], [False, True]], columns=["B", "C"], index=[1, 3] + ) + expected.index.name = "A" + result = gb.any() + tm.assert_frame_equal(result, expected) + + +def test_groupby_agg_coercing_bools(): + # issue 14873 + dat = DataFrame({"a": [1, 1, 2, 2], "b": [0, 1, 2, 3], "c": [None, None, 1, 1]}) + gp = dat.groupby("a") + + index = Index([1, 2], name="a") + + result = gp["b"].aggregate(lambda x: (x != 0).all()) + expected = Series([False, True], index=index, name="b") + tm.assert_series_equal(result, expected) + + result = gp["c"].aggregate(lambda x: x.isnull().all()) + expected = Series([True, False], index=index, name="c") + tm.assert_series_equal(result, expected) + + +@pytest.mark.parametrize("bool_agg_func", ["any", "all"]) +def test_bool_aggs_dup_column_labels(bool_agg_func): + # 21668 + df = DataFrame([[True, True]], columns=["a", "a"]) + grp_by = df.groupby([0]) + result = getattr(grp_by, bool_agg_func)() + + expected = df + tm.assert_frame_equal(result, expected) diff --git a/pandas/tests/groupby/test_function.py b/pandas/tests/groupby/test_function.py index 515774eae009b..79173ac9bd2cb 100644 --- a/pandas/tests/groupby/test_function.py +++ b/pandas/tests/groupby/test_function.py @@ -41,41 +41,6 @@ def numpy_dtypes_for_minmax(request): return (dtype, min_val, max_val) -@pytest.mark.parametrize("agg_func", ["any", "all"]) -@pytest.mark.parametrize("skipna", [True, False]) -@pytest.mark.parametrize( - "vals", - [ - ["foo", "bar", "baz"], - ["foo", "", ""], - ["", "", ""], - [1, 2, 3], - [1, 0, 0], - [0, 0, 0], - [1.0, 2.0, 3.0], - [1.0, 0.0, 0.0], - [0.0, 0.0, 0.0], - [True, True, True], - [True, False, False], - [False, False, False], - [np.nan, np.nan, np.nan], - ], -) -def test_groupby_bool_aggs(agg_func, skipna, vals): - df = DataFrame({"key": ["a"] * 3 + ["b"] * 3, "val": vals * 2}) - - # Figure out expectation using Python builtin - exp = getattr(builtins, agg_func)(vals) - - # edge case for missing data with skipna and 'any' - if skipna and all(isna(vals)) and agg_func == "any": - exp = False - - exp_df = DataFrame([exp] * 2, columns=["val"], index=Index(["a", "b"], name="key")) - result = getattr(df.groupby("key"), agg_func)(skipna=skipna) - tm.assert_frame_equal(result, exp_df) - - def test_max_min_non_numeric(): # #2700 aa = DataFrame({"nn": [11, 11, 22, 22], "ii": [1, 2, 3, 4], "ss": 4 * ["mama"]}) @@ -344,14 +309,6 @@ def test_idxmin(self, gb): result = gb.idxmin() tm.assert_frame_equal(result, expected) - def test_any(self, gb): - expected = DataFrame( - [[True, True], [False, True]], columns=["B", "C"], index=[1, 3] - ) - expected.index.name = "A" - result = gb.any() - tm.assert_frame_equal(result, expected) - def test_mad(self, gb, gni): # mad expected = DataFrame([[0], [np.nan]], columns=["B"], index=[1, 3]) diff --git a/pandas/tests/groupby/test_groupby.py b/pandas/tests/groupby/test_groupby.py index de508b8cd78ec..6c51e32fa9a78 100644 --- a/pandas/tests/groupby/test_groupby.py +++ b/pandas/tests/groupby/test_groupby.py @@ -1978,17 +1978,6 @@ def test_groupby_duplicate_index(): tm.assert_series_equal(result, expected) -@pytest.mark.parametrize("bool_agg_func", ["any", "all"]) -def test_bool_aggs_dup_column_labels(bool_agg_func): - # 21668 - df = DataFrame([[True, True]], columns=["a", "a"]) - grp_by = df.groupby([0]) - result = getattr(grp_by, bool_agg_func)() - - expected = df - tm.assert_frame_equal(result, expected) - - @pytest.mark.parametrize( "idx", [Index(["a", "a"]), MultiIndex.from_tuples((("a", "a"), ("a", "a")))] ) From 9a8f9c9fcb1aa1205812f830a67094c5c25085a6 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 15:13:02 -0400 Subject: [PATCH 03/31] Fixture fixup --- pandas/tests/groupby/test_any_all.py | 17 ++++++----------- pandas/tests/groupby/test_function.py | 1 - 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index 579909281f18e..964ea0fc9fae9 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -1,24 +1,15 @@ import builtins -from io import StringIO import numpy as np import pytest -from pandas.errors import UnsupportedFunctionCall - -import pandas as pd from pandas import ( DataFrame, Index, - MultiIndex, Series, - Timestamp, - date_range, isna, ) import pandas._testing as tm -import pandas.core.nanops as nanops -from pandas.util import _test_decorators as td @pytest.mark.parametrize("agg_func", ["any", "all"]) @@ -56,12 +47,16 @@ def test_groupby_bool_aggs(agg_func, skipna, vals): tm.assert_frame_equal(result, exp_df) -def test_any(gb): +def test_any(): + df = DataFrame( + [[1, 2, "foo"], [1, np.nan, "bar"], [3, np.nan, "baz"]], + columns=["A", "B", "C"], + ) expected = DataFrame( [[True, True], [False, True]], columns=["B", "C"], index=[1, 3] ) expected.index.name = "A" - result = gb.any() + result = df.groupby("A").any() tm.assert_frame_equal(result, expected) diff --git a/pandas/tests/groupby/test_function.py b/pandas/tests/groupby/test_function.py index 79173ac9bd2cb..843d438018a32 100644 --- a/pandas/tests/groupby/test_function.py +++ b/pandas/tests/groupby/test_function.py @@ -14,7 +14,6 @@ Series, Timestamp, date_range, - isna, ) import pandas._testing as tm import pandas.core.nanops as nanops From 5ca9c4b7fdbe009f91dc2575be15c93b15b1729e Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 15:19:06 -0400 Subject: [PATCH 04/31] Unmove test --- .../tests/groupby/aggregate/test_aggregate.py | 16 ++++++++++++++++ pandas/tests/groupby/test_any_all.py | 17 ----------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/pandas/tests/groupby/aggregate/test_aggregate.py b/pandas/tests/groupby/aggregate/test_aggregate.py index 145f2643465ef..fc0b4d86e81bf 100644 --- a/pandas/tests/groupby/aggregate/test_aggregate.py +++ b/pandas/tests/groupby/aggregate/test_aggregate.py @@ -402,6 +402,22 @@ def test_multi_function_flexible_mix(df): grouped.aggregate(d) +def test_groupby_agg_coercing_bools(): + # issue 14873 + dat = DataFrame({"a": [1, 1, 2, 2], "b": [0, 1, 2, 3], "c": [None, None, 1, 1]}) + gp = dat.groupby("a") + + index = Index([1, 2], name="a") + + result = gp["b"].aggregate(lambda x: (x != 0).all()) + expected = Series([False, True], index=index, name="b") + tm.assert_series_equal(result, expected) + + result = gp["c"].aggregate(lambda x: x.isnull().all()) + expected = Series([True, False], index=index, name="c") + tm.assert_series_equal(result, expected) + + @pytest.mark.parametrize( "op", [ diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index 964ea0fc9fae9..4123fb95002dd 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -6,7 +6,6 @@ from pandas import ( DataFrame, Index, - Series, isna, ) import pandas._testing as tm @@ -60,22 +59,6 @@ def test_any(): tm.assert_frame_equal(result, expected) -def test_groupby_agg_coercing_bools(): - # issue 14873 - dat = DataFrame({"a": [1, 1, 2, 2], "b": [0, 1, 2, 3], "c": [None, None, 1, 1]}) - gp = dat.groupby("a") - - index = Index([1, 2], name="a") - - result = gp["b"].aggregate(lambda x: (x != 0).all()) - expected = Series([False, True], index=index, name="b") - tm.assert_series_equal(result, expected) - - result = gp["c"].aggregate(lambda x: x.isnull().all()) - expected = Series([True, False], index=index, name="c") - tm.assert_series_equal(result, expected) - - @pytest.mark.parametrize("bool_agg_func", ["any", "all"]) def test_bool_aggs_dup_column_labels(bool_agg_func): # 21668 From 26146c2ca8968949099b9ce96fa05297d241ceba Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 16:31:07 -0400 Subject: [PATCH 05/31] Add initial tests --- pandas/core/groupby/groupby.py | 2 +- pandas/tests/groupby/test_any_all.py | 45 +++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 00b06ab23299e..160ce143f8a12 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1431,7 +1431,7 @@ def result_to_bool(result: np.ndarray, inference: type, output_mask: np.ndarray if output_mask is None: return result.astype(inference, copy=False) else: - return BooleanArray(result, output_mask) + return BooleanArray(result.astype(bool), output_mask.astype(bool)) return self._get_cythonized_result( "group_any_all", diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index 4123fb95002dd..51e88d996c648 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -2,11 +2,12 @@ import numpy as np import pytest - +import pandas as pd from pandas import ( DataFrame, Index, isna, + Series, ) import pandas._testing as tm @@ -68,3 +69,45 @@ def test_bool_aggs_dup_column_labels(bool_agg_func): expected = df tm.assert_frame_equal(result, expected) + + +@pytest.mark.parametrize("bool_agg_func", ["any", "all"]) +@pytest.mark.parametrize("skipna", [True, False]) +@pytest.mark.parametrize( + # expected indexed as [skipna][bool_agg_func == "all"] + "data,expected", + [ + ([False, False, False], [[False, False], [False, False]]), + ([True, True, True], [[True, True], [True, True]]), + ([pd.NA, pd.NA, pd.NA], [[True, True], [True, True]]), + ([False, pd.NA, False], [[pd.NA, pd.NA], [False, False]]), + ([True, pd.NA, True], [[True, pd.NA], [True, True]]), + ([True, pd.NA, False], [[True, pd.NA], [True, False]]), + ], +) +def test_masked_kleene_logic(bool_agg_func, data, expected, skipna): + # GH-37506 + df = DataFrame(data, dtype="boolean") + expected = DataFrame( + [expected[skipna][bool_agg_func == "all"]], dtype="boolean", index=[1] + ) + + result = df.groupby([1, 1, 1]).agg(bool_agg_func) + + tm.assert_frame_equal(result, expected) + + +@pytest.mark.parametrize("bool_agg_func", ["any", "all"]) +@pytest.mark.parametrize("dtype", ["Int64", "Float64", "boolean"]) +@pytest.mark.parametrize("skipna", [True, False]) +def test_masked_bool_aggs_skipna(bool_agg_func, dtype, skipna, frame_or_series): + # GH-40585 + obj = frame_or_series([pd.NA, 1], dtype=dtype) + expected_res = True + if not skipna and bool_agg_func == "all": + expected_res = pd.NA + expected = frame_or_series([expected_res], index=[1], dtype="boolean") + + result = obj.groupby([1, 1]).agg(bool_agg_func, skipna=skipna) + + tm.assert_equal(result, expected) From 20f475d2a07bc2a6a64d11acc1506bc8242ca38b Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 17:53:25 -0400 Subject: [PATCH 06/31] Add whatsnew, bench --- asv_bench/benchmarks/groupby.py | 2 +- doc/source/whatsnew/v1.3.0.rst | 2 ++ pandas/_libs/groupby.pyx | 16 ---------------- pandas/tests/groupby/test_any_all.py | 2 +- 4 files changed, 4 insertions(+), 18 deletions(-) diff --git a/asv_bench/benchmarks/groupby.py b/asv_bench/benchmarks/groupby.py index 9930c61e34b15..3fd0def8b87c6 100644 --- a/asv_bench/benchmarks/groupby.py +++ b/asv_bench/benchmarks/groupby.py @@ -480,7 +480,7 @@ class GroupByCythonAgg: param_names = ["dtype", "method"] params = [ ["float64"], - ["sum", "prod", "min", "max", "mean", "median", "var", "first", "last"], + ["sum", "prod", "min", "max", "mean", "median", "var", "first", "last", "any", "all"], ] def setup(self, dtype, method): diff --git a/doc/source/whatsnew/v1.3.0.rst b/doc/source/whatsnew/v1.3.0.rst index 21e6f0ea57451..31c4833d4f85c 100644 --- a/doc/source/whatsnew/v1.3.0.rst +++ b/doc/source/whatsnew/v1.3.0.rst @@ -195,6 +195,8 @@ Other enhancements - :class:`RangeIndex` can now be constructed by passing a ``range`` object directly e.g. ``pd.RangeIndex(range(3))`` (:issue:`12067`) - :meth:`round` being enabled for the nullable integer and floating dtypes (:issue:`38844`) - :meth:`pandas.read_csv` and :meth:`pandas.read_json` expose the argument ``encoding_errors`` to control how encoding errors are handled (:issue:`39450`) +- :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` return a ``BooleanDType`` result using Kleene logic for methods ``any`` and ``all`` with nullable data types (:issue:`37506`, :issue:`40585`) +- .. --------------------------------------------------------------------------- diff --git a/pandas/_libs/groupby.pyx b/pandas/_libs/groupby.pyx index 8090ee560603a..5a0e2a6b41424 100644 --- a/pandas/_libs/groupby.pyx +++ b/pandas/_libs/groupby.pyx @@ -450,25 +450,9 @@ def group_any_all(uint8_t[::1] out, if lab < 0 or (mask[i] and skipna): continue - if use_kleene_logic: - if not mask[i]: - - - if use_kleene_logic and not mask[i]: - output_mask[lab] = 0 - - if use_kleene_logic and mask[i]: - output_mask[lab] = 1 - - # Either 'any' and True or 'all' and False if values[i] == flag_val: out[lab] = flag_val - # In both of the above cases, the value - # is known regardless of any missing values - if use_kleene_logic: - output_mask[lab] = 0 - # ---------------------------------------------------------------------- # group_add, group_prod, group_var, group_mean, group_ohlc diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index 51e88d996c648..d71fca2d5a1e7 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -79,7 +79,7 @@ def test_bool_aggs_dup_column_labels(bool_agg_func): [ ([False, False, False], [[False, False], [False, False]]), ([True, True, True], [[True, True], [True, True]]), - ([pd.NA, pd.NA, pd.NA], [[True, True], [True, True]]), + ([pd.NA, pd.NA, pd.NA], [[pd.NA, pd.NA], [False, True]]), ([False, pd.NA, False], [[pd.NA, pd.NA], [False, False]]), ([True, pd.NA, True], [[True, pd.NA], [True, True]]), ([True, pd.NA, False], [[True, pd.NA], [True, False]]), From 924b38e02e0fd0773c7bf6f641d05941ea082aea Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 19:17:38 -0400 Subject: [PATCH 07/31] Clean up edge case --- asv_bench/benchmarks/groupby.py | 14 +++++++++++++- pandas/_libs/groupby.pyx | 27 ++++++++++++++++++--------- pandas/core/groupby/groupby.py | 25 +++++++++++++------------ pandas/tests/groupby/test_any_all.py | 14 ++++++++------ 4 files changed, 52 insertions(+), 28 deletions(-) diff --git a/asv_bench/benchmarks/groupby.py b/asv_bench/benchmarks/groupby.py index 3fd0def8b87c6..b4b20553ec460 100644 --- a/asv_bench/benchmarks/groupby.py +++ b/asv_bench/benchmarks/groupby.py @@ -480,7 +480,19 @@ class GroupByCythonAgg: param_names = ["dtype", "method"] params = [ ["float64"], - ["sum", "prod", "min", "max", "mean", "median", "var", "first", "last", "any", "all"], + [ + "sum", + "prod", + "min", + "max", + "mean", + "median", + "var", + "first", + "last", + "any", + "all", + ], ] def setup(self, dtype, method): diff --git a/pandas/_libs/groupby.pyx b/pandas/_libs/groupby.pyx index 5a0e2a6b41424..d48ababd1a780 100644 --- a/pandas/_libs/groupby.pyx +++ b/pandas/_libs/groupby.pyx @@ -394,9 +394,10 @@ def group_any_all(uint8_t[::1] out, uint8_t[::1] mask, str val_test, bint skipna, - uint8_t[::1] output_mask = None) -> None: + bint use_kleene_logic = False) -> None: """ - Aggregated boolean values to show truthfulness of group elements. + Aggregated boolean values to show truthfulness of group elements. If the + input is a nullable type, Kleene logic will be used. Parameters ---------- @@ -413,19 +414,19 @@ def group_any_all(uint8_t[::1] out, String object dictating whether to use any or all truth testing skipna : bool Flag to ignore nan values during truth testing + use_kleene_logic : bool, default False + Whether or not to compute the result using Kleene logic Notes ----- This method modifies the `out` parameter rather than returning an object. - The returned values will either be 0 or 1 (False or True, respectively). + The returned values will either be 0, 1 (False or True, respectively), or + 2 to signify a masked position in the case of a nullable input. """ cdef: Py_ssize_t i, N = len(labels) intp_t lab uint8_t flag_val - bint use_kleene_logic - - use_kleene_logic = output_mask is not None if val_test == 'all': # Because the 'all' value of an empty iterable in Python is True we can @@ -441,15 +442,23 @@ def group_any_all(uint8_t[::1] out, raise ValueError("'bool_func' must be either 'any' or 'all'!") out[:] = 1 - flag_val - if use_kleene_logic: - output_mask[:] = 1 - skipna with nogil: for i in range(N): lab = labels[i] - if lab < 0 or (mask[i] and skipna): + if lab < 0 or (skipna and mask[i]): + continue + + if use_kleene_logic and mask[i]: + # Set the condition as masked if `out[lab] != flag_val`, which + # would indicate True/False has not yet been seen for any/all, + # so by Kleene logic the result is currently unknown + if out[lab] != flag_val: + out[lab] = 2 continue + # If True and 'any' or False and 'all', the result is + # already determined if values[i] == flag_val: out[lab] = flag_val diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 160ce143f8a12..eb593b9b3515f 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -30,10 +30,6 @@ class providing the base-class of operations. Union, ) -from pandas.core.arrays.masked import BaseMaskedArray -from pandas.core.arrays.boolean import BooleanArray - - import numpy as np from pandas._config.config import option_context @@ -84,6 +80,8 @@ class providing the base-class of operations. Categorical, ExtensionArray, ) +from pandas.core.arrays.boolean import BooleanArray +from pandas.core.arrays.masked import BaseMaskedArray from pandas.core.base import ( DataError, PandasObject, @@ -1421,17 +1419,21 @@ def objs_to_bool(vals: ArrayLike) -> tuple[np.ndarray, type]: if is_object_dtype(vals): vals = np.array([bool(x) for x in vals]) elif isinstance(vals, BaseMaskedArray): - vals = vals._data.astype(bool) + vals = vals._data.astype(bool, copy=False) else: vals = vals.astype(bool) return vals.view(np.uint8), bool - def result_to_bool(result: np.ndarray, inference: type, output_mask: np.ndarray | None = None) -> ArrayLike: - if output_mask is None: - return result.astype(inference, copy=False) + def result_to_bool( + result: np.ndarray, + inference: type, + result_is_nullable: bool = False, + ) -> ArrayLike: + if result_is_nullable: + return BooleanArray(result.astype(bool), result == 2) else: - return BooleanArray(result.astype(bool), output_mask.astype(bool)) + return result.astype(inference, copy=False) return self._get_cythonized_result( "group_any_all", @@ -2745,9 +2747,8 @@ def _get_cythonized_result( func = partial(func, mask) if how == "group_any_all" and isinstance(values, BaseMaskedArray): - output_mask = np.zeros(result_sz, dtype=np.uint8) - func = partial(func, output_mask=output_mask) - post_processing = partial(post_processing, output_mask=output_mask) + post_processing = partial(post_processing, result_is_nullable=True) + func = partial(func, use_kleene_logic=True) if needs_ngroups: func = partial(func, ngroups) diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index d71fca2d5a1e7..c9a92e8163837 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -2,12 +2,12 @@ import numpy as np import pytest + import pandas as pd from pandas import ( DataFrame, Index, isna, - Series, ) import pandas._testing as tm @@ -80,19 +80,19 @@ def test_bool_aggs_dup_column_labels(bool_agg_func): ([False, False, False], [[False, False], [False, False]]), ([True, True, True], [[True, True], [True, True]]), ([pd.NA, pd.NA, pd.NA], [[pd.NA, pd.NA], [False, True]]), - ([False, pd.NA, False], [[pd.NA, pd.NA], [False, False]]), + ([False, pd.NA, False], [[pd.NA, False], [False, False]]), ([True, pd.NA, True], [[True, pd.NA], [True, True]]), - ([True, pd.NA, False], [[True, pd.NA], [True, False]]), + ([True, pd.NA, False], [[True, False], [True, False]]), ], ) def test_masked_kleene_logic(bool_agg_func, data, expected, skipna): - # GH-37506 + # GH#37506 df = DataFrame(data, dtype="boolean") expected = DataFrame( [expected[skipna][bool_agg_func == "all"]], dtype="boolean", index=[1] ) - result = df.groupby([1, 1, 1]).agg(bool_agg_func) + result = df.groupby([1, 1, 1]).agg(bool_agg_func, skipna=skipna) tm.assert_frame_equal(result, expected) @@ -101,7 +101,7 @@ def test_masked_kleene_logic(bool_agg_func, data, expected, skipna): @pytest.mark.parametrize("dtype", ["Int64", "Float64", "boolean"]) @pytest.mark.parametrize("skipna", [True, False]) def test_masked_bool_aggs_skipna(bool_agg_func, dtype, skipna, frame_or_series): - # GH-40585 + # GH#40585 obj = frame_or_series([pd.NA, 1], dtype=dtype) expected_res = True if not skipna and bool_agg_func == "all": @@ -109,5 +109,7 @@ def test_masked_bool_aggs_skipna(bool_agg_func, dtype, skipna, frame_or_series): expected = frame_or_series([expected_res], index=[1], dtype="boolean") result = obj.groupby([1, 1]).agg(bool_agg_func, skipna=skipna) + print(result) + print(expected) tm.assert_equal(result, expected) From 423f43fce9a545b854bafb50a84dae31f24172bc Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 19:20:27 -0400 Subject: [PATCH 08/31] Fix typo --- pandas/_libs/groupby.pyx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pandas/_libs/groupby.pyx b/pandas/_libs/groupby.pyx index d48ababd1a780..9a44b89961e6b 100644 --- a/pandas/_libs/groupby.pyx +++ b/pandas/_libs/groupby.pyx @@ -391,7 +391,7 @@ def group_fillna_indexer(ndarray[int64_t] out, ndarray[intp_t] labels, def group_any_all(uint8_t[::1] out, const uint8_t[::1] values, const intp_t[:] labels, - uint8_t[::1] mask, + const uint8_t[::1] mask, str val_test, bint skipna, bint use_kleene_logic = False) -> None: @@ -450,7 +450,7 @@ def group_any_all(uint8_t[::1] out, continue if use_kleene_logic and mask[i]: - # Set the condition as masked if `out[lab] != flag_val`, which + # Set the position as masked if `out[lab] != flag_val`, which # would indicate True/False has not yet been seen for any/all, # so by Kleene logic the result is currently unknown if out[lab] != flag_val: From b1408aceb35a5a6fdb7f68030cda30c5fcbe31ee Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 19:33:08 -0400 Subject: [PATCH 09/31] Avoid copy if possible --- pandas/core/groupby/groupby.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index eb593b9b3515f..264b3ddcac88e 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1431,7 +1431,7 @@ def result_to_bool( result_is_nullable: bool = False, ) -> ArrayLike: if result_is_nullable: - return BooleanArray(result.astype(bool), result == 2) + return BooleanArray(result.astype(bool, copy=False), result == 2) else: return result.astype(inference, copy=False) From 47ef0376196070dab49f95b5547486cca66bdce2 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 20:17:56 -0400 Subject: [PATCH 10/31] Fix old level test --- doc/source/whatsnew/v1.3.0.rst | 2 +- pandas/tests/reductions/test_reductions.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/doc/source/whatsnew/v1.3.0.rst b/doc/source/whatsnew/v1.3.0.rst index 31c4833d4f85c..292dcfa3b5493 100644 --- a/doc/source/whatsnew/v1.3.0.rst +++ b/doc/source/whatsnew/v1.3.0.rst @@ -195,7 +195,7 @@ Other enhancements - :class:`RangeIndex` can now be constructed by passing a ``range`` object directly e.g. ``pd.RangeIndex(range(3))`` (:issue:`12067`) - :meth:`round` being enabled for the nullable integer and floating dtypes (:issue:`38844`) - :meth:`pandas.read_csv` and :meth:`pandas.read_json` expose the argument ``encoding_errors`` to control how encoding errors are handled (:issue:`39450`) -- :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` return a ``BooleanDType`` result using Kleene logic for methods ``any`` and ``all`` with nullable data types (:issue:`37506`, :issue:`40585`) +- :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` return a ``BooleanDType`` result using Kleene logic for methods ``any`` and ``all`` with nullable data types (:issue:`37506`, :issue:`40585`, :issue:`33449`) - .. --------------------------------------------------------------------------- diff --git a/pandas/tests/reductions/test_reductions.py b/pandas/tests/reductions/test_reductions.py index 2d7dddbc5ea42..f33ad8e774005 100644 --- a/pandas/tests/reductions/test_reductions.py +++ b/pandas/tests/reductions/test_reductions.py @@ -949,14 +949,13 @@ def test_all_any_boolean(self): assert s3.all(skipna=False) assert not s4.any(skipna=False) - # Check level TODO(GH-33449) result should also be boolean s = Series( [False, False, True, True, False, True], index=[0, 0, 1, 1, 2, 2], dtype="boolean", ) - tm.assert_series_equal(s.all(level=0), Series([False, True, False])) - tm.assert_series_equal(s.any(level=0), Series([False, True, True])) + tm.assert_series_equal(s.all(level=0), Series([False, True, False], dtype="boolean")) + tm.assert_series_equal(s.any(level=0), Series([False, True, True], dtype="boolean")) def test_any_axis1_bool_only(self): # GH#32432 From 441506083fec09996715d640a0a682b92708ad89 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 20:18:25 -0400 Subject: [PATCH 11/31] Precommit fixup --- pandas/tests/reductions/test_reductions.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pandas/tests/reductions/test_reductions.py b/pandas/tests/reductions/test_reductions.py index f33ad8e774005..03bad8ad5fd4d 100644 --- a/pandas/tests/reductions/test_reductions.py +++ b/pandas/tests/reductions/test_reductions.py @@ -954,8 +954,12 @@ def test_all_any_boolean(self): index=[0, 0, 1, 1, 2, 2], dtype="boolean", ) - tm.assert_series_equal(s.all(level=0), Series([False, True, False], dtype="boolean")) - tm.assert_series_equal(s.any(level=0), Series([False, True, True], dtype="boolean")) + tm.assert_series_equal( + s.all(level=0), Series([False, True, False], dtype="boolean") + ) + tm.assert_series_equal( + s.any(level=0), Series([False, True, True], dtype="boolean") + ) def test_any_axis1_bool_only(self): # GH#32432 From bb04c1c00f5c954459ae49dec0885e3fad1b7ff0 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 6 Apr 2021 20:20:22 -0400 Subject: [PATCH 12/31] Clean up print --- pandas/tests/groupby/test_any_all.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index c9a92e8163837..62bf7cc419b2b 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -93,7 +93,6 @@ def test_masked_kleene_logic(bool_agg_func, data, expected, skipna): ) result = df.groupby([1, 1, 1]).agg(bool_agg_func, skipna=skipna) - tm.assert_frame_equal(result, expected) @@ -109,7 +108,4 @@ def test_masked_bool_aggs_skipna(bool_agg_func, dtype, skipna, frame_or_series): expected = frame_or_series([expected_res], index=[1], dtype="boolean") result = obj.groupby([1, 1]).agg(bool_agg_func, skipna=skipna) - print(result) - print(expected) - tm.assert_equal(result, expected) From ef3fbe26da959ca44f2ff21d73b7c660f7ba4b05 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Wed, 7 Apr 2021 15:57:24 -0400 Subject: [PATCH 13/31] precommit fixup --- pandas/tests/groupby/test_any_all.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index 6f98939fce9d7..62bf7cc419b2b 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -4,7 +4,6 @@ import pytest import pandas as pd - from pandas import ( DataFrame, Index, From 1c3cb7df0d478209026695cfdaf9f8de53a3bb4d Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Thu, 8 Apr 2021 11:32:55 -0400 Subject: [PATCH 14/31] Split out test --- pandas/tests/reductions/test_reductions.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pandas/tests/reductions/test_reductions.py b/pandas/tests/reductions/test_reductions.py index 03bad8ad5fd4d..70e316e1545f8 100644 --- a/pandas/tests/reductions/test_reductions.py +++ b/pandas/tests/reductions/test_reductions.py @@ -949,17 +949,19 @@ def test_all_any_boolean(self): assert s3.all(skipna=False) assert not s4.any(skipna=False) - s = Series( + @pytest.mark.parametrize( + "bool_agg_func,expected", + [("all", [False, True, False]), ("any", [False, True, True])], + ) + def test_any_all_boolean_level(self, bool_agg_func, expected): + ser = Series( [False, False, True, True, False, True], index=[0, 0, 1, 1, 2, 2], dtype="boolean", ) - tm.assert_series_equal( - s.all(level=0), Series([False, True, False], dtype="boolean") - ) - tm.assert_series_equal( - s.any(level=0), Series([False, True, True], dtype="boolean") - ) + result = getattr(ser, bool_agg_func)(level=0) + expected = Series(expected, dtype="boolean") + tm.assert_series_equal(result, expected) def test_any_axis1_bool_only(self): # GH#32432 From 7cbf85b540fa834cc0f3ecf129ec2f8e89e9c512 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Thu, 8 Apr 2021 11:38:13 -0400 Subject: [PATCH 15/31] Split whatsnew --- doc/source/whatsnew/v1.3.0.rst | 4 +++- pandas/tests/reductions/test_reductions.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.3.0.rst b/doc/source/whatsnew/v1.3.0.rst index 292dcfa3b5493..223e940b9b7f8 100644 --- a/doc/source/whatsnew/v1.3.0.rst +++ b/doc/source/whatsnew/v1.3.0.rst @@ -195,7 +195,8 @@ Other enhancements - :class:`RangeIndex` can now be constructed by passing a ``range`` object directly e.g. ``pd.RangeIndex(range(3))`` (:issue:`12067`) - :meth:`round` being enabled for the nullable integer and floating dtypes (:issue:`38844`) - :meth:`pandas.read_csv` and :meth:`pandas.read_json` expose the argument ``encoding_errors`` to control how encoding errors are handled (:issue:`39450`) -- :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` return a ``BooleanDType`` result using Kleene logic for methods ``any`` and ``all`` with nullable data types (:issue:`37506`, :issue:`40585`, :issue:`33449`) +- :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` use Kleene logic for methods ``any`` and ``all`` with nullable data types (:issue:`37506`) +- :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` return a ``BooleanDType``for methods ``any`` and ``all`` with nullable data types (:issue:`33449`) - .. --------------------------------------------------------------------------- @@ -756,6 +757,7 @@ Groupby/resample/rolling - Bug in :class:`core.window.ewm.ExponentialMovingWindow` when calling ``__getitem__`` would not retain ``com``, ``span``, ``alpha`` or ``halflife`` attributes (:issue:`40164`) - :class:`core.window.ewm.ExponentialMovingWindow` now raises a ``NotImplementedError`` when specifying ``times`` with ``adjust=False`` due to an incorrect calculation (:issue:`40098`) - Bug in :meth:`Series.asfreq` and :meth:`DataFrame.asfreq` dropping rows when the index is not sorted (:issue:`39805`) +- Bug in :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` raising ``ValueError`` when using methods ``any`` and ``all`` with nullable type columns holding ``NA`` even with ``skipna=True`` (:issue:`40585`) Reshaping ^^^^^^^^^ diff --git a/pandas/tests/reductions/test_reductions.py b/pandas/tests/reductions/test_reductions.py index 70e316e1545f8..4089d135db881 100644 --- a/pandas/tests/reductions/test_reductions.py +++ b/pandas/tests/reductions/test_reductions.py @@ -954,6 +954,7 @@ def test_all_any_boolean(self): [("all", [False, True, False]), ("any", [False, True, True])], ) def test_any_all_boolean_level(self, bool_agg_func, expected): + # GH#33449 ser = Series( [False, False, True, True, False, True], index=[0, 0, 1, 1, 2, 2], From 809b8a497d2608390c28361d8ede29ce5b63763e Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Thu, 8 Apr 2021 11:39:41 -0400 Subject: [PATCH 16/31] whatsnew typo --- doc/source/whatsnew/v1.3.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.3.0.rst b/doc/source/whatsnew/v1.3.0.rst index 223e940b9b7f8..9e8ea830fdcd1 100644 --- a/doc/source/whatsnew/v1.3.0.rst +++ b/doc/source/whatsnew/v1.3.0.rst @@ -196,7 +196,7 @@ Other enhancements - :meth:`round` being enabled for the nullable integer and floating dtypes (:issue:`38844`) - :meth:`pandas.read_csv` and :meth:`pandas.read_json` expose the argument ``encoding_errors`` to control how encoding errors are handled (:issue:`39450`) - :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` use Kleene logic for methods ``any`` and ``all`` with nullable data types (:issue:`37506`) -- :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` return a ``BooleanDType``for methods ``any`` and ``all`` with nullable data types (:issue:`33449`) +- :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` return a ``BooleanDType`` for methods ``any`` and ``all`` with nullable data types (:issue:`33449`) - .. --------------------------------------------------------------------------- From 58fd33aef966680902016f0fcfc612cabb8f8e86 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Thu, 8 Apr 2021 12:57:58 -0400 Subject: [PATCH 17/31] Modify dispatch, add mixed test --- pandas/_libs/groupby.pyx | 7 ++--- pandas/core/groupby/groupby.py | 23 +++++++++++----- pandas/tests/groupby/test_any_all.py | 40 ++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/pandas/_libs/groupby.pyx b/pandas/_libs/groupby.pyx index 9a44b89961e6b..e869b6cace1d1 100644 --- a/pandas/_libs/groupby.pyx +++ b/pandas/_libs/groupby.pyx @@ -394,7 +394,7 @@ def group_any_all(uint8_t[::1] out, const uint8_t[::1] mask, str val_test, bint skipna, - bint use_kleene_logic = False) -> None: + bint masked) -> None: """ Aggregated boolean values to show truthfulness of group elements. If the input is a nullable type, Kleene logic will be used. @@ -414,8 +414,8 @@ def group_any_all(uint8_t[::1] out, String object dictating whether to use any or all truth testing skipna : bool Flag to ignore nan values during truth testing - use_kleene_logic : bool, default False - Whether or not to compute the result using Kleene logic + masked : bool + If True, compute the result using Kleene logic Notes ----- @@ -427,6 +427,7 @@ def group_any_all(uint8_t[::1] out, Py_ssize_t i, N = len(labels) intp_t lab uint8_t flag_val + bint use_kleene_logic = masked if val_test == 'all': # Because the 'all' value of an empty iterable in Python is True we can diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index d58d77e06e14f..5aa8856d08cfb 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1428,9 +1428,9 @@ def objs_to_bool(vals: ArrayLike) -> tuple[np.ndarray, type]: def result_to_bool( result: np.ndarray, inference: type, - result_is_nullable: bool = False, + masked: bool = False, ) -> ArrayLike: - if result_is_nullable: + if masked: return BooleanArray(result.astype(bool, copy=False), result == 2) else: return result.astype(inference, copy=False) @@ -1446,6 +1446,7 @@ def result_to_bool( post_processing=result_to_bool, val_test=val_test, skipna=skipna, + masked=False, ) @final @@ -2674,7 +2675,8 @@ def _get_cythonized_result( Function to be applied to result of Cython function. Should accept an array of values as the first argument and type inferences as its second argument, i.e. the signature should be - (ndarray, Type). + (ndarray, Type). Optionally, a third argument can be "masked", to + allow for processing specific to nullable values **kwargs : dict Extra arguments to be passed back to Cython funcs @@ -2700,10 +2702,16 @@ def _get_cythonized_result( output: dict[base.OutputKey, np.ndarray] = {} base_func = getattr(libgroupby, how) + post_processing_accepts_masked = ( + "masked" in inspect.signature(post_processing).parameters + ) + kwargs_accepts_masked = "masked" in kwargs + error_msg = "" for idx, obj in enumerate(self._iterate_slices()): name = obj.name values = obj._values + is_nullable = isinstance(values, BaseMaskedArray) if numeric_only and not is_numeric_dtype(values): continue @@ -2746,13 +2754,12 @@ def _get_cythonized_result( mask = isna(values).view(np.uint8) func = partial(func, mask) - if how == "group_any_all" and isinstance(values, BaseMaskedArray): - post_processing = partial(post_processing, result_is_nullable=True) - func = partial(func, use_kleene_logic=True) - if needs_ngroups: func = partial(func, ngroups) + if kwargs_accepts_masked: + kwargs["masked"] = is_nullable + func(**kwargs) # Call func to modify indexer values in place if needs_2d: @@ -2762,6 +2769,8 @@ def _get_cythonized_result( result = algorithms.take_nd(values, result) if post_processing: + if post_processing_accepts_masked: + post_processing = partial(post_processing, masked=is_nullable) result = post_processing(result, inferences) key = base.OutputKey(label=name, position=idx) diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index 62bf7cc419b2b..97bbce763b4d4 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -96,6 +96,46 @@ def test_masked_kleene_logic(bool_agg_func, data, expected, skipna): tm.assert_frame_equal(result, expected) +@pytest.mark.parametrize( + "dtype1,dtype2,exp_col1,exp_col2", + [ + ( + "float", + "Float64", + pd.array([True], dtype=bool), + pd.array([pd.NA], dtype="boolean"), + ), + ( + "Int64", + "float", + pd.array([pd.NA], dtype="boolean"), + pd.array([True], dtype=bool), + ), + ( + "Int64", + "Int64", + pd.array([pd.NA], dtype="boolean"), + pd.array([pd.NA], dtype="boolean"), + ), + ( + "Float64", + "boolean", + pd.array([pd.NA], dtype="boolean"), + pd.array([pd.NA], dtype="boolean"), + ), + ], +) +def test_masked_mixed_types(dtype1, dtype2, exp_col1, exp_col2): + data = [1.0, np.nan] + df = DataFrame( + {"col1": pd.array(data, dtype=dtype1), "col2": pd.array(data, dtype=dtype2)} + ) + result = df.groupby([1, 1]).agg("all", skipna=False) + + expected = DataFrame({"col1": exp_col1, "col2": exp_col2}, index=[1]) + tm.assert_frame_equal(result, expected) + + @pytest.mark.parametrize("bool_agg_func", ["any", "all"]) @pytest.mark.parametrize("dtype", ["Int64", "Float64", "boolean"]) @pytest.mark.parametrize("skipna", [True, False]) From 80a65bba2051ea806a9789c182b5f56525891234 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Thu, 8 Apr 2021 13:01:01 -0400 Subject: [PATCH 18/31] Fix post proc check --- pandas/core/groupby/groupby.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 5aa8856d08cfb..f3e3f6c976270 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -2702,7 +2702,7 @@ def _get_cythonized_result( output: dict[base.OutputKey, np.ndarray] = {} base_func = getattr(libgroupby, how) - post_processing_accepts_masked = ( + post_processing_accepts_masked = post_processing is not None and ( "masked" in inspect.signature(post_processing).parameters ) kwargs_accepts_masked = "masked" in kwargs From c9b9d5f25820523898970be1a746f752b8e0dc4b Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Thu, 8 Apr 2021 16:13:02 -0400 Subject: [PATCH 19/31] Address review comments --- pandas/core/groupby/groupby.py | 15 +++++++++------ pandas/tests/groupby/test_any_all.py | 15 +++++++++++---- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index f3e3f6c976270..50e9d3ee0e77c 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -77,11 +77,11 @@ class providing the base-class of operations. from pandas.core import nanops import pandas.core.algorithms as algorithms from pandas.core.arrays import ( + BaseMaskedArray, + BooleanArray, Categorical, ExtensionArray, ) -from pandas.core.arrays.boolean import BooleanArray -from pandas.core.arrays.masked import BaseMaskedArray from pandas.core.base import ( DataError, PandasObject, @@ -1444,9 +1444,9 @@ def result_to_bool( needs_mask=True, pre_processing=objs_to_bool, post_processing=result_to_bool, + use_nullable_input_arg=True, val_test=val_test, skipna=skipna, - masked=False, ) @final @@ -2630,6 +2630,7 @@ def _get_cythonized_result( result_is_index: bool = False, pre_processing=None, post_processing=None, + use_nullable_input_arg: bool = False, **kwargs, ): """ @@ -2677,6 +2678,9 @@ def _get_cythonized_result( second argument, i.e. the signature should be (ndarray, Type). Optionally, a third argument can be "masked", to allow for processing specific to nullable values + use_nullable_input_arg : bool, default False + If True, pass an argument "masked" to the cython function specifying + whether or not the input should be treated as masked **kwargs : dict Extra arguments to be passed back to Cython funcs @@ -2705,7 +2709,6 @@ def _get_cythonized_result( post_processing_accepts_masked = post_processing is not None and ( "masked" in inspect.signature(post_processing).parameters ) - kwargs_accepts_masked = "masked" in kwargs error_msg = "" for idx, obj in enumerate(self._iterate_slices()): @@ -2757,8 +2760,8 @@ def _get_cythonized_result( if needs_ngroups: func = partial(func, ngroups) - if kwargs_accepts_masked: - kwargs["masked"] = is_nullable + if use_nullable_input_arg: + func = partial(func, masked=is_nullable) func(**kwargs) # Call func to modify indexer values in place diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index 97bbce763b4d4..bb23b3bbc0d3a 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -74,8 +74,9 @@ def test_bool_aggs_dup_column_labels(bool_agg_func): @pytest.mark.parametrize("bool_agg_func", ["any", "all"]) @pytest.mark.parametrize("skipna", [True, False]) @pytest.mark.parametrize( - # expected indexed as [skipna][bool_agg_func == "all"] - "data,expected", + # expected_data indexed as [[skipna=False/any, skipna=False/all], + # [skipna=True/any, skipna=True/all]] + "data,expected_data", [ ([False, False, False], [[False, False], [False, False]]), ([True, True, True], [[True, True], [True, True]]), @@ -85,16 +86,22 @@ def test_bool_aggs_dup_column_labels(bool_agg_func): ([True, pd.NA, False], [[True, False], [True, False]]), ], ) -def test_masked_kleene_logic(bool_agg_func, data, expected, skipna): +def test_masked_kleene_logic(bool_agg_func, data, expected_data, skipna): # GH#37506 df = DataFrame(data, dtype="boolean") expected = DataFrame( - [expected[skipna][bool_agg_func == "all"]], dtype="boolean", index=[1] + [expected_data[skipna][bool_agg_func == "all"]], dtype="boolean", index=[1] ) result = df.groupby([1, 1, 1]).agg(bool_agg_func, skipna=skipna) tm.assert_frame_equal(result, expected) + # The expected result we compared to should match aggregating on the whole + # series + result = getattr(df[0], bool_agg_func)(skipna=skipna) + expected = expected_data[skipna][bool_agg_func == "all"] + assert (result is pd.NA and expected is pd.NA) or result == expected + @pytest.mark.parametrize( "dtype1,dtype2,exp_col1,exp_col2", From a116bedaeadc6bc3b562e5edadbeb7328d50f0ac Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Sat, 10 Apr 2021 10:46:41 -0400 Subject: [PATCH 20/31] Name arguments better --- pandas/_libs/groupby.pyx | 13 +++++++------ pandas/core/groupby/groupby.py | 14 +++++++------- pandas/tests/groupby/test_any_all.py | 1 + 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/pandas/_libs/groupby.pyx b/pandas/_libs/groupby.pyx index e869b6cace1d1..f567a17ad2272 100644 --- a/pandas/_libs/groupby.pyx +++ b/pandas/_libs/groupby.pyx @@ -394,10 +394,11 @@ def group_any_all(uint8_t[::1] out, const uint8_t[::1] mask, str val_test, bint skipna, - bint masked) -> None: + bint nullable) -> None: """ Aggregated boolean values to show truthfulness of group elements. If the - input is a nullable type, Kleene logic will be used. + input is a nullable type (nullable=True), the result will be computed + using Kleene logic. Parameters ---------- @@ -414,8 +415,9 @@ def group_any_all(uint8_t[::1] out, String object dictating whether to use any or all truth testing skipna : bool Flag to ignore nan values during truth testing - masked : bool - If True, compute the result using Kleene logic + nullable : bool + Whether or not the input is a nullable type. If True, the + result will be computed using Kleene logic Notes ----- @@ -427,7 +429,6 @@ def group_any_all(uint8_t[::1] out, Py_ssize_t i, N = len(labels) intp_t lab uint8_t flag_val - bint use_kleene_logic = masked if val_test == 'all': # Because the 'all' value of an empty iterable in Python is True we can @@ -450,7 +451,7 @@ def group_any_all(uint8_t[::1] out, if lab < 0 or (skipna and mask[i]): continue - if use_kleene_logic and mask[i]: + if nullable and mask[i]: # Set the position as masked if `out[lab] != flag_val`, which # would indicate True/False has not yet been seen for any/all, # so by Kleene logic the result is currently unknown diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 53a0edbf81d72..4601458ac07cd 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1442,9 +1442,9 @@ def result_to_bool( cython_dtype=np.dtype(np.uint8), needs_values=True, needs_mask=True, + needs_nullable=True, pre_processing=objs_to_bool, post_processing=result_to_bool, - use_nullable_input_arg=True, val_test=val_test, skipna=skipna, ) @@ -2625,13 +2625,13 @@ def _get_cythonized_result( needs_counts: bool = False, needs_values: bool = False, needs_2d: bool = False, + needs_nullable: bool = False, min_count: int | None = None, needs_mask: bool = False, needs_ngroups: bool = False, result_is_index: bool = False, pre_processing=None, post_processing=None, - use_nullable_input_arg: bool = False, **kwargs, ): """ @@ -2662,6 +2662,9 @@ def _get_cythonized_result( signature needs_ngroups : bool, default False Whether number of groups is part of the Cython call signature + needs_nullable : bool, default False + Whether a bool representing if the input is nullable is part + of the Cython call signature result_is_index : bool, default False Whether the result of the Cython operation is an index of values to be retrieved, instead of the actual values themselves @@ -2679,9 +2682,6 @@ def _get_cythonized_result( second argument, i.e. the signature should be (ndarray, Type). Optionally, a third argument can be "masked", to allow for processing specific to nullable values - use_nullable_input_arg : bool, default False - If True, pass an argument "masked" to the cython function specifying - whether or not the input should be treated as masked **kwargs : dict Extra arguments to be passed back to Cython funcs @@ -2761,8 +2761,8 @@ def _get_cythonized_result( if needs_ngroups: func = partial(func, ngroups) - if use_nullable_input_arg: - func = partial(func, masked=is_nullable) + if needs_nullable: + func = partial(func, nullable=is_nullable) func(**kwargs) # Call func to modify indexer values in place diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index bb23b3bbc0d3a..a93fdb3863b6f 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -133,6 +133,7 @@ def test_masked_kleene_logic(bool_agg_func, data, expected_data, skipna): ], ) def test_masked_mixed_types(dtype1, dtype2, exp_col1, exp_col2): + # GH#37506 data = [1.0, np.nan] df = DataFrame( {"col1": pd.array(data, dtype=dtype1), "col2": pd.array(data, dtype=dtype2)} From 8a428d4c96549f1466c5876c45726a555fabdef9 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Sat, 10 Apr 2021 10:58:57 -0400 Subject: [PATCH 21/31] Use -1 as mask signal --- pandas/_libs/groupby.pyx | 12 ++++++------ pandas/core/groupby/groupby.py | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pandas/_libs/groupby.pyx b/pandas/_libs/groupby.pyx index f567a17ad2272..7a7dcee0d9726 100644 --- a/pandas/_libs/groupby.pyx +++ b/pandas/_libs/groupby.pyx @@ -388,8 +388,8 @@ def group_fillna_indexer(ndarray[int64_t] out, ndarray[intp_t] labels, @cython.boundscheck(False) @cython.wraparound(False) -def group_any_all(uint8_t[::1] out, - const uint8_t[::1] values, +def group_any_all(int8_t[::1] out, + const int8_t[::1] values, const intp_t[:] labels, const uint8_t[::1] mask, str val_test, @@ -402,12 +402,12 @@ def group_any_all(uint8_t[::1] out, Parameters ---------- - out : np.ndarray[np.uint8] + out : np.ndarray[np.int8] Values into which this method will write its results. labels : np.ndarray[np.intp] Array containing unique label for each group, with its ordering matching up to the corresponding record in `values` - values : np.ndarray[np.uint8] + values : np.ndarray[np.int8] Containing the truth value of each element. mask : np.ndarray[np.uint8] Indicating whether a value is na or not. @@ -423,7 +423,7 @@ def group_any_all(uint8_t[::1] out, ----- This method modifies the `out` parameter rather than returning an object. The returned values will either be 0, 1 (False or True, respectively), or - 2 to signify a masked position in the case of a nullable input. + -1 to signify a masked position in the case of a nullable input. """ cdef: Py_ssize_t i, N = len(labels) @@ -456,7 +456,7 @@ def group_any_all(uint8_t[::1] out, # would indicate True/False has not yet been seen for any/all, # so by Kleene logic the result is currently unknown if out[lab] != flag_val: - out[lab] = 2 + out[lab] = -1 continue # If True and 'any' or False and 'all', the result is diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 4601458ac07cd..81d3f24206a6f 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1423,7 +1423,7 @@ def objs_to_bool(vals: ArrayLike) -> tuple[np.ndarray, type]: else: vals = vals.astype(bool) - return vals.view(np.uint8), bool + return vals.view(np.int8), bool def result_to_bool( result: np.ndarray, @@ -1431,7 +1431,7 @@ def result_to_bool( masked: bool = False, ) -> ArrayLike: if masked: - return BooleanArray(result.astype(bool, copy=False), result == 2) + return BooleanArray(result.astype(bool, copy=False), result == -1) else: return result.astype(inference, copy=False) @@ -1439,7 +1439,7 @@ def result_to_bool( "group_any_all", aggregate=True, numeric_only=False, - cython_dtype=np.dtype(np.uint8), + cython_dtype=np.dtype(np.int8), needs_values=True, needs_mask=True, needs_nullable=True, From 8e3c5be39cca88c3f6037fb4934c8d0551ea515c Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Sat, 10 Apr 2021 12:43:50 -0400 Subject: [PATCH 22/31] Consistent typing --- pandas/_libs/groupby.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/_libs/groupby.pyx b/pandas/_libs/groupby.pyx index 7a7dcee0d9726..29e74fe4c0d8f 100644 --- a/pandas/_libs/groupby.pyx +++ b/pandas/_libs/groupby.pyx @@ -428,7 +428,7 @@ def group_any_all(int8_t[::1] out, cdef: Py_ssize_t i, N = len(labels) intp_t lab - uint8_t flag_val + int8_t flag_val if val_test == 'all': # Because the 'all' value of an empty iterable in Python is True we can From b6276182cd30c1ad3845f786ca9ce63eedf0e475 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Sat, 10 Apr 2021 13:21:54 -0400 Subject: [PATCH 23/31] Don't use inspect --- pandas/core/groupby/groupby.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 81d3f24206a6f..aa4fc6424e6d3 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1428,9 +1428,9 @@ def objs_to_bool(vals: ArrayLike) -> tuple[np.ndarray, type]: def result_to_bool( result: np.ndarray, inference: type, - masked: bool = False, + nullable: bool = False, ) -> ArrayLike: - if masked: + if nullable: return BooleanArray(result.astype(bool, copy=False), result == -1) else: return result.astype(inference, copy=False) @@ -2680,8 +2680,8 @@ def _get_cythonized_result( Function to be applied to result of Cython function. Should accept an array of values as the first argument and type inferences as its second argument, i.e. the signature should be - (ndarray, Type). Optionally, a third argument can be "masked", to - allow for processing specific to nullable values + (ndarray, Type). A third argument should be "nullable", to + allow for processing specific to nullable values if needs_nullable=True **kwargs : dict Extra arguments to be passed back to Cython funcs @@ -2707,16 +2707,11 @@ def _get_cythonized_result( output: dict[base.OutputKey, np.ndarray] = {} base_func = getattr(libgroupby, how) - post_processing_accepts_masked = post_processing is not None and ( - "masked" in inspect.signature(post_processing).parameters - ) - error_msg = "" for idx, obj in enumerate(self._iterate_slices()): name = obj.name values = obj._values - is_nullable = isinstance(values, BaseMaskedArray) - + if numeric_only and not is_numeric_dtype(values): continue @@ -2762,7 +2757,10 @@ def _get_cythonized_result( func = partial(func, ngroups) if needs_nullable: + is_nullable = isinstance(values, BaseMaskedArray) func = partial(func, nullable=is_nullable) + if post_processing: + post_processing = partial(post_processing, nullable=is_nullable) func(**kwargs) # Call func to modify indexer values in place @@ -2773,8 +2771,6 @@ def _get_cythonized_result( result = algorithms.take_nd(values, result) if post_processing: - if post_processing_accepts_masked: - post_processing = partial(post_processing, masked=is_nullable) result = post_processing(result, inferences) key = base.OutputKey(label=name, position=idx) From 23b3b64b77ff56fbb50a02df9e945d459d76adc4 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Sat, 10 Apr 2021 13:34:20 -0400 Subject: [PATCH 24/31] precommit fixup --- pandas/core/groupby/groupby.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index aa4fc6424e6d3..6060d9455be97 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -2711,7 +2711,7 @@ def _get_cythonized_result( for idx, obj in enumerate(self._iterate_slices()): name = obj.name values = obj._values - + if numeric_only and not is_numeric_dtype(values): continue From a30496c1eacb5913efec7c1a15cb8e558cc8730c Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Sat, 10 Apr 2021 13:41:14 -0400 Subject: [PATCH 25/31] Clean up docstring --- pandas/core/groupby/groupby.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 6060d9455be97..43db889618db6 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -2663,7 +2663,7 @@ def _get_cythonized_result( needs_ngroups : bool, default False Whether number of groups is part of the Cython call signature needs_nullable : bool, default False - Whether a bool representing if the input is nullable is part + Whether a bool specifying if the input is nullable is part of the Cython call signature result_is_index : bool, default False Whether the result of the Cython operation is an index of @@ -2680,8 +2680,8 @@ def _get_cythonized_result( Function to be applied to result of Cython function. Should accept an array of values as the first argument and type inferences as its second argument, i.e. the signature should be - (ndarray, Type). A third argument should be "nullable", to - allow for processing specific to nullable values if needs_nullable=True + (ndarray, Type). If `needs_nullable=True`, a third argument should be + `nullable`, to allow for processing specific to nullable values. **kwargs : dict Extra arguments to be passed back to Cython funcs From 4cd283309821d5d345a324dafd19eb34d17aeaf3 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin <37011898+mzeitlin11@users.noreply.github.com> Date: Tue, 13 Apr 2021 10:29:17 -0400 Subject: [PATCH 26/31] Update doc/source/whatsnew/v1.3.0.rst Co-authored-by: Joris Van den Bossche --- doc/source/whatsnew/v1.3.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.3.0.rst b/doc/source/whatsnew/v1.3.0.rst index ffc82ec726608..f557e77ac80ef 100644 --- a/doc/source/whatsnew/v1.3.0.rst +++ b/doc/source/whatsnew/v1.3.0.rst @@ -213,7 +213,7 @@ Other enhancements - :class:`RangeIndex` can now be constructed by passing a ``range`` object directly e.g. ``pd.RangeIndex(range(3))`` (:issue:`12067`) - :meth:`round` being enabled for the nullable integer and floating dtypes (:issue:`38844`) - :meth:`pandas.read_csv` and :meth:`pandas.read_json` expose the argument ``encoding_errors`` to control how encoding errors are handled (:issue:`39450`) -- :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` use Kleene logic for methods ``any`` and ``all`` with nullable data types (:issue:`37506`) +- :meth:`.GroupBy.any` and :meth:`.GroupBy.all` use Kleene logic with nullable data types (:issue:`37506`) - :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` return a ``BooleanDType`` for methods ``any`` and ``all`` with nullable data types (:issue:`33449`) - From 98cd40118e490a6f8606657ac43cfd9536061051 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin <37011898+mzeitlin11@users.noreply.github.com> Date: Tue, 13 Apr 2021 10:29:42 -0400 Subject: [PATCH 27/31] Update doc/source/whatsnew/v1.3.0.rst Co-authored-by: Joris Van den Bossche --- doc/source/whatsnew/v1.3.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.3.0.rst b/doc/source/whatsnew/v1.3.0.rst index f557e77ac80ef..dc16a0a79e972 100644 --- a/doc/source/whatsnew/v1.3.0.rst +++ b/doc/source/whatsnew/v1.3.0.rst @@ -214,7 +214,7 @@ Other enhancements - :meth:`round` being enabled for the nullable integer and floating dtypes (:issue:`38844`) - :meth:`pandas.read_csv` and :meth:`pandas.read_json` expose the argument ``encoding_errors`` to control how encoding errors are handled (:issue:`39450`) - :meth:`.GroupBy.any` and :meth:`.GroupBy.all` use Kleene logic with nullable data types (:issue:`37506`) -- :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` return a ``BooleanDType`` for methods ``any`` and ``all`` with nullable data types (:issue:`33449`) +- :meth:`.GroupBy.any` and :meth:`.GroupBy.all` return a ``BooleanDtype`` for columns with nullable data types (:issue:`33449`) - .. --------------------------------------------------------------------------- From a92c637ec482ee92109b25a73eece23c13254468 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin <37011898+mzeitlin11@users.noreply.github.com> Date: Tue, 13 Apr 2021 10:29:50 -0400 Subject: [PATCH 28/31] Update doc/source/whatsnew/v1.3.0.rst Co-authored-by: Joris Van den Bossche --- doc/source/whatsnew/v1.3.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v1.3.0.rst b/doc/source/whatsnew/v1.3.0.rst index dc16a0a79e972..75c4c580c495b 100644 --- a/doc/source/whatsnew/v1.3.0.rst +++ b/doc/source/whatsnew/v1.3.0.rst @@ -785,7 +785,7 @@ Groupby/resample/rolling - Bug in :meth:`Series.asfreq` and :meth:`DataFrame.asfreq` dropping rows when the index is not sorted (:issue:`39805`) - Bug in aggregation functions for :class:`DataFrame` not respecting ``numeric_only`` argument when ``level`` keyword was given (:issue:`40660`) - Bug in :class:`core.window.RollingGroupby` where ``as_index=False`` argument in ``groupby`` was ignored (:issue:`39433`) -- Bug in :class:`SeriesGroupBy` and :class:`DataFrameGroupBy` raising ``ValueError`` when using methods ``any`` and ``all`` with nullable type columns holding ``NA`` even with ``skipna=True`` (:issue:`40585`) +- Bug in :meth:`.GroupBy.any` and :meth:`.GroupBy.all` raising ``ValueError`` when using with nullable type columns holding ``NA`` even with ``skipna=True`` (:issue:`40585`) Reshaping From 7c5c8e6152d955c4f1d41ddf10796f2453b4ba7e Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin <37011898+mzeitlin11@users.noreply.github.com> Date: Tue, 13 Apr 2021 10:30:21 -0400 Subject: [PATCH 29/31] Update pandas/tests/groupby/test_any_all.py Co-authored-by: Joris Van den Bossche --- pandas/tests/groupby/test_any_all.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index a93fdb3863b6f..7b69322376d20 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -109,7 +109,7 @@ def test_masked_kleene_logic(bool_agg_func, data, expected_data, skipna): ( "float", "Float64", - pd.array([True], dtype=bool), + np.array([True], dtype=bool), pd.array([pd.NA], dtype="boolean"), ), ( From c66d1fd74e0b770870acbbc88dfff46b467ce3b7 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin <37011898+mzeitlin11@users.noreply.github.com> Date: Tue, 13 Apr 2021 10:30:27 -0400 Subject: [PATCH 30/31] Update pandas/tests/groupby/test_any_all.py Co-authored-by: Joris Van den Bossche --- pandas/tests/groupby/test_any_all.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index 7b69322376d20..285278a18fae5 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -116,7 +116,7 @@ def test_masked_kleene_logic(bool_agg_func, data, expected_data, skipna): "Int64", "float", pd.array([pd.NA], dtype="boolean"), - pd.array([True], dtype=bool), + np.array([True], dtype=bool), ), ( "Int64", From c81c1a59d7dff7ac48774cbeb5a8d7f0bf69bfa1 Mon Sep 17 00:00:00 2001 From: Matthew Zeitlin Date: Tue, 13 Apr 2021 11:01:39 -0400 Subject: [PATCH 31/31] Simplify teasts --- pandas/tests/groupby/test_any_all.py | 37 ++++++++++------------ pandas/tests/reductions/test_reductions.py | 37 ++++++++++++++-------- 2 files changed, 39 insertions(+), 35 deletions(-) diff --git a/pandas/tests/groupby/test_any_all.py b/pandas/tests/groupby/test_any_all.py index a93fdb3863b6f..81791cdd3dd6e 100644 --- a/pandas/tests/groupby/test_any_all.py +++ b/pandas/tests/groupby/test_any_all.py @@ -7,6 +7,7 @@ from pandas import ( DataFrame, Index, + Series, isna, ) import pandas._testing as tm @@ -74,33 +75,27 @@ def test_bool_aggs_dup_column_labels(bool_agg_func): @pytest.mark.parametrize("bool_agg_func", ["any", "all"]) @pytest.mark.parametrize("skipna", [True, False]) @pytest.mark.parametrize( - # expected_data indexed as [[skipna=False/any, skipna=False/all], - # [skipna=True/any, skipna=True/all]] - "data,expected_data", + "data", [ - ([False, False, False], [[False, False], [False, False]]), - ([True, True, True], [[True, True], [True, True]]), - ([pd.NA, pd.NA, pd.NA], [[pd.NA, pd.NA], [False, True]]), - ([False, pd.NA, False], [[pd.NA, False], [False, False]]), - ([True, pd.NA, True], [[True, pd.NA], [True, True]]), - ([True, pd.NA, False], [[True, False], [True, False]]), + [False, False, False], + [True, True, True], + [pd.NA, pd.NA, pd.NA], + [False, pd.NA, False], + [True, pd.NA, True], + [True, pd.NA, False], ], ) -def test_masked_kleene_logic(bool_agg_func, data, expected_data, skipna): +def test_masked_kleene_logic(bool_agg_func, skipna, data): # GH#37506 - df = DataFrame(data, dtype="boolean") - expected = DataFrame( - [expected_data[skipna][bool_agg_func == "all"]], dtype="boolean", index=[1] - ) + ser = Series(data, dtype="boolean") - result = df.groupby([1, 1, 1]).agg(bool_agg_func, skipna=skipna) - tm.assert_frame_equal(result, expected) + # The result should match aggregating on the whole series. Correctness + # there is verified in test_reductions.py::test_any_all_boolean_kleene_logic + expected_data = getattr(ser, bool_agg_func)(skipna=skipna) + expected = Series(expected_data, dtype="boolean") - # The expected result we compared to should match aggregating on the whole - # series - result = getattr(df[0], bool_agg_func)(skipna=skipna) - expected = expected_data[skipna][bool_agg_func == "all"] - assert (result is pd.NA and expected is pd.NA) or result == expected + result = ser.groupby([0, 0, 0]).agg(bool_agg_func, skipna=skipna) + tm.assert_series_equal(result, expected) @pytest.mark.parametrize( diff --git a/pandas/tests/reductions/test_reductions.py b/pandas/tests/reductions/test_reductions.py index e315c6f8ac465..906f05fe0f348 100644 --- a/pandas/tests/reductions/test_reductions.py +++ b/pandas/tests/reductions/test_reductions.py @@ -941,20 +941,29 @@ def test_all_any_params(self): with pytest.raises(NotImplementedError, match=msg): s.all(bool_only=True) - def test_all_any_boolean(self): - # Check skipna, with boolean type - s1 = Series([pd.NA, True], dtype="boolean") - s2 = Series([pd.NA, False], dtype="boolean") - assert s1.all(skipna=False) is pd.NA # NA && True => NA - assert s1.all(skipna=True) - assert s2.any(skipna=False) is pd.NA # NA || False => NA - assert not s2.any(skipna=True) - - # GH-33253: all True / all False values buggy with skipna=False - s3 = Series([True, True], dtype="boolean") - s4 = Series([False, False], dtype="boolean") - assert s3.all(skipna=False) - assert not s4.any(skipna=False) + @pytest.mark.parametrize("bool_agg_func", ["any", "all"]) + @pytest.mark.parametrize("skipna", [True, False]) + @pytest.mark.parametrize( + # expected_data indexed as [[skipna=False/any, skipna=False/all], + # [skipna=True/any, skipna=True/all]] + "data,expected_data", + [ + ([False, False, False], [[False, False], [False, False]]), + ([True, True, True], [[True, True], [True, True]]), + ([pd.NA, pd.NA, pd.NA], [[pd.NA, pd.NA], [False, True]]), + ([False, pd.NA, False], [[pd.NA, False], [False, False]]), + ([True, pd.NA, True], [[True, pd.NA], [True, True]]), + ([True, pd.NA, False], [[True, False], [True, False]]), + ], + ) + def test_any_all_boolean_kleene_logic( + self, bool_agg_func, skipna, data, expected_data + ): + ser = Series(data, dtype="boolean") + expected = expected_data[skipna][bool_agg_func == "all"] + + result = getattr(ser, bool_agg_func)(skipna=skipna) + assert (result is pd.NA and expected is pd.NA) or result == expected @pytest.mark.parametrize( "bool_agg_func,expected",