-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Groupby agg works with pd.Series.nunique, but groupby nunique fails with axis=1 #30311
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
Conversation
Thanks to Tom for the tip Co-Authored-By: Tom Augspurger <[email protected]>
I still need to merge with master for CI issues
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.
Thanks for the PR!
PR has been updated accordingly to your remarks |
Great thanks @nlepleux ! |
@@ -1813,9 +1813,20 @@ def groupby_series(obj, col=None): | |||
# Try to consolidate with normal wrapping functions | |||
from pandas.core.reshape.concat import concat | |||
|
|||
results = [groupby_series(content, label) for label, content in obj.items()] | |||
axis_number = obj._get_axis_number(self.axis) |
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.
@WillAyd this is adding quite a bit of very hard and bespoke code
pls don’t merge non trivial things like this
this is also likely non performant
pls revert
What performance are you looking for from the axis=1 case? Wonder if time is better spent optimizing that than reverting
…Sent from my iPhone
On Dec 18, 2019, at 4:17 PM, Jeff Reback ***@***.***> wrote:
@jreback commented on this pull request.
In pandas/core/groupby/generic.py:
> @@ -1813,9 +1813,20 @@ def groupby_series(obj, col=None):
# Try to consolidate with normal wrapping functions
from pandas.core.reshape.concat import concat
- results = [groupby_series(content, label) for label, content in obj.items()]
+ axis_number = obj._get_axis_number(self.axis)
@WillAyd this is adding quite a bit of very hard and bespoke code
pls don’t merge non trivial things like this
this is also likely non performant
pls revert
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
i am way more concerned about the code smell this introduces |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff