-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: DataFrame.agg and apply with 'size' returns a scalar #39935
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
Changes from all commits
8e1c0aa
8bbc1b9
1de58f5
4bb5d86
a8a0b99
824316b
042c68b
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 |
---|---|---|
|
@@ -159,6 +159,10 @@ def f(x): | |
def index(self) -> Index: | ||
return self.obj.index | ||
|
||
@property | ||
def agg_axis(self) -> Index: | ||
return self.obj._get_agg_axis(self.axis) | ||
|
||
@abc.abstractmethod | ||
def apply(self) -> FrameOrSeriesUnion: | ||
pass | ||
|
@@ -541,17 +545,26 @@ def maybe_apply_str(self) -> Optional[FrameOrSeriesUnion]: | |
f = self.f | ||
if not isinstance(f, str): | ||
return None | ||
|
||
obj = self.obj | ||
|
||
# TODO: GH 39993 - Avoid special-casing by replacing with lambda | ||
if f == "size" and isinstance(obj, ABCDataFrame): | ||
# Special-cased because DataFrame.size returns a single scalar | ||
value = obj.shape[self.axis] | ||
return obj._constructor_sliced(value, index=self.agg_axis, name="size") | ||
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 it intentional to set the name here? This isn't consistent with other string agg methods 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. Indeed, thanks @TheNeuralBit! Opened #42046 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. Thank you! |
||
|
||
# Support for `frame.transform('method')` | ||
# Some methods (shift, etc.) require the axis argument, others | ||
# don't, so inspect and insert if necessary. | ||
func = getattr(self.obj, f, None) | ||
func = getattr(obj, f, None) | ||
if callable(func): | ||
sig = inspect.getfullargspec(func) | ||
if "axis" in sig.args: | ||
self.kwargs["axis"] = self.axis | ||
elif self.axis != 0: | ||
raise ValueError(f"Operation {f} does not support axis=1") | ||
return self.obj._try_aggregate_string_function(f, *self.args, **self.kwargs) | ||
return obj._try_aggregate_string_function(f, *self.args, **self.kwargs) | ||
|
||
def maybe_apply_multiple(self) -> Optional[FrameOrSeriesUnion]: | ||
""" | ||
|
@@ -613,10 +626,6 @@ def values(self): | |
def dtypes(self) -> Series: | ||
return self.obj.dtypes | ||
|
||
@property | ||
def agg_axis(self) -> Index: | ||
return self.obj._get_agg_axis(self.axis) | ||
|
||
def apply(self) -> FrameOrSeriesUnion: | ||
""" compute the results """ | ||
# dispatch to agg | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't this affect Series as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a Series, size gives the number of rows and so the default path calling
obj.size
produces the correct result.