ENH: Allow the groupby by param to handle columns and index levels (GH5677)
State: closed

- [x] closes #5677
- [x] tests added
- [x] rebase after #14342 is merged to fix #14334
- [x] tests pass (requires rebase above)
- [x] passes `git diff upstream/master | flake8 --diff`
- [x] whatsnew entry
- [x] updates existing documentation
- [x] add example to whatsnew 0.20 and groupby.rst

Change to allow strings passed as the `by` parameter to `df.groupby` to reference columns (existing behavior) or index level names if no column match is found.  Columns take precedence in the case of ambiguity to maintain backward compatibility.


Labels:
name: Groupby
name: API Design

Comments:
@I think that we should raise in the ambiguous case, which just works on the column now (which this PR uses to establish precedence), e.g.

`df.groupby('inner')` when inner is both a column and a level name.

This is almost always an error by the user, or if its actually wanted, then the user should be more specific (by using `pd.Grouper`).

Currently, AFIK, this just takes the column.

@jreback I have no problem with raising an exception in the ambiguous case.  As you noted, the only reason to have columns take precedence was to reproduce the behavior of previous versions if ambiguity was present.

@shoyer expressed a preference for the precedence approach in https://github.com/pandas-dev/pandas/issues/14355#issuecomment-252666473 for the `df.merge` operation.  And I do agree that it makes sense to handle ambiguity in a consistent way across #5677 (`groupby`), #14353 (`sort_values`), and #14355 (`merge`).

@TomAugspurger @jorisvandenbossche @shoyer Do any of you object to raising an exception in the ambiguous case for each of these 3 enhancements?

Yes, we could error in the ambiguous case, but only eventually, after a deprecation cycle.

@shoyer
If we went the deprecation cycle route, would that mean that we'd keep the column precedence behavior but add a `FutureWarning` in the ambiguous case?  Something like the following?

``` .python
warnings.warn(("'%s' is both a column name and an index level, defaulting to column."
               "This will raise an ambiguity error in a future version") % grp,
               FutureWarning, stacklevel=2)
```

And then I assume we'd also add a deprecation note to the whatsnew file.

@jreback @jorisvandenbossche Thoughts?

@jmmease Yes, that's right. We should probably be a little reluctant to add deprecated behavior right now, though, because there may be a long wait between the next pandas feature release (1.0?) and pandas 2.0.

there is no problem with deprecating things
w certainly are not going to just 'wait' for pandas 2.0 

@jreback @shoyer @jorisvandenbossche 
Rebased now that #14342 has landed.  Added `FutureWarning` for the case where a string matches both a column and an index level.

Example for whatsnew, groupby.rst, and `groupby` doc-string still to come

## [Current coverage](https://codecov.io/gh/pandas-dev/pandas/pull/14432?src=pr) is 85.30% (diff: 100%)
> Merging [#14432](https://codecov.io/gh/pandas-dev/pandas/pull/14432?src=pr) into [master](https://codecov.io/gh/pandas-dev/pandas/branch/master?src=pr) will increase coverage by **0.01%**

```diff
@@             master     #14432   diff @@
==========================================
  Files           140        144     +4   
  Lines         50719      51004   +285   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43259      43510   +251   
- Misses         7460       7494    +34   
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [dca0185...2d35461](https://codecov.io/gh/pandas-dev/pandas/compare/dca0185388d7dce3b1f9e39955c209de1184836a...2d354612e9bce14367e918fa3a962e0e01aaf5de?src=pr)
@jmmease Thanks a lot!
@jmmease can you do a quick followup to catch this warning that's appearing (you may need to run all groupby tests)

```
test_groupby_multi_categorical_as_index (pandas.tests.test_groupby.TestGroupBy) ... /Users/jreback/pandas/pandas/core/groupby.py:370: FutureWarning: 'cat' is both a column name and an index level.
Defaulting to column but this will raise an ambiguity error in a future version
  mutated=self.mutated)
```
Sure. See #14902
So what's the next step of the deprecation cycle? What shall I do if I want to enforce the column to be used in grouping?
@goldenbull this will be changed to an error in 1.0 (after 0.21)