-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
CLN: aggregation.transform #36478
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLN: aggregation.transform #36478
Changes from all commits
72512b7
061f9c6
409fda2
2b2547f
431488c
9e77105
3e72b80
506e804
d0193c9
e53b389
ea2ad3d
e5a7f92
744f1fb
7066f80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,9 +17,17 @@ | |
Sequence, | ||
Tuple, | ||
Union, | ||
cast, | ||
) | ||
|
||
from pandas._typing import AggFuncType, Axis, FrameOrSeries, Label | ||
from pandas._typing import ( | ||
AggFuncType, | ||
AggFuncTypeBase, | ||
Axis, | ||
FrameOrSeries, | ||
FrameOrSeriesUnion, | ||
Label, | ||
) | ||
|
||
from pandas.core.dtypes.common import is_dict_like, is_list_like | ||
from pandas.core.dtypes.generic import ABCDataFrame, ABCSeries | ||
|
@@ -391,7 +399,7 @@ def validate_func_kwargs( | |
|
||
def transform( | ||
obj: FrameOrSeries, func: AggFuncType, axis: Axis, *args, **kwargs | ||
) -> FrameOrSeries: | ||
) -> FrameOrSeriesUnion: | ||
""" | ||
Transform a DataFrame or Series | ||
|
||
|
@@ -424,16 +432,20 @@ def transform( | |
assert not is_series | ||
return transform(obj.T, func, 0, *args, **kwargs).T | ||
|
||
if isinstance(func, list): | ||
if is_list_like(func) and not is_dict_like(func): | ||
func = cast(List[AggFuncTypeBase], func) | ||
# Convert func equivalent dict | ||
if is_series: | ||
func = {com.get_callable_name(v) or v: v for v in func} | ||
else: | ||
func = {col: func for col in obj} | ||
|
||
if isinstance(func, dict): | ||
if is_dict_like(func): | ||
jreback marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func = cast(Dict[Label, Union[AggFuncTypeBase, List[AggFuncTypeBase]]], func) | ||
return transform_dict_like(obj, func, *args, **kwargs) | ||
|
||
# func is either str or callable | ||
func = cast(AggFuncTypeBase, func) | ||
try: | ||
result = transform_str_or_callable(obj, func, *args, **kwargs) | ||
except Exception: | ||
|
@@ -451,37 +463,52 @@ def transform( | |
return result | ||
|
||
|
||
def transform_dict_like(obj, func, *args, **kwargs): | ||
def transform_dict_like( | ||
obj: FrameOrSeries, | ||
func: Dict[Label, Union[AggFuncTypeBase, List[AggFuncTypeBase]]], | ||
*args, | ||
**kwargs, | ||
): | ||
""" | ||
Compute transform in the case of a dict-like func | ||
""" | ||
from pandas.core.reshape.concat import concat | ||
|
||
if len(func) == 0: | ||
raise ValueError("No transform functions were provided") | ||
|
||
if obj.ndim != 1: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe extract method |
||
# Check for missing columns on a frame | ||
cols = sorted(set(func.keys()) - set(obj.columns)) | ||
if len(cols) > 0: | ||
raise SpecificationError(f"Column(s) {cols} do not exist") | ||
|
||
if any(isinstance(v, dict) for v in func.values()): | ||
# Can't use func.values(); wouldn't work for a Series | ||
if any(is_dict_like(v) for _, v in func.items()): | ||
# GH 15931 - deprecation of renaming keys | ||
raise SpecificationError("nested renamer is not supported") | ||
|
||
results = {} | ||
results: Dict[Label, FrameOrSeriesUnion] = {} | ||
for name, how in func.items(): | ||
colg = obj._gotitem(name, ndim=1) | ||
try: | ||
results[name] = transform(colg, how, 0, *args, **kwargs) | ||
except Exception as e: | ||
if str(e) == "Function did not transform": | ||
raise e | ||
except Exception as err: | ||
if ( | ||
str(err) == "Function did not transform" | ||
or str(err) == "No transform functions were provided" | ||
): | ||
raise err | ||
|
||
# combine results | ||
if len(results) == 0: | ||
raise ValueError("Transform function failed") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this tested? is this new? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think you should just let concat work and bubble up if needed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not new, this is hit in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm yeah I think we should change this, maybe to something a bit more userfriendly like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you thinking of the case where an aggregator is used? If we detect the function not transforming, we'll raise here:
before we get to this point. The raising of the code highlighted above should only occur if all the key/value pairs of the dictionary entirely failed - e.g. trying to take a product of strings or There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, no this is the case of an empty list or dict right? (of functions that are supplied for aggregation), IOW its user visible There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see! I did not realize this line would be the one that raised for an empty list or dict. I've added that check before getting to this section of the code with the message "no results" (this is the message from 1.1 - but can change), along with tests for this. |
||
return concat(results, axis=1) | ||
|
||
|
||
def transform_str_or_callable(obj, func, *args, **kwargs): | ||
def transform_str_or_callable( | ||
obj: FrameOrSeries, func: AggFuncTypeBase, *args, **kwargs | ||
) -> FrameOrSeriesUnion: | ||
""" | ||
Compute transform in the case of a string or callable func | ||
""" | ||
|
Uh oh!
There was an error while loading. Please reload this page.