BUG: Fix groupby with "as_index" for categorical multi #13204
State: closed

- [x] closes #13204
- [x] tests added / passed
- [x] passes `git diff upstream/master | flake8 --diff`
- [x] whatsnew entry

Fixes a bug that returns all nan's for groupby(as_index=False) with
multiple column groupers containing a categorical one (#13204).

---

Also:
fixes an internal bug in the string representation of `Grouping`.

``` python
mi = pd.MultiIndex.from_arrays([list("AAB"), list("aba")])
df = pd.DataFrame([[1,2,3]], columns=mi)
gr = df.groupby(df[('A', 'a')])

gr.grouper.groupings
...
TypeError: not all arguments converted during string formatting
```


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

Comments:
## [Current coverage](https://codecov.io/gh/pydata/pandas/pull/13394?src=pr) is **84.34%**

> Merging [#13394](https://codecov.io/gh/pydata/pandas/pull/13394?src=pr) into [master](https://codecov.io/gh/pydata/pandas/branch/master?src=pr) will increase coverage by **<.01%**

``` diff
@@             master     #13394   diff @@
==========================================
  Files           138        138          
  Lines         51113      51120     +7   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43109      43116     +7   
  Misses         8004       8004          
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last updated by [a01644c...374402c](https://codecov.io/gh/pydata/pandas/compare/a01644c07f376b4e2eb48d8dfca7222473e3088d...374402cf769f0af233c22f9015bbba824393bd55)

I'm not quite happy with the last commit and would like to change it. But I'm not sure on the general direction. In particular, whether to preserve categorical dtypes in MultiIndex. In other words, if columns `df['A']` is categorical and

```
out = df.groupby(['A', 'B']).sum()
```

then should/may/must not `out.index.levels[0]` be a CategoricalIndex?

Some possible ways to proceed:
1. keep changes as minimal as possible (no new public methods, no messing with dtypes)
2. ok to change dtypes of MultiIndex levels, but no need for new public methods (expand_all)
3. just cosmetic changes to the last commit
4. take it further and change `MultiIndex.from_product` and `MultiIndex.from_arrays` so that they preserve Categorical dtypes in levels (?)
0. abandon it completely since the code is messy and not much of it can be salvaged

Any comments will be very helpful. 

4) is prob not tested but should preserve (or we would like it to do so )

anything else is a bug - pretty big on trying to preserve types if at all possible
of course want to limit complexity 

this is pretty general - but if u have a specific situation so ears

ideally all of the preservation will be done in Index or subclasses / higher level things should not know/care about dtypes - they just. call methods - so any messiness should be in Index 

@jreback Thanks for the answer.

An example where `pd.MultiIndex.from_product` doesn't preserve categorical types in `levels`:

``` python
cidx = pd.CategoricalIndex(['a', 'b'])

pd.MultiIndex.from_product([cidx, cidx]).levels[0]
Out[84]: Index(['a', 'b'], dtype='object')

# while it preserves all other types
pd.MultiIndex.from_product([pd.datetime(2011,1,1), cidx]).levels[0]
Out[85]: DatetimeIndex(['2011-01-01'], dtype='datetime64[ns]', freq=None)
```

So, I would expect that it returns `CategoricalIndex` if an argument is categorical.

This can be easily changed in `pd.MultiIndex.from_product` and `pd.MultiIndex.from_arrays` (same issue) but the change is a bit hacky (passing `categories=CategoricalIndex(input.categories)` to Categorical constructor).

Now, I think that much more natural is to deal with it inside `Categorical` itself. (Of course, if such a behaviour is desired.) Currently, we have for example:

``` python
# preserves all dtypes inside "categories" as an appropriate Index
pd.Categorical(pd.datetime(2011,1,1)).categories
Out[93]: DatetimeIndex(['2011-01-01'], dtype='datetime64[ns]', freq=None)

# except categorical:
pd.Categorical(cidx).categories
Out[94]: Index(['a', 'b'], dtype='object')
pd.Categorical(pd.Categorical(['a','b'])).categories
Out[95]: Index(['a', 'b'], dtype='object')
```

I'd expect the last two calls return `CategoricalIndex(['a', 'b'])`.

I know that categorical dtype is different than other low level dtypes. So probably the only way to keep it is to have `categories` keep it.

I'd be grateful for more guidance on this (i.e. categorical dtypes):
A. change it in Categorical,
B. change it in MultiIndex
C. start a new issue for discussion
D. do nothing

Thanks

can you update

@jreback Sure, but what to do about this fishiness? 
Is the snippet from my last comment acceptable? Or should I return just an unexpanded data frame?

Its hard to see what you did, push what you have an i'll take another look. If its MultiIndex specific then move to a MI method.

OK, I'll push it soon. 
It's now a very minimal commit (as the last one): only the expansion of a data frame. 

I don't do anything with multiindexes (anything other than `_reindex_output` was already doing) or dtypes - it's independent and maybe I'll get back to it in a separate issue.

@jreback I forgot to ping after the update. If the comments are ok now, should I resubmit it?

yes

@jreback Done

I think this is wrong (from the issue). `index` should not appear here. Note that maybe add a test with a non-integer index as well (it should be dropped), maybe also way one with a name (again should be dropped), and a conflicting name to a column.

```
In [4]: >>> df.groupby(["A","C"], as_index=False).sum().reset_index()
Out[4]: 
   index  A  C  B
0      0  1  1  2
1      1  1  2  2
2      2  1  3  2
```

I'm not sure if I understand. Column `index` is not the index of the original `df`.

Do you mean testing whether `df.index` is not accidentally carried over to `df.groupby(["A","C"], as_index=False).sum()`?

yes the index could should not be there - I am not sure if u r testing this

I'm not testing this. I can do it, no problem.

But something like this seems impossible. With `as_index=False`, the original `df.index` is discarded somewhere inside `groupby` or aggregation, regardless of whether `df` contains categoricals - so such a bug would have appeared earlier. (`_reindex_output` doesn't access `df.index`.)

Moreover, in this case (with a categorical grouper, here column 'C'),  `_reindex_output` is returning `result.reset_index(drop=True)`. So, data frame `df.groupby(["A","C"], as_index=False).sum()` has a `RangeIndex` index, which is then (I believe correctly) reset to a column named `index` by `reset_index()`.

@pijucha I think an error in your tests, pls rebase / update and ping when green. lgtm.

@jreback updated

ok minor change. ping when green.

@jreback Done and updated

thanks @pijucha