DEPR: DataFrameGroupBy.apply operating on the group keys
State: closed

- [x] closes #7155 (Replace xxxx with the GitHub issue number)
- [x] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [x] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [x] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [x] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.

This is a 2nd go at #52477 which was reverted in #52921.

Deprecates groupby.apply sometimes operating on the grouping columns. This only occurs when (a) the grouping columns are part of the DataFrame and (b) when the operation on the grouping columns does not raise a TypeError. Otherwise we fallback to excluding the grouping columns. Other operations (excluding filters) do not operate on the grouping columns. Users can still operate on the grouping columns by including them in selection (this works across groupby), e.g.

    df.groupby('a')[['a', 'b', 'c']].sum()
    df.groupby('a')[['a', 'b', 'c']].apply(lambda x: x.sum())

We do our best to not warn if we can guarantee that the grouping columns were not used. Unfortunately, we can't see if the supplied UDF subsets columns itself. To avoid the noise of the deprecation and adopt future behavior, users can specify `include_groups=False`. This will become the default and only valid value of this argument in pandas 3.0.

All of the above also applies to `.groupby(...).resample` since that uses `.groupby(...).apply` under the hood.

cc @phofl @jorisvandenbossche @topper-123  

Labels:
name: Groupby
name: Deprecate
description: Functionality to remove in pandas
name: Apply
description: Apply, Aggregate, Transform, Map

Comments:
I think there was a previous PR on  this, can you link to it?
Is this issue present in resample etc. also, or is it only in groupby?
> I think there was a previous PR on this, can you link to it?

Added to the OP.

> Is this issue present in resample etc. also, or is it only in groupby?

Yes - added to the OP. That is handled here as well.
> BTW, in https://github.com/pandas-dev/pandas/pull/54747#issuecomment-1703776140 you mentioned that groupby.apply has magical behaviors. Did you think about this try/except here, or is there other code locations you think about also?

No, that comment is about how apply infers the results of the UDF. We can always control what is passed to the UDF, so in my mind that is not a serious issue.
@phofl.
Thanks @rhshadrach 
@rhshadrach just found this case:

```
df = pd.DataFrame({"a": [1,2 ,3 ], "b": 1})
df.groupby(df.b).apply(lambda x: x)
```

I know we are doing some magic here, but I think here is a case for including b in apply as well?
Not sure I understand. Are you saying after the deprecation is enforced we should still be including `b` in the result?
Assuming that is the case, it's contrary to the behavior of other ops.

```
df = pd.DataFrame({"a": [1,2 ,3 ], "b": 1})
print(df.groupby(df.b).sum())
#    a
# b
# 1  6
print(df.groupby(df.b).transform(lambda x: x))
#    a
# 0  1
# 1  2
# 2  3
```

In general, [the docs state](https://pandas.pydata.org/pandas-docs/dev/user_guide/groupby.html#splitting-an-object-into-groups) that `df.groupby('b')` is syntactic sugar for `df.groupby(df.b)`.
> Not sure I understand. Are you saying after the deprecation is enforced we should still be including b in the result?

Yes that was my idea, this syntax looks like the users wants b in apply as well.

> In general, the docs state that df.groupby('b') is syntactic sugar for df.groupby(df.b).

I did not know that, then you can ignore what I've said. Current behavior is good then!
@rhshadrach how annoying would it be to keep this deprecation around for a bit longer? So we can start with a DeprecationWarning for this one? 
(I don't know how much it interferes with other potential improvements or cleanups that are planned)

(of course, if we do another 2.3 that just bumps some deprecation->future warnings, that could do that for this one as well, and still change the behaviour in 3.0)
This is quite bad behavior - it is inconsistent with the rest of groupby, produces self-inconsistent and confusing results, and can lead to confusion when debugging. It would not, however, interfere with future improvements I have planned. Still, I think we should weigh heavily the impact of keeping this behavior for another 1.5 years.
But if we would bump it to FutureWarning in a 2.3 (I know we haven't yet decided on that idea though, but assuming we do that), we can still change the behaviour in 3.0, and no need for keeping it another 1.5 years?

I just have the feeling (from the seaborn example, among others) that this is a quite widespread warning (basically whenever you use groupby.apply, even also when you are not actually using the group columns, which might be the majority of the cases), and so doing a deprecation warning first would reduce some of the noise from usage in libraries.
We already tagged 3.0 and I don’t think we should do a 2.3 generally speaking 
(I mean your idea of doing a 2.3 on the 2.2.x branch, definitely not from main)
That’s something I wouldn’t object to but I have moved away from this idea over the last few weeks tbh. I’ve objected to this deprecation in the past, but @rhshadrach addressed this with the keyword, the fix is a one line change…
I don't have any opposition to 2.3 assuming it doesn't delay 3.0. Likewise, assuming 2.3, I have no opposition to changing this to a DeprecationWarning.
I would prefer starting with a DeprecationWarning then
Another question here: you added the `include_groups` keyword, to allow users to already get the future behaviour (and silence the warning) by passing `include_groups=False`. 
But this doesn't allow to specify `include_groups=True` at the moment (you just still get the warning, since that is the default value for that option). You also mentioned that in the top post:

> This will become the default and only valid value of this argument in pandas 3.0.

Is there a _technical_ reason to not allow that option in the future? Or just because it is deemed unnecessary? (in the idea that most? users would expect it without the group columns)
> Is there a _technical_ reason to not allow that option in the future? Or just because it is deemed unnecessary? (in the idea that most? users would expect it without the group columns)

From a user standpoint, primarily for consistency with other groupby operations. We have `_selected_obj` and `_obj_with_exclusions` that differ in subtle ways and have caused bugs in the past. I would like to keep only one of these.
@jorisvandenbossche @phofl I plan to change this to a FutureWarning in 2.3. Let me know if there is any opposition.