-
-
Notifications
You must be signed in to change notification settings - Fork 19.4k
CLN: Simplify gathering of results in aggregate #37227
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
jreback
left a comment
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.
wow a lot simpler. pls merge master and ping on green.
pandas/core/aggregation.py
Outdated
|
|
||
| # we have a dict of Series | ||
| # return a MI Series | ||
| if any(isinstance(r, NDFrame) for r in results.values()): |
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.
can you add a ABCNDFrame in pandas.core.dtypes.generic
pandas/core/aggregation.py
Outdated
| keys_to_use = keys_to_use if keys_to_use != [] else keys | ||
| axis = 0 if isinstance(obj, ABCSeries) else 1 | ||
| result = concat({k: results[k] for k in keys_to_use}, axis=axis) | ||
| # Raised if some value of results is not a NDFrame |
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.
can you move this comment below the AttributeError (as confusing where it is now)
pandas/core/aggregation.py
Outdated
| try: | ||
| result = concat(results) | ||
| except TypeError as err: | ||
| keys_to_use = [k for k in keys if not results[k].empty] |
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.
where does the AttributeError come from?
can you move things outside the try/except (or use an else)?
|
lgtm merge on green |
|
thanks @rhshadrach |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffReduces the number of paths when collecting results in
aggregation.aggregate; one where we are gathering NDFrames usingconcatand the other where we are gathering scalars usingSeries.The reason for the one test change is as follows. In one case, we previously gathered results using
DataFrame. When this occurs and the indexes are not all equal,DataFramewill sort the index whereasconcatwill have the index in order of appearance. For example withgives
whereas using
concatinstead ofDataFrameon the first line withaxis=1gives:If in this example you replace the 2nd index with ['b', 'a'] (so that they are equal), then both
Dataframeandconcatwill produce the same result with index['b', 'a']. If on the other hand you replace the 2nd index with['a', 'b'], thenDataFramewill result in index['a', 'b']whereas concat will result in index['b', 'a'].