ENH: Implement Keyword Aggregation for DataFrame.agg and Series.agg #29116
Conversation
pandas/core/groupby/helper.py
Outdated
| @@ -0,0 +1,193 @@ | |||
| from collections import OrderedDict | |||
There was a problem hiding this comment.
more informative name than "helper"?
pandas/core/frame.py
Outdated
| func, indexes, order = _normalize_keyword_aggregation(kwargs) | ||
| reordered_indexes = [ | ||
| pair[0] for pair in sorted(zip(indexes, order), key=lambda t: t[1]) | ||
| ] |
There was a problem hiding this comment.
most of this is already implemented in the SelectionMixin
you should not have to add much code
There was a problem hiding this comment.
emm, i think this implementation is sort of post-processing after aggregation from SelectionMixin? no?
i will try my best to shorten the code anyway! thanks! @jreback
charlesdong1991
changed the title
charlesdong1991
changed the title
pandas/core/aggregation.py
Outdated
| Union[ | ||
| Union[Callable, str], | ||
| List[Union[Callable, str]], | ||
| Dict[Label, Union[Union[Callable, str], List[Union[Callable, str]]]], |
There was a problem hiding this comment.
| Dict[Label, Union[Union[Callable, str], List[Union[Callable, str]]]], | |
| Dict[Label, Union[Callable, str], List[Union[Callable, str]]], |
If reading correctly I think this is superfluous
There was a problem hiding this comment.
yeah, it's indeed superfluous, i changed to Dict[Label, Union[Callable, str, List[Union[Callable, str]]]
pandas/core/aggregation.py
Outdated
| func: Optional[ | ||
| Union[ | ||
| Union[Callable, str], | ||
| List[Union[Callable, str]], |
There was a problem hiding this comment.
crazy signatures, can you create an alias to make this easier to read, maybe some of this is Label?
pandas/core/aggregation.py
Outdated
| @@ -198,6 +293,79 @@ def maybe_mangle_lambdas(agg_spec: Any) -> Any: | |||
| return mangled_aggspec | |||
|
|
|||
|
|
|||
| def _relabel_result( | |||
There was a problem hiding this comment.
can you make this public (no _) as we are calling from a related module (its not exposed to users at all, just easier to grok)
|
oh, sorry, i am working on the annotations, just haven't pushed the commit @jreback will ping you for review once i have simplified annotations! |
|
emm, the errors look unrelated to this PR, I will rebase once there is new commits merged to master. In the meantime, maybe pls take a look at the annotations so that i could make adjustments if needed? @jreback |
|
ehh?? looks good again somehow? 😅 i thought i saw errors back then sorry for the noise |
| import pandas.core.common as com | ||
| from pandas.core.indexes.api import Index | ||
| from pandas.core.series import FrameOrSeriesUnion, Series | ||
|
|
||
| # types of `func` kwarg for DataFrame.aggregate and Series.aggregate |
There was a problem hiding this comment.
great, we may want to think about putting these in _typing, but separate issue / PR (you can just try it if you want), but in a follown.
There was a problem hiding this comment.
ahh, okay! i deliberately moved it from _typing to here in commit 05921af since i thought this might be only used in this file.
I will do a PR for moving it to _typing.
|
thanks @charlesdong1991 very nice! |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffThis PR basically deals with three issue: