From ec577d7fefbc5dee0f295398433500b0c331762c Mon Sep 17 00:00:00 2001 From: smithto1 Date: Sat, 11 Jul 2020 21:29:00 +0100 Subject: [PATCH 01/28] set self.observed=True for .sum() and .count() so that reindex can be called with 0 --- pandas/core/groupby/generic.py | 18 +++++++++- pandas/core/groupby/groupby.py | 26 +++++++++++++-- pandas/tests/groupby/test_categorical.py | 42 ++++++------------------ 3 files changed, 50 insertions(+), 36 deletions(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index 1f49ee2b0b665..2b656e3035d0e 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1820,7 +1820,23 @@ def count(self): ) blocks = [make_block(val, placement=loc) for val, loc in zip(counted, locs)] - return self._wrap_agged_blocks(blocks, items=data.items) + # If self.observed=False then self._reindex_output will fill the missing + # categories (if the grouper was grouped on pd.Categorical variables) with + # np.NaN. For .count() we want these values filled in with zero. So we set + # self.observed=True for the call to self._agg_general, and then set + # it back to its orignal value. We then call self._reindex_output with + # fill_value=0. If the original self.observed is False, then we will get + # our result with 0 for the missing categories. + observed_orig = self.observed + self.observed = True + try: + result = self._wrap_agged_blocks(blocks, items=data.items) + except Exception as e: + raise e + finally: + self.observed = observed_orig + + return self._reindex_output(result, fill_value=0) def nunique(self, dropna: bool = True): """ diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index d039b715b3c08..c7600327f46ef 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1531,9 +1531,29 @@ def size(self) -> FrameOrSeriesUnion: @doc(_groupby_agg_method_template, fname="sum", no=True, mc=0) def sum(self, numeric_only: bool = True, min_count: int = 0): - return self._agg_general( - numeric_only=numeric_only, min_count=min_count, alias="add", npfunc=np.sum - ) + + # If self.observed=False then self._reindex_output will fill the missing + # categories (if the grouper was grouped on pd.Categorical variables) with + # np.NaN. For .sum() we want these values filled in with zero. So we set + # self.observed=True for the call to self._agg_general, and then set + # it back to its orignal value. We then call self._reindex_output with + # fill_value=0. If the original self.observed is False, then we will get + # our result with 0 for the missing categories. + observed_orig = self.observed + self.observed = True + try: + result = self._agg_general( + numeric_only=numeric_only, + min_count=min_count, + alias="add", + npfunc=np.sum, + ) + except Exception as e: + raise e + finally: + self.observed = observed_orig + + return self._reindex_output(result, fill_value=0) @doc(_groupby_agg_method_template, fname="prod", no=True, mc=0) def prod(self, numeric_only: bool = True, min_count: int = 0): diff --git a/pandas/tests/groupby/test_categorical.py b/pandas/tests/groupby/test_categorical.py index 118d928ac02f4..d96efe1c2c732 100644 --- a/pandas/tests/groupby/test_categorical.py +++ b/pandas/tests/groupby/test_categorical.py @@ -19,7 +19,7 @@ import pandas._testing as tm -def cartesian_product_for_groupers(result, args, names): +def cartesian_product_for_groupers(result, args, names, fill_value=np.NaN): """ Reindex to a cartesian production for the groupers, preserving the nature (Categorical) of each grouper """ @@ -33,7 +33,7 @@ def f(a): return a index = MultiIndex.from_product(map(f, args), names=names) - return result.reindex(index).sort_index() + return result.reindex(index, fill_value=fill_value).sort_index() _results_for_groupbys_with_missing_categories = dict( @@ -309,7 +309,7 @@ def test_observed(observed): result = gb.sum() if not observed: expected = cartesian_product_for_groupers( - expected, [cat1, cat2, ["foo", "bar"]], list("ABC") + expected, [cat1, cat2, ["foo", "bar"]], list("ABC"), fill_value=0 ) tm.assert_frame_equal(result, expected) @@ -319,7 +319,9 @@ def test_observed(observed): expected = DataFrame({"values": [1, 2, 3, 4]}, index=exp_index) result = gb.sum() if not observed: - expected = cartesian_product_for_groupers(expected, [cat1, cat2], list("AB")) + expected = cartesian_product_for_groupers( + expected, [cat1, cat2], list("AB"), fill_value=0 + ) tm.assert_frame_equal(result, expected) @@ -1189,8 +1191,11 @@ def test_seriesgroupby_observed_false_or_none(df_cat, observed, operation): ).sortlevel() expected = Series(data=[2, 4, np.nan, 1, np.nan, 3], index=index, name="C") + if operation == "agg": + expected = expected.fillna(0, downcast="infer") grouped = df_cat.groupby(["A", "B"], observed=observed)["C"] result = getattr(grouped, operation)(sum) + tm.assert_series_equal(result, expected) @@ -1340,15 +1345,6 @@ def test_series_groupby_on_2_categoricals_unobserved_zeroes_or_nans( ) request.node.add_marker(mark) - if reduction_func == "sum": # GH 31422 - mark = pytest.mark.xfail( - reason=( - "sum should return 0 but currently returns NaN. " - "This is a known bug. See GH 31422." - ) - ) - request.node.add_marker(mark) - df = pd.DataFrame( { "cat_1": pd.Categorical(list("AABB"), categories=list("ABC")), @@ -1370,7 +1366,7 @@ def test_series_groupby_on_2_categoricals_unobserved_zeroes_or_nans( assert (pd.isna(zero_or_nan) and pd.isna(val)) or (val == zero_or_nan) # If we expect unobserved values to be zero, we also expect the dtype to be int - if zero_or_nan == 0: + if zero_or_nan == 0 and reduction_func != "sum": assert np.issubdtype(result.dtype, np.integer) @@ -1412,24 +1408,6 @@ def test_dataframe_groupby_on_2_categoricals_when_observed_is_false( if reduction_func == "ngroup": pytest.skip("ngroup does not return the Categories on the index") - if reduction_func == "count": # GH 35028 - mark = pytest.mark.xfail( - reason=( - "DataFrameGroupBy.count returns np.NaN for missing " - "categories, when it should return 0. See GH 35028" - ) - ) - request.node.add_marker(mark) - - if reduction_func == "sum": # GH 31422 - mark = pytest.mark.xfail( - reason=( - "sum should return 0 but currently returns NaN. " - "This is a known bug. See GH 31422." - ) - ) - request.node.add_marker(mark) - df = pd.DataFrame( { "cat_1": pd.Categorical(list("AABB"), categories=list("ABC")), From 586b1bd386cdd7b07ddb72e89959360209ed5b88 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Sat, 11 Jul 2020 22:39:35 +0100 Subject: [PATCH 02/28] wrote test for .sum() and .count() exception handling --- pandas/tests/groupby/test_categorical.py | 28 +++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/pandas/tests/groupby/test_categorical.py b/pandas/tests/groupby/test_categorical.py index d96efe1c2c732..b04e8953360cb 100644 --- a/pandas/tests/groupby/test_categorical.py +++ b/pandas/tests/groupby/test_categorical.py @@ -1365,7 +1365,10 @@ def test_series_groupby_on_2_categoricals_unobserved_zeroes_or_nans( val = result.loc[idx] assert (pd.isna(zero_or_nan) and pd.isna(val)) or (val == zero_or_nan) - # If we expect unobserved values to be zero, we also expect the dtype to be int + # If we expect unobserved values to be zero, we also expect the dtype to be int. + # Except for .sum(). If the observed categories sum to dtype=float (i.e. their + # sums have decimals), then the zeros for the missing categories should also be + # floats. if zero_or_nan == 0 and reduction_func != "sum": assert np.issubdtype(result.dtype, np.integer) @@ -1430,6 +1433,29 @@ def test_dataframe_groupby_on_2_categoricals_when_observed_is_false( assert (res.loc[unobserved_cats] == expected).all().all() +@pytest.mark.parametrize('func', ['sum', 'count']) +def test_sum_and_count_exception_handling(func: str, observed: bool, monkeypatch): + df = pd.DataFrame( + { + "cat_1": pd.Categorical(list("AABB"), categories=list("ABC")), + "cat_2": pd.Categorical(list("1111"), categories=list("12")), + "value": [0.1, 0.1, 0.1, 0.1], + } + ) + df_grp = df.groupby(["cat_1", "cat_2"], observed=observed) + + def _mock_method(*args, **kwargs): + raise ZeroDivisionError + + to_patch = {'count' : '_wrap_agged_blocks', 'sum' : '_agg_general'} + monkeypatch.setattr(df_grp, to_patch[func], _mock_method) + + with pytest.raises(ZeroDivisionError): + getattr(df_grp, func)() + + assert df_grp.observed is observed + + def test_series_groupby_categorical_aggregation_getitem(): # GH 8870 d = {"foo": [10, 8, 4, 1], "bar": [10, 20, 30, 40], "baz": ["d", "c", "d", "c"]} From ec4bd286ac9501a46eea4c4365fe10c1019c5d97 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Sat, 11 Jul 2020 22:40:12 +0100 Subject: [PATCH 03/28] black --- pandas/tests/groupby/test_categorical.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pandas/tests/groupby/test_categorical.py b/pandas/tests/groupby/test_categorical.py index b04e8953360cb..4c83ecbc53c3d 100644 --- a/pandas/tests/groupby/test_categorical.py +++ b/pandas/tests/groupby/test_categorical.py @@ -1433,7 +1433,7 @@ def test_dataframe_groupby_on_2_categoricals_when_observed_is_false( assert (res.loc[unobserved_cats] == expected).all().all() -@pytest.mark.parametrize('func', ['sum', 'count']) +@pytest.mark.parametrize("func", ["sum", "count"]) def test_sum_and_count_exception_handling(func: str, observed: bool, monkeypatch): df = pd.DataFrame( { @@ -1443,17 +1443,17 @@ def test_sum_and_count_exception_handling(func: str, observed: bool, monkeypatch } ) df_grp = df.groupby(["cat_1", "cat_2"], observed=observed) - + def _mock_method(*args, **kwargs): raise ZeroDivisionError - - to_patch = {'count' : '_wrap_agged_blocks', 'sum' : '_agg_general'} + + to_patch = {"count": "_wrap_agged_blocks", "sum": "_agg_general"} monkeypatch.setattr(df_grp, to_patch[func], _mock_method) - + with pytest.raises(ZeroDivisionError): getattr(df_grp, func)() - assert df_grp.observed is observed + assert df_grp.observed is observed def test_series_groupby_categorical_aggregation_getitem(): From eed48cb021bdc157185162f0345c56db00099d17 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Sat, 11 Jul 2020 22:49:53 +0100 Subject: [PATCH 04/28] updated pivot tests --- pandas/tests/reshape/test_pivot.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pandas/tests/reshape/test_pivot.py b/pandas/tests/reshape/test_pivot.py index c07a5673fe503..67b3151b0ff9c 100644 --- a/pandas/tests/reshape/test_pivot.py +++ b/pandas/tests/reshape/test_pivot.py @@ -1817,7 +1817,7 @@ def test_categorical_aggfunc(self, observed): ["A", "B", "C"], categories=["A", "B", "C"], ordered=False, name="C1" ) expected_columns = pd.Index(["a", "b"], name="C2") - expected_data = np.array([[1.0, np.nan], [1.0, np.nan], [np.nan, 2.0]]) + expected_data = np.array([[1, 0], [1, 0], [0, 2]], dtype=np.int64) expected = pd.DataFrame( expected_data, index=expected_index, columns=expected_columns ) @@ -1851,18 +1851,19 @@ def test_categorical_pivot_index_ordering(self, observed): values="Sales", index="Month", columns="Year", - dropna=observed, + observed=observed, aggfunc="sum", ) expected_columns = pd.Int64Index([2013, 2014], name="Year") expected_index = pd.CategoricalIndex( - ["January"], categories=months, ordered=False, name="Month" + months, categories=months, ordered=False, name="Month" ) + expected_data = [[320, 120]] + [[0, 0]] * 11 expected = pd.DataFrame( - [[320, 120]], index=expected_index, columns=expected_columns + expected_data, index=expected_index, columns=expected_columns ) - if not observed: - result = result.dropna().astype(np.int64) + if observed: + expected = expected.loc[["January"]] tm.assert_frame_equal(result, expected) From 88d8a148fef8efccaeef631cc57f426fd869a4d1 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Sat, 11 Jul 2020 23:07:07 +0100 Subject: [PATCH 05/28] whatsnew --- doc/source/whatsnew/v1.1.0.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index 5f93e08d51baa..70dcc580a2006 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -1091,6 +1091,9 @@ Groupby/resample/rolling - Bug in :meth:`Rolling.apply` where ``center=True`` was ignored when ``engine='numba'`` was specified (:issue:`34784`) - Bug in :meth:`DataFrame.ewm.cov` was throwing ``AssertionError`` for :class:`MultiIndex` inputs (:issue:`34440`) - Bug in :meth:`core.groupby.DataFrameGroupBy.transform` when ``func='nunique'`` and columns are of type ``datetime64``, the result would also be of type ``datetime64`` instead of ``int64`` (:issue:`35109`) +- Bug in :meth:`DataFrameGroupBy.count` was returning ``NaN`` for missing categories when grouped on multiple ``Categoricals``. Now returning ``0`` (:issue:`35028`) +- Bug in :meth:`DataFrameGroupBy.sum` and :meth:`SeriesGroupBy.sum` was reutrning ``NaN`` for missing categories when grouped on multiple ``Categorials``. Now returning ``0`` (:issue:`31422`) + Reshaping ^^^^^^^^^ @@ -1124,6 +1127,8 @@ Reshaping - Bug in :meth:`Series.where` with an empty Series and empty ``cond`` having non-bool dtype (:issue:`34592`) - Fixed regression where :meth:`DataFrame.apply` would raise ``ValueError`` for elements whth ``S`` dtype (:issue:`34529`) - Bug in :meth:`DataFrame.append` leading to sorting columns even when ``sort=False`` is specified (:issue:`35092`) +- Bug in :meth:`DataFrame.pivot_table` with ``aggfunc='count'``, was returning ``NaN`` for missing categories when pivoted on a ``Categorical``. Now returning ``0`` (:issue:`35028`) +- Bug in :meth:`DataFrame.pivot_table` with ``aggfunc='sum'``, was reutrning ``NaN`` for missing categories when pivoted on a ``Categorical``. Now returning ``0`` (:issue:`31422`) Sparse ^^^^^^ From 940b32ed13306dc29c465d8b6a0915cfd5e5d561 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Sat, 11 Jul 2020 23:29:37 +0100 Subject: [PATCH 06/28] comments --- pandas/core/groupby/generic.py | 11 ++++------- pandas/core/groupby/groupby.py | 11 ++++------- pandas/tests/groupby/test_categorical.py | 8 ++++++++ 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index 2b656e3035d0e..119bb3acf51c9 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1820,13 +1820,10 @@ def count(self): ) blocks = [make_block(val, placement=loc) for val, loc in zip(counted, locs)] - # If self.observed=False then self._reindex_output will fill the missing - # categories (if the grouper was grouped on pd.Categorical variables) with - # np.NaN. For .count() we want these values filled in with zero. So we set - # self.observed=True for the call to self._agg_general, and then set - # it back to its orignal value. We then call self._reindex_output with - # fill_value=0. If the original self.observed is False, then we will get - # our result with 0 for the missing categories. + # GH 35028: We want .count() to return 0 for missing categories + # rather than NaN. So we set self.observed=True to turn off the + # reindexing within self._wrap_agged_blocks, then reindex below with + # fill_value=0 observed_orig = self.observed self.observed = True try: diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index c7600327f46ef..801123ea4bc53 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1532,13 +1532,10 @@ def size(self) -> FrameOrSeriesUnion: @doc(_groupby_agg_method_template, fname="sum", no=True, mc=0) def sum(self, numeric_only: bool = True, min_count: int = 0): - # If self.observed=False then self._reindex_output will fill the missing - # categories (if the grouper was grouped on pd.Categorical variables) with - # np.NaN. For .sum() we want these values filled in with zero. So we set - # self.observed=True for the call to self._agg_general, and then set - # it back to its orignal value. We then call self._reindex_output with - # fill_value=0. If the original self.observed is False, then we will get - # our result with 0 for the missing categories. + # GH 31422: We want .sum() to return 0 for missing categories + # rather than NaN. So we set self.observed=True to turn off the + # reindexing within self._agg_general, then reindex below with + # fill_value=0 observed_orig = self.observed self.observed = True try: diff --git a/pandas/tests/groupby/test_categorical.py b/pandas/tests/groupby/test_categorical.py index 4c83ecbc53c3d..595c6df8241ba 100644 --- a/pandas/tests/groupby/test_categorical.py +++ b/pandas/tests/groupby/test_categorical.py @@ -1435,6 +1435,14 @@ def test_dataframe_groupby_on_2_categoricals_when_observed_is_false( @pytest.mark.parametrize("func", ["sum", "count"]) def test_sum_and_count_exception_handling(func: str, observed: bool, monkeypatch): + # GH 31422 + # GH 35028 + # In order to return 0 instead of NaN for missing categories in + # GroupBy.count() and GroupBy.sum(), both methods overwrite the value of + # self.observed and then use a try-except-finally block. This test ensures + # that: + # a) An exception from a internal method is still raised + # b) self.observed is set back to its original value df = pd.DataFrame( { "cat_1": pd.Categorical(list("AABB"), categories=list("ABC")), From 3e39f1aa3ff9f89a2ce9c5c0fc1171ab4a396542 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Sat, 11 Jul 2020 23:31:10 +0100 Subject: [PATCH 07/28] black --- pandas/core/groupby/generic.py | 6 +++--- pandas/core/groupby/groupby.py | 6 +++--- pandas/tests/groupby/test_categorical.py | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index 119bb3acf51c9..bdfc29ac563dc 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1820,9 +1820,9 @@ def count(self): ) blocks = [make_block(val, placement=loc) for val, loc in zip(counted, locs)] - # GH 35028: We want .count() to return 0 for missing categories - # rather than NaN. So we set self.observed=True to turn off the - # reindexing within self._wrap_agged_blocks, then reindex below with + # GH 35028: We want .count() to return 0 for missing categories + # rather than NaN. So we set self.observed=True to turn off the + # reindexing within self._wrap_agged_blocks, then reindex below with # fill_value=0 observed_orig = self.observed self.observed = True diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 801123ea4bc53..992cbaa6af860 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1532,9 +1532,9 @@ def size(self) -> FrameOrSeriesUnion: @doc(_groupby_agg_method_template, fname="sum", no=True, mc=0) def sum(self, numeric_only: bool = True, min_count: int = 0): - # GH 31422: We want .sum() to return 0 for missing categories - # rather than NaN. So we set self.observed=True to turn off the - # reindexing within self._agg_general, then reindex below with + # GH 31422: We want .sum() to return 0 for missing categories + # rather than NaN. So we set self.observed=True to turn off the + # reindexing within self._agg_general, then reindex below with # fill_value=0 observed_orig = self.observed self.observed = True diff --git a/pandas/tests/groupby/test_categorical.py b/pandas/tests/groupby/test_categorical.py index 595c6df8241ba..d5897b8ba00cb 100644 --- a/pandas/tests/groupby/test_categorical.py +++ b/pandas/tests/groupby/test_categorical.py @@ -1437,11 +1437,11 @@ def test_dataframe_groupby_on_2_categoricals_when_observed_is_false( def test_sum_and_count_exception_handling(func: str, observed: bool, monkeypatch): # GH 31422 # GH 35028 - # In order to return 0 instead of NaN for missing categories in - # GroupBy.count() and GroupBy.sum(), both methods overwrite the value of - # self.observed and then use a try-except-finally block. This test ensures + # In order to return 0 instead of NaN for missing categories in + # GroupBy.count() and GroupBy.sum(), both methods overwrite the value of + # self.observed and then use a try-except-finally block. This test ensures # that: - # a) An exception from a internal method is still raised + # a) An exception from a internal method is still raised # b) self.observed is set back to its original value df = pd.DataFrame( { From bd4c4fe345541d5783f796ea906b90459ad0dc09 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Tue, 14 Jul 2020 21:36:28 +0100 Subject: [PATCH 08/28] .sum() and .count() create inner groupby with observed=True, and then reindex with 0 --- pandas/core/groupby/generic.py | 35 +++++++++++++--------- pandas/core/groupby/groupby.py | 38 +++++++++++++----------- pandas/tests/groupby/test_categorical.py | 31 ------------------- 3 files changed, 42 insertions(+), 62 deletions(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index bdfc29ac563dc..d4def12e7db07 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1807,6 +1807,26 @@ def count(self): DataFrame Count of values within each group. """ + if not self.observed: + groupby_tmp = get_groupby( + obj=self.obj, + by=self.keys, + axis=self.axis, + level=self.level, + grouper=self.grouper, + exclusions=self.exclusions, + selection=self._selection, + as_index=self.as_index, + sort=self.sort, + group_keys=self.group_keys, + squeeze=self.squeeze, + observed=True, + mutated=self.mutated, + dropna=self.dropna, + ) + result = groupby_tmp.count() + return self._reindex_output(result, fill_value=0) + data = self._get_data_to_aggregate() ids, _, ngroups = self.grouper.group_info mask = ids != -1 @@ -1820,20 +1840,7 @@ def count(self): ) blocks = [make_block(val, placement=loc) for val, loc in zip(counted, locs)] - # GH 35028: We want .count() to return 0 for missing categories - # rather than NaN. So we set self.observed=True to turn off the - # reindexing within self._wrap_agged_blocks, then reindex below with - # fill_value=0 - observed_orig = self.observed - self.observed = True - try: - result = self._wrap_agged_blocks(blocks, items=data.items) - except Exception as e: - raise e - finally: - self.observed = observed_orig - - return self._reindex_output(result, fill_value=0) + return self._wrap_agged_blocks(blocks, items=data.items) def nunique(self, dropna: bool = True): """ diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 992cbaa6af860..4369dc1f25a33 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1532,25 +1532,29 @@ def size(self) -> FrameOrSeriesUnion: @doc(_groupby_agg_method_template, fname="sum", no=True, mc=0) def sum(self, numeric_only: bool = True, min_count: int = 0): - # GH 31422: We want .sum() to return 0 for missing categories - # rather than NaN. So we set self.observed=True to turn off the - # reindexing within self._agg_general, then reindex below with - # fill_value=0 - observed_orig = self.observed - self.observed = True - try: - result = self._agg_general( - numeric_only=numeric_only, - min_count=min_count, - alias="add", - npfunc=np.sum, + if not self.observed: + inner_groupby = get_groupby( + obj=self.obj, + by=self.keys, + axis=self.axis, + level=self.level, + grouper=self.grouper, + exclusions=self.exclusions, + selection=self._selection, + as_index=self.as_index, + sort=self.sort, + group_keys=self.group_keys, + squeeze=self.squeeze, + observed=True, + mutated=self.mutated, + dropna=self.dropna, ) - except Exception as e: - raise e - finally: - self.observed = observed_orig + result = inner_groupby.sum(numeric_only, min_count) + return self._reindex_output(result, fill_value=0) - return self._reindex_output(result, fill_value=0) + return self._agg_general( + numeric_only=numeric_only, min_count=min_count, alias="add", npfunc=np.sum + ) @doc(_groupby_agg_method_template, fname="prod", no=True, mc=0) def prod(self, numeric_only: bool = True, min_count: int = 0): diff --git a/pandas/tests/groupby/test_categorical.py b/pandas/tests/groupby/test_categorical.py index d5897b8ba00cb..eb76df3e0f306 100644 --- a/pandas/tests/groupby/test_categorical.py +++ b/pandas/tests/groupby/test_categorical.py @@ -1433,37 +1433,6 @@ def test_dataframe_groupby_on_2_categoricals_when_observed_is_false( assert (res.loc[unobserved_cats] == expected).all().all() -@pytest.mark.parametrize("func", ["sum", "count"]) -def test_sum_and_count_exception_handling(func: str, observed: bool, monkeypatch): - # GH 31422 - # GH 35028 - # In order to return 0 instead of NaN for missing categories in - # GroupBy.count() and GroupBy.sum(), both methods overwrite the value of - # self.observed and then use a try-except-finally block. This test ensures - # that: - # a) An exception from a internal method is still raised - # b) self.observed is set back to its original value - df = pd.DataFrame( - { - "cat_1": pd.Categorical(list("AABB"), categories=list("ABC")), - "cat_2": pd.Categorical(list("1111"), categories=list("12")), - "value": [0.1, 0.1, 0.1, 0.1], - } - ) - df_grp = df.groupby(["cat_1", "cat_2"], observed=observed) - - def _mock_method(*args, **kwargs): - raise ZeroDivisionError - - to_patch = {"count": "_wrap_agged_blocks", "sum": "_agg_general"} - monkeypatch.setattr(df_grp, to_patch[func], _mock_method) - - with pytest.raises(ZeroDivisionError): - getattr(df_grp, func)() - - assert df_grp.observed is observed - - def test_series_groupby_categorical_aggregation_getitem(): # GH 8870 d = {"foo": [10, 8, 4, 1], "bar": [10, 20, 30, 40], "baz": ["d", "c", "d", "c"]} From fcbb58b2e442bc420bf5da472f3604b0a1ebd366 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Wed, 15 Jul 2020 02:10:04 +0100 Subject: [PATCH 09/28] modify self.observed with a context manager --- pandas/core/groupby/generic.py | 25 +++++----------------- pandas/core/groupby/groupby.py | 39 ++++++++++++++++------------------ 2 files changed, 23 insertions(+), 41 deletions(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index d4def12e7db07..f0c3756f8c3e9 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -72,6 +72,7 @@ _apply_docs, _transform_template, get_groupby, + _observed_is_true, ) from pandas.core.indexes.api import Index, MultiIndex, all_indexes_same import pandas.core.indexes.base as ibase @@ -1807,25 +1808,6 @@ def count(self): DataFrame Count of values within each group. """ - if not self.observed: - groupby_tmp = get_groupby( - obj=self.obj, - by=self.keys, - axis=self.axis, - level=self.level, - grouper=self.grouper, - exclusions=self.exclusions, - selection=self._selection, - as_index=self.as_index, - sort=self.sort, - group_keys=self.group_keys, - squeeze=self.squeeze, - observed=True, - mutated=self.mutated, - dropna=self.dropna, - ) - result = groupby_tmp.count() - return self._reindex_output(result, fill_value=0) data = self._get_data_to_aggregate() ids, _, ngroups = self.grouper.group_info @@ -1840,7 +1822,10 @@ def count(self): ) blocks = [make_block(val, placement=loc) for val, loc in zip(counted, locs)] - return self._wrap_agged_blocks(blocks, items=data.items) + with _observed_is_true(self): + result = self._wrap_agged_blocks(blocks, items=data.items) + + return self._reindex_output(result, fill_value=0) def nunique(self, dropna: bool = True): """ diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 4369dc1f25a33..30c67551f2c3b 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -464,6 +464,17 @@ def _group_selection_context(groupby): groupby._reset_group_selection() +@contextmanager +def _observed_is_true(groupby): + """ + Set GroupBy.observed to True, then restore to original value + """ + original_value = groupby.observed + groupby.observed = True + yield groupby + groupby.observed = original_value + + _KeysArgType = Union[ Hashable, List[Hashable], @@ -1532,29 +1543,15 @@ def size(self) -> FrameOrSeriesUnion: @doc(_groupby_agg_method_template, fname="sum", no=True, mc=0) def sum(self, numeric_only: bool = True, min_count: int = 0): - if not self.observed: - inner_groupby = get_groupby( - obj=self.obj, - by=self.keys, - axis=self.axis, - level=self.level, - grouper=self.grouper, - exclusions=self.exclusions, - selection=self._selection, - as_index=self.as_index, - sort=self.sort, - group_keys=self.group_keys, - squeeze=self.squeeze, - observed=True, - mutated=self.mutated, - dropna=self.dropna, + with _observed_is_true(self): + result = self._agg_general( + numeric_only=numeric_only, + min_count=min_count, + alias="add", + npfunc=np.sum, ) - result = inner_groupby.sum(numeric_only, min_count) - return self._reindex_output(result, fill_value=0) - return self._agg_general( - numeric_only=numeric_only, min_count=min_count, alias="add", npfunc=np.sum - ) + return self._reindex_output(result, fill_value=0) @doc(_groupby_agg_method_template, fname="prod", no=True, mc=0) def prod(self, numeric_only: bool = True, min_count: int = 0): From 5f89c173592f65c4f0283eefc049fc73b48df116 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Wed, 15 Jul 2020 02:22:17 +0100 Subject: [PATCH 10/28] added comments --- pandas/core/groupby/generic.py | 6 +++++- pandas/core/groupby/groupby.py | 5 +++++ pandas/tests/groupby/test_categorical.py | 1 - 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index f0c3756f8c3e9..891f749780c4b 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1808,7 +1808,6 @@ def count(self): DataFrame Count of values within each group. """ - data = self._get_data_to_aggregate() ids, _, ngroups = self.grouper.group_info mask = ids != -1 @@ -1822,6 +1821,11 @@ def count(self): ) blocks = [make_block(val, placement=loc) for val, loc in zip(counted, locs)] + # If we are grouping on categoricals we want unobserved categories to + # return zero, rather than the default of NaN which the reindexing in + # _wrap_agged_blocks() returns. We set self.observed=True for the call to + # _wrap_agged_blocks() to avoid it reindexing. We then reindex below with + # fill_value=0. See GH 35028 with _observed_is_true(self): result = self._wrap_agged_blocks(blocks, items=data.items) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 30c67551f2c3b..939095fdf2ad4 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1543,6 +1543,11 @@ def size(self) -> FrameOrSeriesUnion: @doc(_groupby_agg_method_template, fname="sum", no=True, mc=0) def sum(self, numeric_only: bool = True, min_count: int = 0): + # If we are grouping on categoricals we want unobserved categories to + # return zero, rather than the default of NaN which the reindexing in + # _agg_general() returns. We set self.observed=True for the call to + # _agg_general() to avoid it reindexing. We then reindex below with + # fill_value=0. See GH 31422 with _observed_is_true(self): result = self._agg_general( numeric_only=numeric_only, diff --git a/pandas/tests/groupby/test_categorical.py b/pandas/tests/groupby/test_categorical.py index eb76df3e0f306..492276bc0057b 100644 --- a/pandas/tests/groupby/test_categorical.py +++ b/pandas/tests/groupby/test_categorical.py @@ -1195,7 +1195,6 @@ def test_seriesgroupby_observed_false_or_none(df_cat, observed, operation): expected = expected.fillna(0, downcast="infer") grouped = df_cat.groupby(["A", "B"], observed=observed)["C"] result = getattr(grouped, operation)(sum) - tm.assert_series_equal(result, expected) From 9da5b18461edf3d1f1d2aace8abc32b5b4998926 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Wed, 15 Jul 2020 09:28:45 +0100 Subject: [PATCH 11/28] Fixed import order that caused Linting error --- pandas/core/groupby/generic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index a894cc8bc8420..4c260dc5c12e1 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -70,9 +70,9 @@ GroupBy, _agg_template, _apply_docs, + _observed_is_true, _transform_template, get_groupby, - _observed_is_true, ) from pandas.core.indexes.api import Index, MultiIndex, all_indexes_same import pandas.core.indexes.base as ibase From 534d60fcd628438588df6d9173f6db93b0883395 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Thu, 16 Jul 2020 09:27:03 +0100 Subject: [PATCH 12/28] addressing comments --- doc/source/whatsnew/v1.1.0.rst | 6 ++---- pandas/core/groupby/generic.py | 3 +-- pandas/core/groupby/groupby.py | 13 +------------ 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index 69fc0b1431d8c..4a67ece629e1d 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -1081,8 +1081,7 @@ Groupby/resample/rolling - Bug in :meth:`DataFrame.ewm.cov` was throwing ``AssertionError`` for :class:`MultiIndex` inputs (:issue:`34440`) - Bug in :meth:`core.groupby.DataFrameGroupBy.transform` when ``func='nunique'`` and columns are of type ``datetime64``, the result would also be of type ``datetime64`` instead of ``int64`` (:issue:`35109`) - Bug in :meth:'DataFrameGroupBy.first' and :meth:'DataFrameGroupBy.last' that would raise an unnecessary ``ValueError`` when grouping on multiple ``Categoricals`` (:issue:`34951`) -- Bug in :meth:`DataFrameGroupBy.count` was returning ``NaN`` for missing categories when grouped on multiple ``Categoricals``. Now returning ``0`` (:issue:`35028`) -- Bug in :meth:`DataFrameGroupBy.sum` and :meth:`SeriesGroupBy.sum` was reutrning ``NaN`` for missing categories when grouped on multiple ``Categorials``. Now returning ``0`` (:issue:`31422`) +- Bug in :meth:`DataFrameGroupBy.count` and :meth:`GroupBy.sum` returning ``NaN`` for missing categories when grouped on multiple ``Categoricals``. Now returning ``0`` (:issue:`35028`) Reshaping @@ -1118,8 +1117,7 @@ Reshaping - Bug in :meth:`Series.where` with an empty Series and empty ``cond`` having non-bool dtype (:issue:`34592`) - Fixed regression where :meth:`DataFrame.apply` would raise ``ValueError`` for elements whth ``S`` dtype (:issue:`34529`) - Bug in :meth:`DataFrame.append` leading to sorting columns even when ``sort=False`` is specified (:issue:`35092`) -- Bug in :meth:`DataFrame.pivot_table` with ``aggfunc='count'``, was returning ``NaN`` for missing categories when pivoted on a ``Categorical``. Now returning ``0`` (:issue:`35028`) -- Bug in :meth:`DataFrame.pivot_table` with ``aggfunc='sum'``, was reutrning ``NaN`` for missing categories when pivoted on a ``Categorical``. Now returning ``0`` (:issue:`31422`) +- Bug in :meth:`DataFrame.pivot_table` with ``aggfunc='count'`` or ``aggfunc='sum'`` returning ``NaN`` for missing categories when pivoted on a ``Categorical``. Now returning ``0`` (:issue:`31422`) Sparse ^^^^^^ diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index 4c260dc5c12e1..e89af45a2c807 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -70,7 +70,6 @@ GroupBy, _agg_template, _apply_docs, - _observed_is_true, _transform_template, get_groupby, ) @@ -1833,7 +1832,7 @@ def count(self): # _wrap_agged_blocks() returns. We set self.observed=True for the call to # _wrap_agged_blocks() to avoid it reindexing. We then reindex below with # fill_value=0. See GH 35028 - with _observed_is_true(self): + with com.temp_setattr(self, "observed", True): result = self._wrap_agged_blocks(blocks, items=data.items) return self._reindex_output(result, fill_value=0) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 939095fdf2ad4..e704dc7e15637 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -464,17 +464,6 @@ def _group_selection_context(groupby): groupby._reset_group_selection() -@contextmanager -def _observed_is_true(groupby): - """ - Set GroupBy.observed to True, then restore to original value - """ - original_value = groupby.observed - groupby.observed = True - yield groupby - groupby.observed = original_value - - _KeysArgType = Union[ Hashable, List[Hashable], @@ -1548,7 +1537,7 @@ def sum(self, numeric_only: bool = True, min_count: int = 0): # _agg_general() returns. We set self.observed=True for the call to # _agg_general() to avoid it reindexing. We then reindex below with # fill_value=0. See GH 31422 - with _observed_is_true(self): + with com.temp_setattr(self, "observed", True): result = self._agg_general( numeric_only=numeric_only, min_count=min_count, From d2abd4db3bbef5a3c39bf48dd5a9cc0d678742b4 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Thu, 16 Jul 2020 10:12:55 +0100 Subject: [PATCH 13/28] fixing whatsnew comment --- 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 76e0e46f920ff..7271759025703 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -1122,8 +1122,8 @@ Groupby/resample/rolling - Bug in :meth:`Rolling.apply` where ``center=True`` was ignored when ``engine='numba'`` was specified (:issue:`34784`) - Bug in :meth:`DataFrame.ewm.cov` was throwing ``AssertionError`` for :class:`MultiIndex` inputs (:issue:`34440`) - Bug in :meth:`core.groupby.DataFrameGroupBy.transform` when ``func='nunique'`` and columns are of type ``datetime64``, the result would also be of type ``datetime64`` instead of ``int64`` (:issue:`35109`) -- Bug in :meth:'DataFrameGroupBy.first' and :meth:'DataFrameGroupBy.last' that would raise an unnecessary ``ValueError`` when grouping on multiple ``Categoricals`` (:issue:`34951`) -- Bug in :meth:`DataFrameGroupBy.count` and :meth:`GroupBy.sum` returning ``NaN`` for missing categories when grouped on multiple ``Categoricals``. Now returning ``0`` (:issue:`35028`) +- Bug in :meth:`DataFrameGroupBy.first` and :meth:`DataFrameGroupBy.last` that would raise an unnecessary ``ValueError`` when grouping on multiple ``Categoricals`` (:issue:`34951`) +- Bug in :meth:`DataFrameGroupBy.count` and :meth:`SeriesGroupBy.sum` returning ``NaN`` for missing categories when grouped on multiple ``Categoricals``. Now returning ``0`` (:issue:`35028`) Reshaping From 361098aad6ec172703b76caf9fedc57ec54045ff Mon Sep 17 00:00:00 2001 From: smithto1 Date: Thu, 16 Jul 2020 12:47:43 +0100 Subject: [PATCH 14/28] ammend comment --- 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 9684332430fd9..757fc7255bf92 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1539,7 +1539,7 @@ def sum(self, numeric_only: bool = True, min_count: int = 0): # return zero, rather than the default of NaN which the reindexing in # _agg_general() returns. We set self.observed=True for the call to # _agg_general() to avoid it reindexing. We then reindex below with - # fill_value=0. See GH 31422 + # fill_value=0. See GH #31422 with com.temp_setattr(self, "observed", True): result = self._agg_general( numeric_only=numeric_only, From 9879199a25845ea415ac4d7ffd6441a8bb9e330b Mon Sep 17 00:00:00 2001 From: smithto1 Date: Thu, 16 Jul 2020 13:38:31 +0100 Subject: [PATCH 15/28] ammend comment --- pandas/core/groupby/generic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index 8092c9769abbe..edfdc8c5f9550 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1828,7 +1828,7 @@ def count(self): # return zero, rather than the default of NaN which the reindexing in # _wrap_agged_blocks() returns. We set self.observed=True for the call to # _wrap_agged_blocks() to avoid it reindexing. We then reindex below with - # fill_value=0. See GH 35028 + # fill_value=0. See GH #35028 with com.temp_setattr(self, "observed", True): result = self._wrap_agged_blocks(blocks, items=data.items) From 39ee6d0416c42d655cd9f860de71bb321613793f Mon Sep 17 00:00:00 2001 From: smithto1 Date: Thu, 16 Jul 2020 16:38:36 +0100 Subject: [PATCH 16/28] addressing PR comments --- pandas/core/groupby/generic.py | 4 +--- pandas/core/groupby/groupby.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index edfdc8c5f9550..ec0ecf8c5c79f 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1826,9 +1826,7 @@ def count(self): # If we are grouping on categoricals we want unobserved categories to # return zero, rather than the default of NaN which the reindexing in - # _wrap_agged_blocks() returns. We set self.observed=True for the call to - # _wrap_agged_blocks() to avoid it reindexing. We then reindex below with - # fill_value=0. See GH #35028 + # _wrap_agged_blocks() returns. GH 35028 with com.temp_setattr(self, "observed", True): result = self._wrap_agged_blocks(blocks, items=data.items) diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py index 757fc7255bf92..d314c84aeecda 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1537,9 +1537,7 @@ def sum(self, numeric_only: bool = True, min_count: int = 0): # If we are grouping on categoricals we want unobserved categories to # return zero, rather than the default of NaN which the reindexing in - # _agg_general() returns. We set self.observed=True for the call to - # _agg_general() to avoid it reindexing. We then reindex below with - # fill_value=0. See GH #31422 + # _agg_general() returns. GH 31422 with com.temp_setattr(self, "observed", True): result = self._agg_general( numeric_only=numeric_only, From fc1a846dd3f3dca67d21d2ef65746f8603fefbe9 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Fri, 17 Jul 2020 00:32:39 +0100 Subject: [PATCH 17/28] ammend comment --- pandas/core/groupby/generic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index ec0ecf8c5c79f..710c5e898fe04 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1826,7 +1826,7 @@ def count(self): # If we are grouping on categoricals we want unobserved categories to # return zero, rather than the default of NaN which the reindexing in - # _wrap_agged_blocks() returns. GH 35028 + # _wrap_agged_blocks() returns. GH #35028 with com.temp_setattr(self, "observed", True): result = self._wrap_agged_blocks(blocks, items=data.items) From f774288c3907ffb6baccc7490fd3fb798ee00fd6 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Fri, 17 Jul 2020 11:11:10 +0100 Subject: [PATCH 18/28] amend comment --- 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 d314c84aeecda..76bdf2c9cfd39 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1537,7 +1537,7 @@ def sum(self, numeric_only: bool = True, min_count: int = 0): # If we are grouping on categoricals we want unobserved categories to # return zero, rather than the default of NaN which the reindexing in - # _agg_general() returns. GH 31422 + # _agg_general() returns. GH #31422 with com.temp_setattr(self, "observed", True): result = self._agg_general( numeric_only=numeric_only, From 1986f519172d456ef6cc49ead49e828b7122ad1e Mon Sep 17 00:00:00 2001 From: smithto1 Date: Fri, 17 Jul 2020 12:29:26 +0100 Subject: [PATCH 19/28] amend comment (just to rerun tests --- 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 76bdf2c9cfd39..d314c84aeecda 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1537,7 +1537,7 @@ def sum(self, numeric_only: bool = True, min_count: int = 0): # If we are grouping on categoricals we want unobserved categories to # return zero, rather than the default of NaN which the reindexing in - # _agg_general() returns. GH #31422 + # _agg_general() returns. GH 31422 with com.temp_setattr(self, "observed", True): result = self._agg_general( numeric_only=numeric_only, From c4c1a926adc7d72a4dca7dc29d87aab0ee713939 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Fri, 17 Jul 2020 15:50:55 +0100 Subject: [PATCH 20/28] ammend comment to start new tests (tests failing due to some flakey tests that fail occassionally) --- 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 d314c84aeecda..76bdf2c9cfd39 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1537,7 +1537,7 @@ def sum(self, numeric_only: bool = True, min_count: int = 0): # If we are grouping on categoricals we want unobserved categories to # return zero, rather than the default of NaN which the reindexing in - # _agg_general() returns. GH 31422 + # _agg_general() returns. GH #31422 with com.temp_setattr(self, "observed", True): result = self._agg_general( numeric_only=numeric_only, From 5f8ba0e25db4c630e617f8e957df2a9877357059 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Sat, 18 Jul 2020 15:11:15 +0100 Subject: [PATCH 21/28] amend comment to kick off tests --- 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 c98d1b632afde..5c618f17f49bf 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1537,7 +1537,7 @@ def sum(self, numeric_only: bool = True, min_count: int = 0): # If we are grouping on categoricals we want unobserved categories to # return zero, rather than the default of NaN which the reindexing in - # _agg_general() returns. GH #31422 + # _agg_general() returns. GH 31422 with com.temp_setattr(self, "observed", True): result = self._agg_general( numeric_only=numeric_only, From 49b5fc8963d034055ad48c9a982ff62a26264990 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Sat, 18 Jul 2020 20:22:57 +0100 Subject: [PATCH 22/28] ammend test to kick off tests --- 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 5c618f17f49bf..c98d1b632afde 100644 --- a/pandas/core/groupby/groupby.py +++ b/pandas/core/groupby/groupby.py @@ -1537,7 +1537,7 @@ def sum(self, numeric_only: bool = True, min_count: int = 0): # If we are grouping on categoricals we want unobserved categories to # return zero, rather than the default of NaN which the reindexing in - # _agg_general() returns. GH 31422 + # _agg_general() returns. GH #31422 with com.temp_setattr(self, "observed", True): result = self._agg_general( numeric_only=numeric_only, From 28794e3377d41faa90290a2492285701a31382d2 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Mon, 27 Jul 2020 00:00:01 +0100 Subject: [PATCH 23/28] fixed merge --- doc/source/whatsnew/v1.1.0.rst | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index b5736871c65fc..9c62d526a6cb7 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -1164,19 +1164,11 @@ Reshaping - Bug in :func:`concat` was not allowing for concatenation of :class:`DataFrame` and :class:`Series` with duplicate keys (:issue:`33654`) - Bug in :func:`cut` raised an error when the argument ``labels`` contains duplicates (:issue:`33141`) - Ensure only named functions can be used in :func:`eval()` (:issue:`32460`) -<<<<<<< HEAD -- Bug in :func:`Dataframe.aggregate` and :func:`Series.aggregate` was causing recursive loop in some cases (:issue:`34224`) -- Fixed bug in :func:`melt` where melting MultiIndex columns with ``col_level`` > 0 would raise a ``KeyError`` on ``id_vars`` (:issue:`34129`) -- Bug in :meth:`Series.where` with an empty Series and empty ``cond`` having non-bool dtype (:issue:`34592`) -- Fixed regression where :meth:`DataFrame.apply` would raise ``ValueError`` for elements whth ``S`` dtype (:issue:`34529`) -- Bug in :meth:`DataFrame.pivot_table` with ``aggfunc='count'`` or ``aggfunc='sum'`` returning ``NaN`` for missing categories when pivoted on a ``Categorical``. Now returning ``0`` (:issue:`31422`) - -======= - Bug in :meth:`Dataframe.aggregate` and :meth:`Series.aggregate` was causing a recursive loop in some cases (:issue:`34224`) - Fixed bug in :func:`melt` where melting :class:`MultiIndex` columns with ``col_level > 0`` would raise a ``KeyError`` on ``id_vars`` (:issue:`34129`) - Bug in :meth:`Series.where` with an empty :class:`Series` and empty ``cond`` having non-bool dtype (:issue:`34592`) - Fixed regression where :meth:`DataFrame.apply` would raise ``ValueError`` for elements with ``S`` dtype (:issue:`34529`) ->>>>>>> upstream/master +- Bug in :meth:`DataFrame.pivot_table` with ``aggfunc='count'`` or ``aggfunc='sum'`` returning ``NaN`` for missing categories when pivoted on a ``Categorical``. Now returning ``0`` (:issue:`31422`) Sparse ^^^^^^ From f616d5a3a56a253f1bddbb904ed790bd42c08853 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Mon, 27 Jul 2020 00:01:20 +0100 Subject: [PATCH 24/28] merge fix --- doc/source/whatsnew/v1.1.0.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index 9c62d526a6cb7..06ac524111d5e 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -1130,7 +1130,6 @@ Groupby/resample/rolling - Bug in :meth:`DataFrame.ewm.cov` was throwing ``AssertionError`` for :class:`MultiIndex` inputs (:issue:`34440`) - Bug in :meth:`core.groupby.DataFrameGroupBy.quantile` raised ``TypeError`` for non-numeric types rather than dropping the columns (:issue:`27892`) - Bug in :meth:`core.groupby.DataFrameGroupBy.transform` when ``func='nunique'`` and columns are of type ``datetime64``, the result would also be of type ``datetime64`` instead of ``int64`` (:issue:`35109`) -- Bug in :meth:`DataFrameGroupBy.first` and :meth:`DataFrameGroupBy.last` that would raise an unnecessary ``ValueError`` when grouping on multiple ``Categoricals`` (:issue:`34951`) - Bug in :meth:`DataFrame.groupby` raising an ``AttributeError`` when selecting a column and aggregating with ``as_index=False`` (:issue:`35246`). - Bug in :meth:`DataFrameGroupBy.first` and :meth:`DataFrameGroupBy.last` that would raise an unnecessary ``ValueError`` when grouping on multiple ``Categoricals`` (:issue:`34951`) - Bug in :meth:`DataFrameGroupBy.count` and :meth:`SeriesGroupBy.sum` returning ``NaN`` for missing categories when grouped on multiple ``Categoricals``. Now returning ``0`` (:issue:`35028`) From 80d4b7dd971a990114c87f46dc7e2cc510251345 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Mon, 27 Jul 2020 10:56:25 +0100 Subject: [PATCH 25/28] udpate comment to kick off tests --- pandas/core/groupby/generic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index affb1e7a05da6..6c8f6de54b49c 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1831,7 +1831,7 @@ def count(self): # If we are grouping on categoricals we want unobserved categories to # return zero, rather than the default of NaN which the reindexing in - # _wrap_agged_blocks() returns. GH #35028 + # _wrap_agged_blocks() returns. GH 35028 with com.temp_setattr(self, "observed", True): result = self._wrap_agged_blocks(blocks, items=data.items) From 442ad1d0f163a8ec46d5c8173a4690e5ef936829 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Sat, 1 Aug 2020 23:51:29 +0100 Subject: [PATCH 26/28] amend comment to restart checks --- pandas/core/groupby/generic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index 6c8f6de54b49c..affb1e7a05da6 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1831,7 +1831,7 @@ def count(self): # If we are grouping on categoricals we want unobserved categories to # return zero, rather than the default of NaN which the reindexing in - # _wrap_agged_blocks() returns. GH 35028 + # _wrap_agged_blocks() returns. GH #35028 with com.temp_setattr(self, "observed", True): result = self._wrap_agged_blocks(blocks, items=data.items) From ed99b604ce0742ffcd9b1c835ce296225e50c133 Mon Sep 17 00:00:00 2001 From: smithto1 Date: Wed, 5 Aug 2020 15:18:21 +0100 Subject: [PATCH 27/28] doc updates --- doc/source/whatsnew/v1.1.0.rst | 2 -- doc/source/whatsnew/v1.2.0.rst | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/doc/source/whatsnew/v1.1.0.rst b/doc/source/whatsnew/v1.1.0.rst index 013799a6344aa..a49b29d691692 100644 --- a/doc/source/whatsnew/v1.1.0.rst +++ b/doc/source/whatsnew/v1.1.0.rst @@ -1131,7 +1131,6 @@ Groupby/resample/rolling - Bug in :meth:`core.groupby.DataFrameGroupBy.transform` when ``func='nunique'`` and columns are of type ``datetime64``, the result would also be of type ``datetime64`` instead of ``int64`` (:issue:`35109`) - Bug in :meth:`DataFrame.groupby` raising an ``AttributeError`` when selecting a column and aggregating with ``as_index=False`` (:issue:`35246`). - Bug in :meth:`DataFrameGroupBy.first` and :meth:`DataFrameGroupBy.last` that would raise an unnecessary ``ValueError`` when grouping on multiple ``Categoricals`` (:issue:`34951`) -- Bug in :meth:`DataFrameGroupBy.count` and :meth:`SeriesGroupBy.sum` returning ``NaN`` for missing categories when grouped on multiple ``Categoricals``. Now returning ``0`` (:issue:`35028`) Reshaping ^^^^^^^^^ @@ -1166,7 +1165,6 @@ Reshaping - Fixed bug in :func:`melt` where melting :class:`MultiIndex` columns with ``col_level > 0`` would raise a ``KeyError`` on ``id_vars`` (:issue:`34129`) - Bug in :meth:`Series.where` with an empty :class:`Series` and empty ``cond`` having non-bool dtype (:issue:`34592`) - Fixed regression where :meth:`DataFrame.apply` would raise ``ValueError`` for elements with ``S`` dtype (:issue:`34529`) -- Bug in :meth:`DataFrame.pivot_table` with ``aggfunc='count'`` or ``aggfunc='sum'`` returning ``NaN`` for missing categories when pivoted on a ``Categorical``. Now returning ``0`` (:issue:`31422`) Sparse ^^^^^^ diff --git a/doc/source/whatsnew/v1.2.0.rst b/doc/source/whatsnew/v1.2.0.rst index b16ca0a80c5b4..7f6863efa9998 100644 --- a/doc/source/whatsnew/v1.2.0.rst +++ b/doc/source/whatsnew/v1.2.0.rst @@ -132,14 +132,14 @@ Plotting Groupby/resample/rolling ^^^^^^^^^^^^^^^^^^^^^^^^ -- +- Bug in :meth:`DataFrameGroupBy.count` and :meth:`SeriesGroupBy.sum` returning ``NaN`` for missing categories when grouped on multiple ``Categoricals``. Now returning ``0`` (:issue:`35028`) - Reshaping ^^^^^^^^^ -- +- Bug in :meth:`DataFrame.pivot_table` with ``aggfunc='count'`` or ``aggfunc='sum'`` returning ``NaN`` for missing categories when pivoted on a ``Categorical``. Now returning ``0`` (:issue:`31422`) - Sparse From 528ff7e239accde7dbb98b0c4f80f417b72ef51a Mon Sep 17 00:00:00 2001 From: smithto1 Date: Wed, 5 Aug 2020 18:42:39 +0100 Subject: [PATCH 28/28] restart tests --- pandas/core/groupby/generic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/groupby/generic.py b/pandas/core/groupby/generic.py index d32077808edd3..740463f0cf356 100644 --- a/pandas/core/groupby/generic.py +++ b/pandas/core/groupby/generic.py @@ -1831,7 +1831,7 @@ def count(self): # If we are grouping on categoricals we want unobserved categories to # return zero, rather than the default of NaN which the reindexing in - # _wrap_agged_blocks() returns. GH #35028 + # _wrap_agged_blocks() returns. GH 35028 with com.temp_setattr(self, "observed", True): result = self._wrap_agged_blocks(blocks, items=data.items)