Repository: pandas-dev/pandas
Pull request: BUG: regression when applying groupby aggregation on categorical columns
- [ ] closes #31256 
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Problem statement:
Regression bugs when applying GroupBy Aggregations to Categorical columns

```python
>>> import pandas as pd

>>> ids = range(5)
>>> groups = pd.Series([0, 1, 1, 2, 2])
>>> values = pd.Categorical([0, 0, 0, 0, 1])
>>> df = pd.DataFrame({
            'id': ids,
            'groups': groups,
            'value': values
    }).set_index('id')

>>> df.groupby('groups').agg(pd.Series.nunique)
       value
groups
0          1
1          1
2        NaN

>>> df.groupby('groups').agg(pd.Series.nunique)['value'].dtype
CategoricalDtype(categories=[0, 1], ordered=False)

>>> df.groupby('groups').agg(pd.Series.count)
       value
groups
0          1
1        NaN
2        NaN

>>> df.groupby('groups').agg(pd.Series.mode)
                                                                           value
groups
0                                                                              0
1              0    0
Name: value, dtype: category
Categories (2, int64): [0, 1]
2       0    0
1    1
Name: value, dtype: category
Categories (2, int64): [0, 1]

```
#### Problem description
There's some regression bugs in `1.0.0rc0` when applying groupby aggregations to Categorical columns. All of these examples work as expected in `0.25.3`. 

Applying `pd.Series.nunique` and `pd.Series.count` both result in `NaN` for some groupings, the result has a Categorical dtype. The resulting dtype should be int64 and `NaN` should not be produced.

Applying `pd.Series.mode` returns the correct result for the first grouping but produces nonsensical output for subsequent groupings. 


#### Expected Output
Aggregations should work as expected on Categorical columns as they do in `0.25.3`:
```python
>>> df.groupby('groups').agg(pd.Series.nunique)
        value
groups
0           1
1           1
2           2

>>> df.groupby('groups').agg(pd.Series.nunique)['value'].dtype
dtype('int64')

>>> df.groupby('groups').agg(pd.Series.count)
        value
groups
0           1
1           2
2           2

>>> df.groupby('groups').agg(pd.Series.mode)
         value
groups
0      0     0
1      0     0
2      0     0
       1     1
```

#### Output of ``pd.show_versions()``

<details>

INSTALLED VERSIONS
------------------
commit           : None
python           : 3.6.9.final.0
python-bits      : 64
OS               : Darwin
OS-release       : 18.7.0
machine          : x86_64
processor        : i386
byteorder        : little
LC_ALL           : None
LANG             : en_US.UTF-8
LOCALE           : en_US.UTF-8

pandas           : 1.0.0rc0
numpy            : 1.18.1
pytz             : 2019.3
dateutil         : 2.8.1
pip              : 20.0.1
setuptools       : 45.1.0
Cython           : None
pytest           : 5.2.0
hypothesis       : None
sphinx           : 2.0.1
blosc            : None
feather          : None
xlsxwriter       : None
lxml.etree       : None
html5lib         : None
pymysql          : None
psycopg2         : None
jinja2           : 2.10.3
IPython          : 7.2.0
pandas_datareader: None
bs4              : None
bottleneck       : None
fastparquet      : 0.3.2
gcsfs            : None
lxml.etree       : None
matplotlib       : 3.0.2
numexpr          : None
odfpy            : None
openpyxl         : None
pandas_gbq       : None
pyarrow          : None
pytables         : None
pytest           : 5.2.0
s3fs             : 0.4.0
scipy            : 1.4.1
sqlalchemy       : None
tables           : None
tabulate         : None
xarray           : None
xlrd             : None
xlwt             : None
xlsxwriter       : None
numba            : 0.47.0

</details>

Hints from issue discussion:
I don't believe this is fixed by https://github.com/pandas-dev/pandas/pull/31238.
no, it won't be fixed by 31238. i initially thought this was a duplicate, but after seeing the fix i re-assigned to myself. i already found the issue, will submit a patch later tonight to make sure this can be included before 1.0.0 releases.
Hi @TomAugspurger 

I have a question about the current behaviour, currently, on 0.25.3, 
```python
>>> ids = range(5)
>>> groups = pd.Series([0, 1, 1, 2, 2])
>>> values = pd.Categorical([0, 0, 0, 0, 1])
>>> df = pd.DataFrame({
            'id': ids,
            'groups': groups,
            'value': values
    }).set_index('id')
>>> df.groupby('groups').agg(pd.Series.mode)
         value
groups
0      0     0
1      0     0
2      0     0
       1     1
```
there is an extra index after aggregation, and I am not very sure if this is desired behaviour? could you pls give some thoughts on it? it seems that this extra index comes from output of `pd.Series.mode` which is a `Series`. should this be like this? or this extra index should be removed?
I'm not sure what the desired behavior is there.

On Mon, Jan 27, 2020 at 2:47 PM Kaiqi Dong <notifications@github.com> wrote:

> Hi @TomAugspurger <https://github.com/TomAugspurger>
>
> I have a question about the current behaviour, currently, on 0.25.3,
>
> >>> ids = range(5)>>> groups = pd.Series([0, 1, 1, 2, 2])>>> values = pd.Categorical([0, 0, 0, 0, 1])>>> df = pd.DataFrame({
>             'id': ids,
>             'groups': groups,
>             'value': values
>     }).set_index('id')>>> df.groupby('groups').agg(pd.Series.mode)
>          value
> groups0      0     01      0     02      0     0
>        1     1
>
> there is an extra index after aggregation, and I am not very sure if this
> is desired behaviour? could you pls give some thoughts on it? it seems that
> this extra index comes from output of mode which is a Series. should this
> be like this? or this extra index should be removed?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/issues/31256?email_source=notifications&email_token=AAKAOIS63TI4RUUQSRS5R5TQ75B7VA5CNFSM4KK3WLF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKBACOY#issuecomment-578945339>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKAOIWLJ4ZFOZRMCEN5CS3Q75B7VANCNFSM4KK3WLFQ>
> .
>

@charlesdong1991 I don't think that case is valid. `.agg(f)` is supposed to return a scalar. But `Series.mode` isn't an aggregation, it returns a Series.
@jorisvandenbossche do you think this is a blocker?
The mode issue is fishy (I am also not sure how/if this should work, as it is not a typical aggregation), but the other ones seem more as actual regressions:

pd 0.25:
```
In [1]: >>> import pandas as pd 
   ...:  
   ...: >>> ids = range(5) 
   ...: >>> groups = pd.Series([0, 1, 1, 2, 2]) 
   ...: >>> values = pd.Categorical([0, 0, 0, 0, 1]) 
   ...: >>> df = pd.DataFrame({ 
   ...:             'id': ids, 
   ...:             'groups': groups, 
   ...:             'value': values 
   ...:     }).set_index('id') 
   ...:  
   ...: >>> df.groupby('groups').agg(pd.Series.nunique)  
Out[1]: 
        value
groups       
0           1
1           1
2           2
```

vs master
```
In [1]: >>> import pandas as pd 
   ...:  
   ...: >>> ids = range(5) 
   ...: >>> groups = pd.Series([0, 1, 1, 2, 2]) 
   ...: >>> values = pd.Categorical([0, 0, 0, 0, 1]) 
   ...: >>> df = pd.DataFrame({ 
   ...:             'id': ids, 
   ...:             'groups': groups, 
   ...:             'value': values 
   ...:     }).set_index('id') 
   ...:  
   ...: >>> df.groupby('groups').agg(pd.Series.nunique) 
Out[1]: 
       value
groups      
0          1
1          1
2        NaN
```
Here it is the column being aggregated that is a categorical, the other issue it was the index of the dataframe being categorical
cc @jbrockmendel 
Hi, @jorisvandenbossche  and @TomAugspurger  (I am trying to fix tonight at least for `nunique` and `count`) Many thanks for your answer!

While working on the PR, i found another behaviour issue: is `Int64` an extension array dtype?

```python
>>> from pandas.core.dtypes.common import is_extension_array_dtype
>>> is_extension_array_dtype("Int64")
True

>>> from pandas.core.dtypes.common import is_extension_type
>>> is_extension_type("Int64")
False
```

IIUC, `is_extension_type` should be replaced by `is_extension_array_dtype`, and later be deprecated, so at least this two should return the same result. And `Int64` does not seem to be an extension type, right (or it's IntegerArray?) Please correct me if I misunderstand the usage, thanks!

Commit message: fixup

Patch to review:
diff --git a/doc/source/whatsnew/v1.0.0.rst b/doc/source/whatsnew/v1.0.0.rst
index 2abe85f042af1..3e9920b422814 100755
--- a/doc/source/whatsnew/v1.0.0.rst
+++ b/doc/source/whatsnew/v1.0.0.rst
@@ -626,6 +626,52 @@ consistent with the behaviour of :class:`DataFrame` and :class:`Index`.
    DeprecationWarning: The default dtype for empty Series will be 'object' instead of 'float64' in a future version. Specify a dtype explicitly to silence this warning.
    Series([], dtype: float64)
 
+Result dtype inference changes for resample operations
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The rules for the result dtype in :meth:`DataFrame.resample` aggregations have change for extension types (:issue:`31359`).
+Previously, pandas would attempt to convert the result back to the original dtype, falling back to the usual
+inference rules if that was not possible. Now, pandas will only return a result of the original dtype if the
+scalar values in the result are instances of the extension dtype's scalar type.
+
+.. ipython:: python
+
+   df = pd.DataFrame({"A": ['a', 'b']}, dtype='category',
+                     index=pd.date_range('2000', periods=2))
+   df
+
+
+*pandas 0.25.x*
+
+.. code-block:: python
+
+   >>> df.resample("2D").agg(lambda x: 'a').A.dtype
+   CategoricalDtype(categories=['a', 'b'], ordered=False)
+
+*pandas 1.0.0*
+
+.. ipython:: python
+
+   df.resample("2D").agg(lambda x: 'a').A.dtype
+
+This fixes an inconsistency between ``resample`` and ``groupby``.
+This also fixes a potential bug, where the **values** of the result might change
+depending on how the results are cast back to the original dtype.
+
+*pandas 0.25.x*
+
+.. code-block:: python
+
+   >>> df.resample("2D").agg(lambda x: 'c')
+
+        A
+   0  NaN
+
+.. ipython:: python
+
+   df.resample("2D").agg(lambda x: 'c')
+
+
 .. _whatsnew_100.api_breaking.python:
 
 Increased minimum version for Python
diff --git a/pandas/core/groupby/groupby.py b/pandas/core/groupby/groupby.py
index aa21aa452be95..02e9383314d36 100644
--- a/pandas/core/groupby/groupby.py
+++ b/pandas/core/groupby/groupby.py
@@ -813,9 +813,10 @@ def _try_cast(self, result, obj, numeric_only: bool = False):
                 # datetime64tz is handled correctly in agg_series,
                 #  so is excluded here.
 
-                # return the same type (Series) as our caller
-                cls = dtype.construct_array_type()
-                result = try_cast_to_ea(cls, result, dtype=dtype)
+                if len(result) and isinstance(result[0], dtype.type):
+                    cls = dtype.construct_array_type()
+                    result = try_cast_to_ea(cls, result, dtype=dtype)
+
             elif numeric_only and is_numeric_dtype(dtype) or not numeric_only:
                 result = maybe_downcast_to_dtype(result, dtype)
 
diff --git a/pandas/core/groupby/ops.py b/pandas/core/groupby/ops.py
index 679d3668523c2..2e95daa392976 100644
--- a/pandas/core/groupby/ops.py
+++ b/pandas/core/groupby/ops.py
@@ -543,6 +543,17 @@ def _cython_operation(
             if mask.any():
                 result = result.astype("float64")
                 result[mask] = np.nan
+        elif (
+            how == "add"
+            and is_integer_dtype(orig_values.dtype)
+            and is_extension_array_dtype(orig_values.dtype)
+        ):
+            # We need this to ensure that Series[Int64Dtype].resample().sum()
+            # remains int64 dtype.
+            # Two options for avoiding this special case
+            # 1. mask-aware ops and avoid casting to float with NaN above
+            # 2. specify the result dtype when calling this method
+            result = result.astype("int64")
 
         if kind == "aggregate" and self._filter_empty_groups and not counts.all():
             assert result.ndim != 2
diff --git a/pandas/tests/groupby/aggregate/test_aggregate.py b/pandas/tests/groupby/aggregate/test_aggregate.py
index 67bdcc246579e..2d31996a8a964 100644
--- a/pandas/tests/groupby/aggregate/test_aggregate.py
+++ b/pandas/tests/groupby/aggregate/test_aggregate.py
@@ -663,6 +663,27 @@ def test_aggregate_mixed_types():
     tm.assert_frame_equal(result, expected)
 
 
+@pytest.mark.xfail(reason="Not implemented.")
+def test_aggregate_udf_na_extension_type():
+    # https://github.com/pandas-dev/pandas/pull/31359
+    # This is currently failing to cast back to Int64Dtype.
+    # The presence of the NA causes two problems
+    # 1. NA is not an instance of Int64Dtype.type (numpy.int64)
+    # 2. The presence of an NA forces object type, so the non-NA values is
+    #    a Python int rather than a NumPy int64. Python ints aren't
+    #    instances of numpy.int64.
+    def aggfunc(x):
+        if all(x > 2):
+            return 1
+        else:
+            return pd.NA
+
+    df = pd.DataFrame({"A": pd.array([1, 2, 3])})
+    result = df.groupby([1, 1, 2]).agg(aggfunc)
+    expected = pd.DataFrame({"A": pd.array([1, pd.NA], dtype="Int64")}, index=[1, 2])
+    tm.assert_frame_equal(result, expected)
+
+
 class TestLambdaMangling:
     def test_basic(self):
         df = pd.DataFrame({"A": [0, 0, 1, 1], "B": [1, 2, 3, 4]})
diff --git a/pandas/tests/groupby/test_categorical.py b/pandas/tests/groupby/test_categorical.py
index 9323946581a0d..6beb43a39383b 100644
--- a/pandas/tests/groupby/test_categorical.py
+++ b/pandas/tests/groupby/test_categorical.py
@@ -1342,3 +1342,33 @@ def test_series_groupby_categorical_aggregation_getitem():
     result = groups["foo"].agg("mean")
     expected = groups.agg("mean")["foo"]
     tm.assert_series_equal(result, expected)
+
+
+@pytest.mark.parametrize(
+    "func, expected_values",
+    [(pd.Series.nunique, [1, 1, 2]), (pd.Series.count, [1, 2, 2])],
+)
+def test_groupby_agg_categorical_columns(func, expected_values):
+    # 31256
+    df = pd.DataFrame(
+        {
+            "id": [0, 1, 2, 3, 4],
+            "groups": [0, 1, 1, 2, 2],
+            "value": pd.Categorical([0, 0, 0, 0, 1]),
+        }
+    ).set_index("id")
+    result = df.groupby("groups").agg(func)
+
+    expected = pd.DataFrame(
+        {"value": expected_values}, index=pd.Index([0, 1, 2], name="groups"),
+    )
+    tm.assert_frame_equal(result, expected)
+
+
+def test_groupby_agg_non_numeric():
+    df = pd.DataFrame(
+        {"A": pd.Categorical(["a", "a", "b"], categories=["a", "b", "c"])}
+    )
+    result = df.groupby([1, 2, 1]).nunique()
+    expected = pd.DataFrame({"A": [2, 1]}, index=[1, 2])
+    tm.assert_frame_equal(result, expected)
diff --git a/pandas/tests/resample/test_datetime_index.py b/pandas/tests/resample/test_datetime_index.py
index 4860329718f54..3ad82b9e075a8 100644
--- a/pandas/tests/resample/test_datetime_index.py
+++ b/pandas/tests/resample/test_datetime_index.py
@@ -122,7 +122,9 @@ def test_resample_integerarray():
 
     result = ts.resample("3T").mean()
     expected = Series(
-        [1, 4, 7], index=pd.date_range("1/1/2000", periods=3, freq="3T"), dtype="Int64"
+        [1, 4, 7],
+        index=pd.date_range("1/1/2000", periods=3, freq="3T"),
+        dtype="float64",
     )
     tm.assert_series_equal(result, expected)
 
diff --git a/pandas/tests/resample/test_timedelta.py b/pandas/tests/resample/test_timedelta.py
index d1bcdc55cb509..a4d14f127b80e 100644
--- a/pandas/tests/resample/test_timedelta.py
+++ b/pandas/tests/resample/test_timedelta.py
@@ -105,7 +105,7 @@ def test_resample_categorical_data_with_timedeltaindex():
         index=pd.to_timedelta([0, 10], unit="s"),
     )
     expected = expected.reindex(["Group_obj", "Group"], axis=1)
-    expected["Group"] = expected["Group_obj"].astype("category")
+    expected["Group"] = expected["Group_obj"]
     tm.assert_frame_equal(result, expected)
 
 


Code review comments:
diff_hunk: @@ -626,6 +626,52 @@ consistent with the behaviour of :class:`DataFrame` and :class:`Index`.
    DeprecationWarning: The default dtype for empty Series will be 'object' instead of 'float64' in a future version. Specify a dtype explicitly to silence this warning.
    Series([], dtype: float64)
 
+Result dtype inference changes for resample operations
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The rules for the result dtype in :meth:`DataFrame.resample` aggregations have change for extension types (:issue:`31359`).
path: doc/source/whatsnew/v1.0.0.rst
text: @user1:
```suggestion
The rules for the result dtype in :meth:`DataFrame.resample` aggregations have changed for extension types (:issue:`31359`).
```

diff_hunk: @@ -1342,3 +1342,33 @@ def test_series_groupby_categorical_aggregation_getitem():
     result = groups["foo"].agg("mean")
     expected = groups.agg("mean")["foo"]
     tm.assert_series_equal(result, expected)
+
+
+@pytest.mark.parametrize(
+    "func, expected_values",
+    [(pd.Series.nunique, [1, 1, 2]), (pd.Series.count, [1, 2, 2])],
+)
+def test_groupby_agg_categorical_columns(func, expected_values):
+    # 31256
+    df = pd.DataFrame(
+        {
+            "id": [0, 1, 2, 3, 4],
+            "groups": [0, 1, 1, 2, 2],
+            "value": pd.Categorical([0, 0, 0, 0, 1]),
+        }
+    ).set_index("id")
+    result = df.groupby("groups").agg(func)
+
+    expected = pd.DataFrame(
+        {"value": expected_values}, index=pd.Index([0, 1, 2], name="groups"),
+    )
+    tm.assert_frame_equal(result, expected)
+
+
+def test_groupby_agg_non_numeric():
+    df = pd.DataFrame(
+        {"A": pd.Categorical(["a", "a", "b"], categories=["a", "b", "c"])}
+    )
+    result = df.groupby([1, 2, 1]).nunique()
path: pandas/tests/groupby/test_categorical.py
text: @user1:
I would also do the `agg(pd.Series.nunique)` here. The nunique method takes a different path (and was actually working fine on master)