API: User-control of result keys in GroupBy.apply
State: closed

Currently we determine whether to include the group keys as a level of the result's MultiIndex by looking at whether the result's index matches the original DataFrame. This provides a keyword to control that behavior so that users can consistently get the group keys or not, regardless of whether the udf happens to be a transform or not. The default behavior remains the same.

I called the keyword `result_group_keys`, but would welcome alternatives.

Closes #34809
Closes #31612
Closes #14927
Closes #13056
Closes #27212
Closes #9704

 cc @WillAyd and @jorisvandenbossche from the issue.

Labels:
name: Groupby
name: Deprecate
description: Functionality to remove in pandas

Comments:
Is it possible to leverage the existing as_index keyword instead of introducing a new one? I've never been quite clear on what the former should do, so maybe it should cover at least part of this
adding more keywords is a huge -1 here. Ok if you deprecate and remove one or more, but adding more to an already *really* complex operation is sure to cause way more bugs.
The documentation of `group_keys` isn't clear. Is that adding the groups to the inputs before the `apply` or the outputs after the `apply`? If it's intended for the outputs I can resuse it, but currently it seemingly has no effect for this usecase:

```python
In [10]: def f(x):
    ...:     return x.copy()  # same index

In [11]: def g(x):
    ...:     return x.copy().rename(lambda x: x + 1)  # different index

In [12]: df = pd.DataFrame({"A": ['a', 'b'], "B": [1, 2]})


In [4]: df.groupby("A", group_keys=True).apply(f)
Out[4]:
   A  B
0  a  1
1  b  2

In [5]: df.groupby("A", group_keys=True).apply(g)
Out[5]:
     A  B
A
a 1  a  1
b 2  b  2

In [6]: df.groupby("A", group_keys=False).apply(f)
Out[6]:
   A  B
0  a  1
1  b  2

In [7]: df.groupby("A", group_keys=False).apply(g)
Out[7]:
   A  B
1  a  1
2  b  2
```
`group_keys` does kick in here, when the applied function returns a subset of the dataframe..

```python

In [8]: df = DataFrame({"key": [1, 1, 1, 2, 2, 2, 3, 3, 3], "value": range(9)})
   ...: df.groupby("key", group_keys=True).apply(lambda x: x[:2])
   ...:
   ...:
   ...:
Out[8]:
       key  value
key
1   0    1      0
    1    1      1
2   3    2      3
    4    2      4
3   6    3      6
    7    3      7

In [9]: df = DataFrame({"key": [1, 1, 1, 2, 2, 2, 3, 3, 3], "value": range(9)})
   ...: df.groupby("key", group_keys=False).apply(lambda x: x[:2])
   ...:
   ...:
   ...:
   ...:
Out[9]:
   key  value
0    1      0
1    1      1
3    2      3
4    2      4
6    3      6
7    3      7
```

So it seems the intent is similar to what we want. However I don't know about the defaults. `group_keys` defaults to true. We'll need to see whether that's appropriate for this case.
might be ok to add result_type keyword similar to what we use in apply but must depreciate group_keys then
Well, 50 failures in `tests/groupby` when I just treat `group_keys` like you're suggesting (adding the group keys to the output). I'll see what I can do.
It does seem like the intent of `group_keys` is to control this sort of behavior. We just didn't apply it consistently for a few reasons.

1. It just wasn't used in `_GroupBy._python_apply_general`.
2. It's dropped when slicing a DataFrameGroupBy object

```python

In [2]: df = pd.DataFrame({"A": [0, 0, 1], "B": [1, 2, 3]})

In [3]: gb = df.groupby("A", group_keys=False)

In [4]: gb.group_keys
Out[4]: False

In [5]: gb[['B']].group_keys
Out[5]: True

In [6]: gb['B'].group_keys
Out[6]: True
```

I'll push a commit later with some updates to always honor group keys, but my initial reaction is that this is too large of a change. I'm sure there are people relying on the buggy behavior, so we'll need to do this with a deprecation.
OK, tests should hopefully pass, but this isn't mergeable yet because of the behavior change. Namely honoring `group_keys` changes the output shape of "transform-like" `.apply`:

```python
# 1.0.4
In [2]: df = pd.DataFrame({"A": [0, 0, 1], "B": [1, 2, 3]})

In [3]: df.groupby("A").apply(lambda x: x)
Out[3]:
   A  B
0  0  1
1  0  2
2  1  3
```

```python
# PR
In [3]: df.groupby("A").apply(lambda x: x)
Out[3]:
     A  B
A
0 0  0  1
  1  0  2
1 2  1  3
```

My next commit will have compatibility layer. We'll change the default `group_keys` to None and warn if we get to a case where `group_keys=None` and we've detected a transform-like apply.

Note that this doesn't help people who were already doing `.groupby(..., group_keys=True).apply(lambda x: x)`, but I think that's unavoidable. I think we just call that buggy behavior that's been fixed.
Here's the warning

```python
In [1]: import pandas as pd

In [2]: df = pd.DataFrame({"A": [0, 0, 1], "B": [1, 2, 3]})

In [3]: df.groupby("A").apply(lambda x: x)
/Users/taugspurger/.virtualenvs/pandas-dev/bin/ipython:1: FutureWarning: Not prepending group keys to the result index of transform-like apply. In the future, the group keys will be included in the index, regardless of whether the applied function returns a like-indexed object.
To preserve the previous behavior, use

        >>> .groupby(..., group_keys=False)

To adopt the future behavior and silence this warning, use

        >>> .groupby(..., group_keys=True)
  #!/Users/taugspurger/Envs/pandas-dev/bin/python
Out[3]:
   A  B
0  0  1
1  0  2
2  1  3
```
On board with the direction here; I was initially thinking through if the default should be True or False going forward but I think the way you have it is nice if can get green
There's a ton of warnings. At least some of them are "false positives"

```
In [3]: df = pd.DataFrame({"A": [0, 0, 1, 1], "B": [0, 1, 2, 2], "C": [1, 2, 3, 4]})

In [4]: df.groupby(["A", "B"]).apply(len)
/Users/taugspurger/.virtualenvs/pandas-dev/bin/ipython:1: FutureWarning: Not prepending group keys to the result index of ...
Out[4]:
A  B
0  0    1
   1    1
1  2    2
dtype: int64
```

That's because the fast and slow paths in BaseGrouper.apply have different definitions of `mutated`. Hopefully address that will cut down the number of warninsg.
Could I get an opinion on expected behavior for two cases? With 1.0.4, we're inconsistent in prepending the group keys to the result index when `as_index=False` and slicing the GroupBy object:
 
**Case 1**

Edit from Joris: used example dataframe might be `df = pd.DataFrame({"A": [1, 1, 5], "B": [2, 4, 6]})`

```python
# 1.0.4
In [45]: df.groupby("A", as_index=False).apply(lambda x: x.cumsum())
Out[45]:
   A  B
0  1  2
1  2  6
2  5  6

In [46]: df.groupby("A", as_index=False)['B'].apply(lambda x: x.cumsum())
Out[46]:
0  0    2
   1    6
1  2    6
Name: B, dtype: int64
```

IMO, `Out[46]` should match this

```python
In [50]: df.B.groupby(df.A).apply(lambda x: x.cumsum())
Out[50]:
0    2
1    6
2    6
Name: B, dtype: int64
```

but I'm not 100% sure on the expected behavior of `as_index=False`. In terms of this PR, that In[45] and In[46] will both warn, since they're transforms, and `group_keys` should be used to determine whether the keys are in the index.

**Case 2**

Right now, methods like `GroupBy.diff()` are implemented in terms of apply, and so run into this behavior:

```python
In [14]: df.groupby("A").diff()
/Users/taugspurger/.virtualenvs/dask-dev/bin/ipython:1: FutureWarning: Not prepending group keys to the result index of transform-like apply. In the future, the group keys will be included in the index, regardless of whether the applied function returns a like-indexed object.
To preserve the previous behavior, use

        >>> .groupby(..., group_keys=False)

To adopt the future behavior and silence this warning, use

        >>> .groupby(..., group_keys=True)
  #!/Users/taugspurger/Envs/dask-dev/bin/python
Out[14]:
     B    C
0  NaN  NaN
1  1.0  1.0
2  NaN  NaN
3  0.0  1.0
```

What do we want there? IMO going through apply is just an implementation detail and these should rather be treated like the other transforms (no warnings, and `group_keys`  does not apply here)
> but I'm not 100% sure on the expected behavior of `as_index=False`. In terms of this PR, that In[45] and In[46] will both warn, since they're transforms, and `group_keys` should be used to determine whether the keys are in the index.

Yea the keywords here are a little vague on intent, but I suppose `as_index` is only documented to work for aggregated output if that helps. So maybe it should be ignored in both cases since they are not aggregations?
> So maybe it should be ignored in both cases since they are not aggregations?

That's my understanding as well. But then it feels like `as_index` controls the result index for aggregations, and `group_keys` controls the index for for `apply`. So why do we need both? Anyway, fixing that is out of scope for now I think.
Philosophy question for today: is this a reduction or a transformation?

```python
>>> df = pd.DataFrame(columns=["A", "B"])
>>> df.groupby("A").sum()
```

The index is the same (empty) so it's a transform. OTOH, it's clearly a reduction :)

Right now this actually goes through `_python_apply_general` (via a very circuitous route) and so we warn. I'm hopeful we can fix this by not going through `_python_apply_general`, since something like `df.groupby("A").apply(lambda x: x)` on an empty dataframe *should* warn.
Hello @TomAugspurger! 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 2022-03-29 02:09:45 UTC
Down to the last couple warnings. Right now I'm running into an issue with how BinGrouper treats NA groups differently than BaseGrouper

```python
In [79]: df = pd.DataFrame({"A": [1, 2, 3]}, index=pd.PeriodIndex([pd.NaT, pd.NaT, pd.NaT], freq="D"))

In [80]: a = df.groupby(df.index).grouper

In [81]: b = df.resample("1D")._get_binner()[1]

In [82]: a.codes
Out[82]: [array([-1, -1, -1])]

In [83]: b.codes
Out[83]: [array([], dtype=int64)]

```

Will try to solve that independently.
I added a few `type: ignore`s in 0fa2104fbd39a8b18853f2a7023fa6c434e8f19f. I wasn't able to understand any of the mypy errors.

One seems especially strange: `Incompatible types in assignment (expression has type "NDFrame", variable has type "Series")`. Series is a subclass of NDFrame, so why is mypy complaining?
> group_key and as_index serve almost exactly the same purpose

Yeah, just looking at the docs I think they're about identical, aside from `group_keys` being for `.apply` and `as_index` being for `.agg`. Though I don't know how `as_index` interacts with grouping by non-column groupers.
There is still another difference between the two transform like examples (like-indexed vs non like-indexed), which show if the order the values in column "A" in the example is changed ([1, 2, 1] instead of [1, 2, 2], so non-contiguous groups):

```
In [12]: df = pd.DataFrame({"A": [1, 2, 1], "B": [1, 2, 3]})  

In [14]: df.groupby("A", group_keys=False).apply(lambda x: x) 
Out[14]: 
   A  B
0  1  1
1  2  2
2  1  3

In [15]: df.groupby("A", group_keys=True).apply(lambda x: x) 
Out[15]: 
     A  B
A        
1 0  1  1
  2  1  3
2 1  2  2

In [16]: df.groupby("A", group_keys=True).apply(lambda x: x.rename(np.exp))  
Out[16]: 
            A  B
A               
1 1.000000  1  1
  7.389056  1  3
2 2.718282  2  2

In [17]: df.groupby("A", group_keys=False).apply(lambda x: x.rename(np.exp))  
Out[17]: 
          A  B
1.000000  1  1
7.389056  1  3
2.718282  2  2
```

The output of `Out[14]:` is the only one that has a different order of the rows.

So that's still a "special case" for the like-indexed return value, which is what we are trying to avoid here?
Hmm, thanks. I'm not sure what to think about this yet.

We get Out[14] there becuase of this explicit reindex: https://github.com/pandas-dev/pandas/blob/9827ce00f95927ddf6a0565450bc175879454d4e/pandas/core/groupby/groupby.py#L1128. I think that only seems possible when when know the result index exactly: it matches the input index. Is it possible to say the same when `group_keys=True`? I suppose so...

But then this cuts against the other behavior of `groupby` grouping your keys together... I don't know what to do here yet.
This also comes back to my comment in the original issue here: https://github.com/pandas-dev/pandas/issues/34809#issuecomment-644993342. Personally, I don't think that `groupby().apply()` should try to be that smart and return a quite different output based on a brittle check like exactly equal index.
Do you have a preference on how to resolve that situation? Is it for `.groupby.apply()` to not do the reindex in the like-index case, and have the result of the groupby always be ordered by group? That leaves transform for when you need to do groupby-like operations that return a like-indexed dataframe.

However, I think we'd need to add the ability to transform table-wise, rather than just columnwise, before we can deprecate the current apply behavior.
> However, I think we'd need to add the ability to transform table-wise, rather than just columnwise, before we can deprecate the current apply behavior.

Yeah, that's also what I noticed in https://github.com/pandas-dev/pandas/issues/34809#issuecomment-644993342, that `transform` is quite different right now, and might not be a straightforward replacement for all cases.

Another route might also be a `return_type` keyword in `apply`.

See also https://github.com/pandas-dev/pandas/issues/13056#issuecomment-403300216 with an older overview of some of the logic (not sure it's still fully correct).

Given that we are so close the an RC, and it's not yet clear what behaviour we actually want, we might also want to leave it for 1.2 (and then potentially temporally reverting  #31613)
I would prefer to wait until 1.2 for this (e.g. merge immediately after the tag). We normally let new features sit in master for a while.
Note this is not a new feature, it is trying to finalize changes that were started in other PRs (so if delaying, we might need to revert the original PR, as mentioned above)
I don't think reverting #31613 makes any sense here. This PR is a substantial (though really good) change. it needs to sit in master for a while.
So, waiting till 1.2 makes things difficult for me. I need to update dask to support pandas 1.1.x, and the implementation of `.groupby().apply` makes things basically impossible (since Dask uses empty dataframes to infer result shapes, which breaks when pandas has values-dependent behavior).

https://github.com/pandas-dev/pandas/pull/31613 can I think be safely classified as a bugfix, but it introduces behavior changes that are hard to handle without this PR.
Regardless, doesn't the "does `.apply` reindex to the original output?" discussion need to happen regardless of whether `apply` always respects `group_keys`? To me they seem mostly orthogonal.
> Regardless, doesn't the "does .apply reindex to the original output?" discussion need to happen regardless of whether apply always respects group_keys? To me they seem mostly orthogonal.

They are not fully orthogonal, I think, because if we want to deprecate that, we have right now the chance to do that with the `group_keys` keyword (otherwise we are going to need another keyword, just for the deprecation). 
(of course this all depends on how we see the future behaviour. If we would add something like a `result_type` keyword, the deprecation could also be handled that way, or we might not even have to deprecate anything)

> #31613 can I think be safely classified as a bugfix

If the old behaviour is what we actually want in the future, it seems a bit stupid to now "fix" it to then deprecate it in a next release to go back to the old behaviour. 
(but again, all depends on what we want long-term)

> since Dask uses empty dataframes to infer result shapes, which breaks when pandas has values-dependent behavior

but that problem already exists right now with pandas 1.0 as well? Or what changed in 1.1 that makes this more difficult?