Skip to content

BUG: SeriesGroupBy.apply sets name attribute if result is DataFrame #49908

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 27, 2022
Merged

BUG: SeriesGroupBy.apply sets name attribute if result is DataFrame #49908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c863f89
dont set name if dataframe
Nov 25, 2022
191b4cb
use ndim in check, add gh reference to test
Nov 25, 2022
b191f50
Merge remote-tracking branch 'upstream/main' into grp-apply-sets-name
Nov 25, 2022
962d02a
back to isinstance
Nov 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.5.3.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Fixed regressions
- Fixed performance regression in :meth:`Series.isin` when ``values`` is empty (:issue:`49839`)
- Fixed regression in :meth:`DataFrameGroupBy.transform` when used with ``as_index=False`` (:issue:`49834`)
- Enforced reversion of ``color`` as an alias for ``c`` and ``size`` as an alias for ``s`` in function :meth:`DataFrame.plot.scatter` (:issue:`49732`)
- Fixed regression in :meth:`SeriesGroupBy.apply` setting a ``name`` attribute on the result if the result was a :class:`DataFrame` (:issue:`49907`)
-

.. ---------------------------------------------------------------------------
Expand Down
3 changes: 2 additions & 1 deletion pandas/core/groupby/generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,8 @@ def _wrap_applied_output(
not_indexed_same=not_indexed_same,
is_transform=is_transform,
)
result.name = self.obj.name
if isinstance(result, Series):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use result.ndim == 1 here instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, will do - though just for my understanding, why?

Copy link
Member

@rhshadrach rhshadrach Nov 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting - it used to be that isinstance checks were quite a bit more expensive than checking the ndim attribute; I'm now seeing isinstance is slightly cheaper. With type checkers able to naturally derive type information from isinstance checks in code paths, we should certainly prefer isinstance if they are just as performant.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarcoGorelli - I'm good with either using ndim or isinstance here; I plan on taking a more in depth look of our usage. Let me know what you think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no preference from my end

result.name = self.obj.name
return result
else:
# GH #6265 #24880
Expand Down
1 change: 1 addition & 0 deletions pandas/tests/groupby/test_apply.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ def f(piece):
result = grouped.apply(f)

assert isinstance(result, DataFrame)
assert not hasattr(result, "name") # GH49907
tm.assert_index_equal(result.index, ts.index)


Expand Down