-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CLN: Refactor groupby._make_wrapper #48400
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
) | ||
|
||
|
||
def test_frame_consistency(request, groupby_func): |
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.
Side note: Doing something like this for the entire API would be really cool. It would help the ensure when adding/modifying/deprecating methods & their signatures that consistency remains.
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.
Overall LGTM. Could use another set of eyes too, but happy to merge if there's no other comments.
Awesome! A lot more straightforward, thanks @rhshadrach |
* CLN: Refactor groupby._make_wrapper * Remove type: ignore * Revert behavior change for corrwith; fix skew and mad * Add docstring
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.Currently keeping the docs the same, plan to make groupby-specific docstrings in the future. Note that most of the SeriesGroupBy docs don't make it into the documentation, I've opened #48399. The only change from a user-perspective here should be that Jupyter / IDEs can now get the correct signature (instead of
*args, **kwargs
).