Skip to content
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

DEPR: deprecate obj argument in GroupBy.get_group #53571

Merged

Conversation

natmokval
Copy link
Contributor

@natmokval natmokval commented Jun 8, 2023

obj argument was deprecated in GroupBy.get_group and removed from the parameters in GroupBy.get_group docs.

@natmokval natmokval marked this pull request as ready for review June 9, 2023 09:18
@natmokval
Copy link
Contributor Author

@rhshadrach, could you please take a look at this pr?


Returns
-------
same type as obj
DataFrame
Copy link
Member

Choose a reason for hiding this comment

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

Small issue: I think that it returns Series or DataFrame

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I rolled back my changes there completely.

@mroeschke mroeschke added the Deprecate Functionality to remove in pandas label Jun 9, 2023
@rhshadrach
Copy link
Member

Thanks for the PR! This is indeed how the code should look after the deprecation is enforced. However we need to first introduce the deprecation so as to not break any user code but rather warn them about the change. If you're unfamiliar with this, I would suggest looking at merged PRs with the "Deprecate" label to get a better sense as to what this entails.

@natmokval
Copy link
Contributor Author

natmokval commented Jun 11, 2023

Thank you, @rhshadrach for the comment. Sorry, my fault, I corrected my pr. Could you please take a look at update.

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Looks good; could you add a test for the warning? I'd recommend placing it in tests/groupby/test_groupby.py.

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

lgtm

@rhshadrach rhshadrach added this to the 2.1 milestone Jun 19, 2023
@rhshadrach rhshadrach merged commit 641427e into pandas-dev:main Jun 19, 2023
36 of 38 checks passed
@natmokval
Copy link
Contributor Author

Thank you @rhshadrach for reviewing this. Sorry for the delay with adding the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas Groupby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DEPR: obj argument in GroupBy.get_group
4 participants