ENH: Support nested renaming / selection
State: closed

This adds special support for controlling the output column names when performing column-specific groupby aggregations. We use kwargs, using the keywords as the output names, and expecting tuples of `(selection, aggfunc)`.

```python
In [2]: df = pd.DataFrame({'kind': ['cat', 'dog', 'cat', 'dog'],
   ...:                    'height': [9.1, 6.0, 9.5, 34.0],
   ...:                    'weight': [7.9, 7.5, 9.9, 198.0]})
   ...:

In [3]: df
Out[3]:
  kind  height  weight
0  cat     9.1     7.9
1  dog     6.0     7.5
2  cat     9.5     9.9
3  dog    34.0   198.0

In [4]: df.groupby('kind').agg(min_height=('height', 'min'), max_weight=('weight', 'max'))
Out[4]:
      min_height  max_weight
kind
cat          9.1         9.9
dog          6.0       198.0
```

Closes #18366 

---

Putting this up as a WIP before the meeting on Thursday.

TODO:

- [ ] Update old deprecation message to point to new way
- [ ] Decide if we want to allow this elsewhere (DataFrame.agg? SeriesGroupBy.agg?)

cc @jorisvandenbossche @WillAyd 

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

Comments:
Thanks for bringing this back up as this had certainly gone stale. I might not have been as vocal as I should have and may be in the minority but I rather dislike the mapping of dict keys to labels for the following reasons:

 - It makes our API more difficult to manage going forward; any keyword we reasonably think to add to `agg` now has to come with some sort of deprecation cycle for user code that may be using that keyword
 - It limits the range of valid labels that can be used
 - It can have very strange side-effects when conflicting with other keywords (slightly duplicative of my first point, but we see this issue with using `name` in GroupBy ops for example)
 - It seems "unpythonic" to me (this is purely an opinion so not necessarily correct)

I unfortunately have a travel conflict for the meeting Thursday but just providing these points as food for thought. I'll be sure to check the follow ups - thanks again!
> I rather dislike the mapping of dict keys to labels

@WillAyd For clarification: you mean the mapping of "keyword argument names" instead of "dict keys" ? (because it is currently that we use dict keys for it)
That’s right

Sent from my iPhone

> On May 14, 2019, at 10:57 PM, Joris Van den Bossche <notifications@github.com> wrote:
> 
> I rather dislike the mapping of dict keys to labels
> 
> For clarification: you mean the mapping of "keyword argument names" instead of "dict keys" ? (because it is currently that we use dict keys for it)
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.

For some of your points about limitation what you can use: note that you can still pass it as a dict (and then do `**{..}`, although not that nice) if you want to have names that are not valid python identifiers.
Thanks @WillAyd 

> I rather dislike the mapping of dict keys to labels

I agree that it's awkward. In particular, I don't like how idiosyncratic this is; it really is a special mode with its own set of rules. But our set of options (this, nothing, or some other API), I think this is best. I need to read through the discussion again, but do we agree that we need to do something to replace the deprecated feature?

> any keyword we reasonably think to add to agg now has to come with some sort of deprecation cycle for user code that may be using that keyword

I *think* this isn't an issue, or it isn't made any worse by this PR. I believe that in "normal" mode (user passing an aggfunc) pandas promises to pass through kwargs to the aggfunc. So I don't think we'd be able to add keywords to `.agg` for use by pandas without deprecating things anyway.

> It limits the range of valid labels that can be used

I try to document this. You need to use the (ugly) `.agg(**{'not an identifier': ('col', 'sum')})` syntax. This is the same limitation for assign.

> It can have very strange side-effects when conflicting with other keywords

If you have a chance, can you make an example? We kinda have that with `_level`, since we use that internally.

```pytb
In [6]: df.groupby('kind').agg(_level=('height', 'sum'))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-6-45280b886040> in <module>
----> 1 df.groupby('kind').agg(_level=('height', 'sum'))

~/sandbox/pandas/pandas/core/groupby/generic.py in aggregate(self, arg, *args, **kwargs)
   1335     @Appender(_shared_docs['aggregate'])
   1336     def aggregate(self, arg=None, *args, **kwargs):
-> 1337         return super().aggregate(arg, *args, **kwargs)
   1338
   1339     agg = aggregate

~/sandbox/pandas/pandas/core/groupby/generic.py in aggregate(self, func, *args, **kwargs)
    166         elif func is None:
    167             # nicer error message
--> 168             raise TypeError("Must provide 'func' or tuples of "
    169                             "'(column, aggfunc).")
    170

TypeError: Must provide 'func' or tuples of '(column, aggfunc).
```
How do people feel about the tuple `(selection, aggfunc)` convention? Is it too opaque? In the last call we discussed using a dict like `{selection: aggfunc}`, but I disliked that because it would always be a single-item dict, which felt strange.

We could expose a namedtuple, like


```python
In [16]: NamedAgg = namedtuple("NamedAgg", ["column", "aggfunc"])

In [17]: NamedAgg('height', 'sum')
Out[17]: NamedAgg(column='height', aggfunc='sum')
``` 

but I'm not sure where in the pandas namespace that would fit.

---

One other concern: in the issue someone raise an annoyance with multiple lambdas. Currently we raise a SpecificationError

```python
In [23]: df.groupby('kind').agg(a=('height', lambda x: 1), b=('height', lambda x: 2))
---------------------------------------------------------------------------
SpecificationError                        Traceback (most recent call last)
~/sandbox/pandas/pandas/core/base.py in _aggregate(self, arg, *args, **kwargs)
    483                 try:
--> 484                     result = _agg(arg, _agg_1dim)
    485                 except SpecificationError:

~/sandbox/pandas/pandas/core/base.py in _agg(arg, func)
    434                 for fname, agg_how in arg.items():
--> 435                     result[fname] = func(fname, agg_how)
    436                 return result

~/sandbox/pandas/pandas/core/base.py in _agg_1dim(name, how, subset)
    417                                              "in aggregation")
--> 418                 return colg.aggregate(how, _level=(_level or 0) + 1)
    419

~/sandbox/pandas/pandas/core/groupby/generic.py in aggregate(self, func_or_funcs, *args, **kwargs)
    766             ret = self._aggregate_multiple_funcs(func_or_funcs,
--> 767                                                  (_level or 0) + 1)
    768         else:

~/sandbox/pandas/pandas/core/groupby/generic.py in _aggregate_multiple_funcs(self, arg, _level)
    829                     'Function names must be unique, found multiple named '
--> 830                     '{}'.format(name))
    831

SpecificationError: Function names must be unique, found multiple named <lambda>
```

Addressing that will require a bit of work; I've tried to keep this PR small by reusing existing machinery where possible.
If it is between this and still doing nothing I'd vote for the latter without offering anything better. I think this will be pretty buggy and not provide that great of a user experience anyway, so hesitant to start down this path
FYI, this seems somewhat hard to support in Python 3.5, since before 3.6 the order of `**kwargs` was undefined. That means that for 3.5 and earlier we can't reliably know the output column order. I'm in favor of raising a runtime error for 3.5.

---

> If it is between this and still doing nothing I'd vote for the latter without offering anything better.

And how about if I restrict your choices this or nothing? 😄

If you [sort our issues by most +1s](https://github.com/pandas-dev/pandas/issues?q=is%3Aissue+is%3Aopen+sort%3Areactions-%2B1-desc), this is the 3rd-most upvoted issue right now. So I'm inclined to do *something*. Right now this seems like the least-bad something to me :)
> And how about if I restrict your choices this or nothing? 😄

Ha tough one! I'd still say nothing in that case. From my (very biased) workflow I couldn't imagine ever really using this instead of just assigning the columns I want after the fact. (caveat: I don't use pipe / method chaining)
Do we want to keep the *fundamental* discussion on the issue? (so about to do this or not, not the technical details of the PR) 
> Do we want to keep the fundamental discussion on the issue? 

Sure, I'll do a short summary of this PR over there.
It's mainly Will's arguments that are relevant there.

> FYI, this seems somewhat hard to support in Python 3.5, since before 3.6 the order of **kwargs was undefined. That means that for 3.5 and earlier we can't reliably know the output column order. I'm in favor of raising a runtime error for 3.5.

But don't we have the same issue now already with the dict of dicts approach? In those cases we simply sort the keys, so why not here?

I suppose sorting is best for python 3.5.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=h1) Report
> Merging [#26399](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/ff4437eb3b27440ea2304a8613369ebec0b8b72a?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `96.29%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/26399/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #26399      +/-   ##
==========================================
- Coverage   91.69%   91.68%   -0.01%     
==========================================
  Files         174      174              
  Lines       50739    50763      +24     
==========================================
+ Hits        46524    46542      +18     
- Misses       4215     4221       +6
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.19% <96.29%> (ø)` | :arrow_up: |
| #single | `41.15% <14.81%> (-0.19%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/groupby/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9nZW5lcmljLnB5) | `89.18% <96.29%> (+0.21%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `78.94% <0%> (-10.53%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.02% <0%> (-0.12%)` | :arrow_down: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `90.6% <0%> (-0.11%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=footer). Last update [ff4437e...06a86ec](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=h1) Report
> Merging [#26399](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/ff4437eb3b27440ea2304a8613369ebec0b8b72a?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `96.29%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/26399/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #26399      +/-   ##
==========================================
- Coverage   91.69%   91.68%   -0.01%     
==========================================
  Files         174      174              
  Lines       50739    50763      +24     
==========================================
+ Hits        46524    46542      +18     
- Misses       4215     4221       +6
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.19% <96.29%> (ø)` | :arrow_up: |
| #single | `41.15% <14.81%> (-0.19%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/groupby/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9nZW5lcmljLnB5) | `89.18% <96.29%> (+0.21%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `78.94% <0%> (-10.53%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.02% <0%> (-0.12%)` | :arrow_down: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `90.6% <0%> (-0.11%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=footer). Last update [ff4437e...06a86ec](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=h1) Report
> Merging [#26399](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/a91da0c94e541217865cdf52b9f6ea694f0493d3?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `97.14%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/26399/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #26399      +/-   ##
==========================================
- Coverage   91.77%   91.76%   -0.01%     
==========================================
  Files         174      174              
  Lines       50649    50677      +28     
==========================================
+ Hits        46483    46505      +22     
- Misses       4166     4172       +6
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.3% <97.14%> (ø)` | :arrow_up: |
| #single | `41.66% <31.42%> (-0.09%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/groupby/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9fX2luaXRfXy5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [pandas/core/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYmFzZS5weQ==) | `98.19% <100%> (ø)` | :arrow_up: |
| [pandas/core/api.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXBpLnB5) | `100% <100%> (ø)` | :arrow_up: |
| [pandas/core/groupby/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS9nZW5lcmljLnB5) | `89.22% <96.77%> (+0.26%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `78.94% <0%> (-10.53%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97% <0%> (-0.12%)` | :arrow_down: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/26399/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `90.6% <0%> (-0.11%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=footer). Last update [a91da0c...25dca1a](https://codecov.io/gh/pandas-dev/pandas/pull/26399?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@TomAugspurger : thanks for keeping this alive, but I think 'ENH' is underselling it, it was essentially an outright BUG ([#18366](https://github.com/pandas-dev/pandas/issues/18366#issuecomment-492784947)) to deprecate existing behavior in `groupby.agg` that users were relying on. (It also made us underperform a useful feature of R dplyr/tidyverse(/plyr) aggregate/summarize/mutate, which have essentially implemented this for 10 years). Should not break core pandas functionality.

@WillAyd : can you please be more specific and quantifiable about your objections? Yes, dict-based-renaming limits to valid ASCII(/Unicode?) names. If a user wants an arbitrary name, they can either name-mangle, or just use a different method than dict-based-renaming. But for 99+% of users this functionality will work out-of-the-box. Why compromise the API for the sake of 0.01%(?) of users, or whatever? Can you actually ballpark-estimate what % of pandas user code has non-compliant names?

Dict-based-renaming names moreover cannot be existing kwargs of that command; so yeah we need a Warning which checks name-collision of the dict with kwargs, the warning should default to on, and be  disableable for speed. (That's not too hard, right?). Obviously we should strive for kwarg naming consistency across all commands that would support dict-based-renaming to minimize such collisions. That addresses all your objections, right?

> * It makes our API more difficult to manage going forward; any keyword we reasonably think to add to `agg` now has to come with some sort of deprecation cycle for user code that may be using that keyword

Yes, that's just life in API-land. This one has huge tangible benefits, it's worth the price.

> * It seems "unpythonic" to me (this is purely an opinion so not necessarily correct)

I personally disagree, anyway that's subjective. And R supports this sort of thing for 10+ years; this behavior is expected and achievable.
> I don't think you should sort for 3.5, though do we for .assign?

Yeah. I think the consistency with `.assign` argues for sorting, rather than raising.
> Yeah. I think the consistency with .assign argues for sorting, rather than raising.

We have always been sorting, not only in assign (also in the dataframe constructor, in the current groupby.agg with dictionaries, ..), so I don't see why not here.
I have a solution for the multiple-lambda stuff. It's already +60 LOC, with no tests, so I opened https://github.com/pandas-dev/pandas/issues/26430 and will do that independently. Then things will work out here.
I added the `pd.Agg` helper namedtupele. Are people OK with the name of the object? Do we prefer NamedAgg or KeywordAgg? Are the field names (column, aggfunc) OK?
I'd prefer `KeywordAgg` so it's purpose is a little more explicit. 
For consistency, we should probably implement DataFrame.agg as well. Would it be easier to review that in a separate PR?
Also, not sure if this is done somewhere else, but the deprecations added in https://github.com/pandas-dev/pandas/pull/15931/files should probably be updated to point users towards the new system.
Can we try to think of a better term? 
I don't directly have a better suggestion though, but I find "keyword aggregation" not very clear unless if you already know exactly what it is (it is also not the keyword that gets aggregated).
I haven't come up with anything better than  "keyword aggregation" or "named aggregation".
> I haven't come up with anything better than "keyword aggregation" or "named aggregation".

Ah, I actually like "named aggregation" more (it clarifies more the goal, instead of mentioning the technical means) 

I see now in the discussion above that you actually originally proposed NamedAgg, before Agg, and then @mroeschke preferred KeywordAgg.


> I see now in the discussion above that you actually originally proposed NamedAgg, before Agg, and then @mroeschke preferred KeywordAgg.

Yeah, that was my initial preference between the two, but it's not strong. NamedAgg / "named aggregation" emphasizes the "what"l more, KeywordAgg / "keyword aggregation" emphasizes the "how" more. @mroeschke did you have a strong reason for KeywordAgg?
Opened up a couple issues for SeriesGroupBy.agg and NDFrame.agg. I'll do the last round of doc fixups here once we decide on KeywordAgg vs. NamedAgg, (also gives conda a chance to fix itself).
i think NamedAgg might be more descriptive here