BUG: GroupBy.apply() returns different results if a different GroupBy method is called first
State: closed

- [x] closes #34656 
- [x] closes #34271 
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

_Behavioural Changes_
_.apply()_
Calls  to `self._set_group_selection` have been replaced with `with _group_selection_context(self):` for `_agg_general`, `_make_wrapper`, and `nth`. 

Previously these calls to `self._set_group_selection` created a bug in `GroupBy.apply` where calling another method before `.apply` would change the output of `.apply`. This bug is now fixed. 


_Tests_ 
One new test is added to check that the output of `.apply` is constant whether another method is called on the same grouper first. 

Two existing tests were actually dependent on the old buggy-behaviour (i.e. they called GroupBy.sum first and then expected that GroupBy.apply(sum) would exclude the index columns from the results). All of these tests have been amended in a manner that enforces the new consistent output format while preserving the existing test. 

Both of the copy-pastable examples in the linked bug-reports are fixed. 

Labels:
name: Bug
name: Groupby

Comments:
Hello @smithto1! 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-08-07 14:14:48 UTC
@TomAugspurger I think all the issues are resolved with this PR. Can you do a second review? 
It seems to me that the root issue is `_agg_general` is calling `self._set_group_selection()` rather than using the context manager `with _group_selection_context(self)`. 

Wrapping `_agg_general` in this context manager, the two issues handled by this PR are fixed and only two groupby tests fail. In each of these tests on master, if you comment out the above computations (e.g. `g.sum()`), the result changes. So I think that it is the tests that have an issue.

Update: The two tests that would need to be fixed with this suggestion are already done in this PR. The change to the shift transform test would not be necessary.
> It seems to me that the root issue is `_agg_general` is calling `self._set_group_selection()` rather than using the context manager `with _group_selection_context(self)`.
> 
> Wrapping `_agg_general` in this context manager, the two issues handled by this PR are fixed and only two groupby tests fail. In each of these tests on master, if you comment out the above computations (e.g. `g.sum()`), the result changes. So I think that it is the tests that have an issue.
> 
> Update: The two tests that would need to be fixed with this suggestion are already done in this PR. The change to the shift transform test would not be necessary.

Thanks @rhshadrach , I think your recommendation is better and have gone with that. 

As well as `_agg_general`, I had to make the same update in `_make_wrapper` and `nth` which were also calling `self._set_group_selection`. I've replaced all of these with `with _group_selection_context(self):`. 
@TomAugspurger @jreback 

I've implemented the change recommended by @rhshadrach : https://github.com/pandas-dev/pandas/pull/35314#issuecomment-664193224 

Rather than calling `self._reset_group_selection` inside `.apply`, I've found the three places where we were calling `self._set_group_selection` and replaced those with `with _group_selection_context(self):`. 

So this PR no longer has any impact `.describe` which we were discussing above. `.describe` is unchanged from it current code & behaviour in master/1.0.5. 

The new test that I've written to catch the bug (`test_apply.py/test_apply_is_unchanged_when_other_methods_are_called_first`) is unchanged. Also, the two existing tests that relied on the old buggy behaviour are still updated. 

Could you guys please re-review? 
I'm not going to have a chance to look through this closely today. Given that we're in an RC period and this isn't a regression fix, I think we should target this for 1.1.1.
> I'm not going to have a chance to look through this closely today. Given that we're in an RC period and this isn't a regression fix, I think we should target this for 1.1.1.

sgtm
@jreback I think this one is ready to be merged. 
lgtm. can you merge master again and ping on green (as merged a few other groupby patches recently).
thanks @smithto1 (failure is unrelated)