BUG: groupby().agg fails on categorical column
State: closed

- [ ] closes #31450 
- [ ] tests added / passed
- [ ] passes `black pandas`
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Groupby
name: Categorical
description: Categorical Data Type

Comments:
Hello @charlesdong1991! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:

































There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-02-04 08:07:08 UTC
is this limited to "first"?  i think the conclusion from previous thread is that we need to be more systematic about when we call _try_cast
@jbrockmendel yeah, very right, i was about to change something in `_try_cast` or before `_try_cast`, but haven't seen the follow-up discussions yet, and due to the added change of `isinstance(result[0], dtype.type)` in `_try_cast`, the case of `first` won't be casted back which it should be. 
so I just added as an experiment to see if the problem could be fixed, and prepare a more generic solution if so.
Haven't had a chance to look closely yet, but I think my preferred approach is for the calling function to decide what the output dtype should be based on the input dtype. Not sure how best to put that to code either.
@WillAyd @TomAugspurger I agree, and i am not a big fan of this current approach, this is the reason i have [WIP] in the title. I tried to fix when to call this `_try_cast` depending on the situation and type, but not working, and causes a lot of errors locally, so i am still trying to find a better/more robust fix. 

Sorry about this noise in PRs, and i wish i could update this soon!
Thanks for working on this. It's good that we're exploring possible
solutions. We should take a bit of time to try and get this right.

On Fri, Jan 31, 2020 at 7:37 AM Kaiqi Dong <notifications@github.com> wrote:

> @WillAyd <https://github.com/WillAyd> @TomAugspurger
> <https://github.com/TomAugspurger> I agree, and i am not a big fan of
> this current approach, this is the reason i have [WIP] in the title. I
> tried to fix when to call this _try_cast depending on the situation and
> type, but not working, and causes a lot of errors locally, so i am still
> trying to find a better/more robust fix.
>
> Sorry about this noise in PRs, and i wish i could update this soon!
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/31470?email_source=notifications&email_token=AAKAOIQL5KBDBER4RG25CMDRAQSSLA5CNFSM4KN3O4JKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKOVKXI#issuecomment-580736349>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKAOISV7CVPVR2LFT33PPTRAQSSLANCNFSM4KN3O4JA>
> .
>

I still have some confusion on what should be the correct behaviour, based on the https://github.com/pandas-dev/pandas/pull/31359#issuecomment-579799671
it seems for `IntegerArray`, `mean` should return a float type result because `IntegerArray._reduce("mean")` returns float. However, in the case below, currently doing `agg.mean` on a normal array with integer returns `int64` output instead of `float`? is it correct? if so, could this be considered as inconsistency between extension array and normal array and then cause confusion to users? @TomAugspurger @jreback @jbrockmendel @WillAyd @jorisvandenbossche 

```
def test_groupby_extension_agg(self, as_index, data_for_grouping):
        df = pd.DataFrame({"A": [1, 1, 2, 2, 3, 3, 1, 4], "B": data_for_grouping})
        result = df.groupby("B", as_index=as_index).A.mean()
        _, index = pd.factorize(data_for_grouping, sort=True)
    
        index = pd.Index(index, name="B")
        expected = pd.Series([3, 1, 4], index=index, name="A")
        if as_index:
>           self.assert_series_equal(result, expected)
E           AssertionError: Attributes of Series are different
E           
E           Attribute "dtype" are different
E           [left]:  float64
E           [right]: int64
```

IMO the result dtype should only depend on the dtype of the inputs, not the values. IIUC the result can only be int dtype in this case because the mean of the values happens to be equal to an integer? If so, I’d say that’s a bug. The result dtype should always be a float. 
@TomAugspurger  thanks for your quick reply. Indeed, in these cases the output has int dtype because it happens to be equal to integer. 
the int return in mean is a bug it should be float
likely just happened to coerce it before
Thanks for working on this @charlesdong1991. I hope to write up more later, but in general, I like the general idea of the calling function indicating the result dtype is a good idea.

For our immediate needs, I think we can use a more targeted fix to restore the 0.25.x behavior. I'll propose that as a separate PR.
https://github.com/pandas-dev/pandas/pull/31668 for the narrowly scoped alternative to just fix the regression, but not the general issue.
merged in https://github.com/pandas-dev/pandas/pull/31668, which is a backportable patch. thanks @charlesdong1991 
> #31668 for the narrowly scoped alternative to just fix the regression, but not the general issue.

this is much cleaner for fixing regression!! awesome!! @TomAugspurger 👍