ENH: DataFrameGroupBy.transform now accepts list, dict, and NamedAgg arguments (GH#58318) - #65164
Conversation
…arguments (GH#58318)
|
Thanks for working on this, will take a look. If I don't update in a week, please send me a ping! |
|
@rhshadrach - Let me know if anything needs to be updated! |
rhshadrach
left a comment
There was a problem hiding this comment.
Looks very good, some comments. Regarding using NamedAgg, we'll need to make a decision here as to the naming since this isn't great when used with e.g. cumsum. The implementation doesn't limit it to aggregation functions already. I'd support adding an alias NamedFunc and the (possibly) deprecating NamedAgg.
| raise NotImplementedError( | ||
| "Passing a list to SeriesGroupBy.transform is not yet supported " | ||
| "and is intended to be implemented in a future release. " | ||
| "See GH#58318." |
There was a problem hiding this comment.
For references within the pandas code this would be fine, but for user-facing I think we should have the full URL.
| 1 1 0 1 0 | ||
| 2 2 2 2 2 | ||
|
|
||
| .. versionchanged:: 3.0.0 |
| if func is None: | ||
| # Named-aggregation style: | ||
| # .transform(val_sum=NamedAgg(column="val", aggfunc="sum"), ...) | ||
| transformed_func: dict = dict(kwargs.items()) |
| func, *args, engine=engine, engine_kwargs=engine_kwargs, **kwargs | ||
| ) | ||
| else: | ||
| # Original single-function path; unchanged. |
There was a problem hiding this comment.
This comment will be not meaningful when this PR is merged; can you remove.
| return self._transform_multiple_funcs( | ||
| transformed_func, *args, engine=engine, engine_kwargs=engine_kwargs | ||
| ) | ||
| elif isinstance(func, dict): |
There was a problem hiding this comment.
This is False for e.g. UserDict. Can you use is_dict_like.
| return self._transform_multiple_funcs( | ||
| func, *args, engine=engine, engine_kwargs=engine_kwargs, **kwargs | ||
| ) | ||
| elif isinstance(func, list): |
| """ | ||
| from pandas.core.reshape.concat import concat | ||
|
|
||
| if isinstance(func, dict): |
| # ── list path ──────────────────────────────────────────────────────── | ||
| # Apply every func to every non-key column. | ||
| # _obj_with_exclusions already omits groupby keys and excluded columns. | ||
| assert isinstance(func, list) |
| if (self._obj_with_exclusions.columns == column_name).sum() > 1: | ||
| raise ValueError( | ||
| f"Column label '{column_name}' is not unique in this DataFrame. " | ||
| "DataFrameGroupBy.transform with a dict or NamedAgg does not " | ||
| "support duplicate column names. See GH#58318." | ||
| ) |
There was a problem hiding this comment.
Why do we need to impose this restriction?
|
@rhshadrach - addressed all the code comments. Thank you! Added pd.NamedFunc as an alias for pd.NamedAgg per reviewer suggestion. Deprecation of NamedAgg is left as a separate follow-up. |
Fine by me, no strong opinion. |
rhshadrach
left a comment
There was a problem hiding this comment.
Looking good! Just some minor style requests.
| 1 1 0 1 0 | ||
| 2 2 2 2 2 | ||
|
|
||
| .. versionchanged:: 3.0.1 |
There was a problem hiding this comment.
Instead of these versionchanged throughout the example, can you do a single one after the func argument description.
| ``output_name=NamedFunc(column, func)`` for named transformation or | ||
| ``output_name=NamedAgg(column, aggfunc)`` for named aggregation. |
There was a problem hiding this comment.
This makes it seem like you have to use NamedAgg if you have an aggregation function, which is not the case. I'd suggest just leaving NamedAgg off here.
| ``output_name=NamedFunc(column, func)`` for named transformation or | |
| ``output_name=NamedAgg(column, aggfunc)`` for named aggregation. | |
| ``output_name=NamedFunc(column, func)``. |
| from pandas.core.reshape.concat import concat | ||
|
|
||
| if is_dict_like(func): | ||
| # ── dict / NamedAgg path ───────────────────────────────────────── |
There was a problem hiding this comment.
| # ── dict / NamedAgg path ───────────────────────────────────────── | |
| # Also includes NamedAgg / NamedFunc |
| results.append(result) | ||
| return concat(results, axis=1) | ||
|
|
||
| # ── list path ──────────────────────────────────────────────────────── |
There was a problem hiding this comment.
| # ── list path ──────────────────────────────────────────────────────── | |
| # list path |
|
|
||
| # ── list path ──────────────────────────────────────────────────────── | ||
| # Apply every func to every non-key column. | ||
| # _obj_with_exclusions already omits groupby keys and excluded columns. |
There was a problem hiding this comment.
| # _obj_with_exclusions already omits groupby keys and excluded columns. |
I don't think this comment is adding anything.
|
Made the changes. |
rhshadrach
left a comment
There was a problem hiding this comment.
I think we should just refer to NamedFunc when it comes to transform; also can you add one more note in the whatsnew for NamedFunc. Something along the lines of:
- Added :class:`NamedFunc`, an alias to :class:`NamedAgg` for a more general name; either can accept arbitrary functions (:issue:`65164`)
| - Numba JIT function with ``engine='numba'`` specified. | ||
| - List of strings/functions: applied to every non-key column, | ||
| returning a MultiIndex-column DataFrame ``(column, func)``. | ||
| - Dict ``{column: func}`` or ``{name: NamedAgg(column, func)}``: |
| applied per-column as specified. | ||
|
|
||
| .. versionchanged:: 3.1.0 | ||
| Added support for list-like, dict, and :class:`NamedAgg` arguments. |
| ^^^^^^^^^^^^^^^^^^ | ||
| - :class:`Period` now supports f-string formatting via ``__format__``, e.g. ``f"{period:%Y-%m}"`` (:issue:`48536`) | ||
| - :meth:`.DataFrameGroupBy.agg` now allows for the provided ``func`` to return a NumPy array (:issue:`63957`) | ||
| - :meth:`DataFrameGroupBy.transform` now accepts list-like and dict arguments similar to :meth:`GroupBy.agg`, and supports :class:`NamedAgg` (:issue:`58318`) |
| "MultiIndex", | ||
| "NaT", | ||
| "NamedAgg", | ||
| "NamedFunc", |
There was a problem hiding this comment.
This should also be imported in pandas/__init__.py. There will likely be some API tests that break; those tests just then need updated with what to expect.
There was a problem hiding this comment.
Done. Minimal updates needed in API tests.
|
I don’t have bandwidth to do a proper review. No objections based on a quick glance. Happy to defer to rhshadrach |
|
Thanks @berasaikat |
…arguments (GH#58318) (pandas-dev#65164)
Description
Closes #58318
What this does
GroupBy.transformcurrently only accepts a single callable or string, whileGroupBy.aggsupports list-like, dict, andNamedAggkeyword arguments. This PR closes that gap by extendingDataFrameGroupBy.transformto support the same call signatures asGroupBy.agg.Changes
pandas/core/groupby/generic.pyfuncis now optional (defaults toNone) to support theNamedAggkwargs pathMultiIndex-column DataFrame(column, func){"col": "sum"}and{"name": NamedAgg(column, func)}are both supported, as are plain tuples as aNamedAggequivalentNamedAgg/ plain tuple kwargs dispatch: named output columns with explicit source column selection_transform_multiple_funcs(orchestrates list and dict paths) and_transform_single_column(applies one function to one column viaSeriesGroupBy)NotImplementedErrorwith a clear message forSeriesGroupBy.transform(list)and dict-of-lists — both are intended for a future releaseValueErrorfor duplicate column names in dict/NamedAggpathspandas/tests/groupby/transform/test_transform.pytest_transform_with_list_liketest_transform_with_list_like_single_columntest_transform_with_dicttest_transform_with_dict_subset_columnstest_transform_with_dict_of_lists_raisestest_transform_with_namedaggtest_transform_with_namedagg_same_source_columntest_transform_with_namedagg_plain_tupletest_transform_series_groupby_list_raisestest_transform_dict_duplicate_column_names_raisesdoc/source/whatsnew/v3.1.0.rstChecklist
doc/source/whatsnew/v3.1.0.rst