[BUG] Fixed behavior of DataFrameGroupBy.apply to respect _group_selection_context
State: closed

- [x] closes #28549
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

This issue needs to be addressed before #28541 can be merged


Labels:
name: Groupby
name: API - Consistency
description: Internal Consistency of API/Behavior

Comments:
Yes there are impacts. The change to [this section](https://github.com/pandas-dev/pandas/blob/master/pandas/core/groupby/groupby.py#L724) guarantees that the grouper column from `apply` will never be in the output unless `as_index` is set to `False`.  Before, it was based on whether or not the reduction operation would fail on the grouper column.
Hmm there is still something weird going on here. On this branch I now see the following behavior:

```python
>>> df = pd.DataFrame({"a": [1, 1, 2, 2, 3, 3], "b": [1, 2, 3, 4, 5, 6]})
>>> df.groupby("a", as_index=False).shift()
     b
0  NaN
1  1.0
2  NaN
3  3.0
4  NaN
5  5.0
>>> df.groupby("a", as_index=False).apply(lambda x: x.shift())
     a    b
0  NaN  NaN
1  1.0  1.0
2  NaN  NaN
3  2.0  3.0
4  NaN  NaN
5  3.0  5.0
```

I would expect both of those outputs to be the same. Came across this looking at #13519 

We generally might need to step back and think about how the `as_index` parameter is presented to end users. Right now it seems to work for `agg` and `apply` but `transform` doesn't respect it (to be fair, the documentation states it is only valid for aggregated output). 

That might be OK, but at the same time I don't think we want people to start doing `apply(lambda x: x.shift())` instead of using `.shift()` if they do desire output with the grouper column(s) in tact
@christopherzimmerman can you rebase
@WillAyd I tracked down the issue with `shift` that you mentioned.  Is that something that is worth a PR or should the behavior stay as it is now?
can you merge master
I am not fully convinced this is a "bug fix". 

The whole `as_index` / when is the group key set as index and when not or whether it is included in the group df or not, is a complete mess, for sure. 
But I think the possibly biggest change in this PR is in the dataframe that is passed to the applied function. While the whatsnew note mostly speaks about including the group key *in the result*, users can actually write functions right now that assume the group key is part of this dataframe on which the function operates.

Basically, that you can think of `df.groupby('a').apply(f)` as doing something like

```
for name, group in df.groupby('a'):
    res = f(group)
    ....
```

which is no longer true (as the `group` passed to `f` changed).

For reducing functions I agree that you typically don't want the group column to be included in the grouped dataframe on which to apply the function. But in general, for custom functions that are not necessarily reducing functions, this might not be necessarily true.


can you merge master and we'll take another look (and @jorisvandenbossche comments as well)
@christopherzimmerman looks like CI is red and some merge conflicts; any chance you can fix those up?
@WillAyd I merged and fixed the failing tests.  It seems like tests keep getting written that rely on the old behavior of apply.
@jreback I'll look into making a PR that is a bit less of an overhaul than this one.  Is the behavior that this PR introduces the desired behavior, with `apply` not containing the grouper columns unless specified?  If that *is* desired, then I'll also need to follow up with PRs to handle `shift`, `resample`, and several others so that they all respect the context as well.

I understand what @jorisvandenbossche is saying about this making some major changes, and I just want to make sure it's correct before spending too much more time on it.
@christopherzimmerman I think the behvaior changes in this PR *are* correct, meaning that these are some long standing bugs. However it would be better to do the non-controversial / easy ones first; the reason is we might need to deprecate some behavior rather than just breaking it. So cleanly separating these changes is key.

thanks for working on this!
@christopherzimmerman happy to have a lesser scope PR to just fix the problem.
@christopherzimmerman if you can push a more limited scope i think we would like to fix these bugs
closing this as stale. as indicated above we would welcome this type of change, but we need to do it incrementally to avoid breaking the world.