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.

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

Jump to bottom

DEPR: group by one element list gets scalar keys #59179

Merged
merged 9 commits into from
Jul 23, 2024
Merged

DEPR: group by one element list gets scalar keys #59179

merged 9 commits into from
Jul 23, 2024
+50 −18

Conversation

natmokval
Copy link
Contributor

@natmokval natmokval commented Jul 4, 2024 •
edited
Loading

changed the behavior: group keys to be 1-tuples instead of scalar when grouping with a length-1 list

@natmokval natmokval added Groupby API - Consistency Internal Consistency of API/Behavior labels Jul 14, 2024
@natmokval
Copy link
Contributor Author

natmokval commented Jul 14, 2024 •
edited
Loading

I deprecated the behavior for groups: group keys are now 1-tuples instead of scalars when grouping with a length-1 list.
I fixed the tests except for this one test_groupby_raises_category_on_category in pandas/tests/groupby/test_raises.py. I got errors like:

FAILED pandas/tests/groupby/test_raises.py::test_groupby_raises_category_on_category[by3-False-idxmin-False-agg] - ValueError: Can't get idxmin of an empty group due to unobserved categories. Specify observed=True in groupby instead.

@rhshadrach, could you please advise me on how to fix this test?

rhshadrach
rhshadrach reviewed Jul 18, 2024
View reviewed changes
pandas/core/groupby/grouper.py Outdated
Comment on lines 674 to 682
if not self.key_dtype_str:
warnings.warn(
"`groups` by one element list returns scalar is deprecated "
"and will be removed. In a future version `groups` by one element "
"list will return tuple. Use ``df.groupby(by='a').groups`` "
"instead of ``df.groupby(by=['a']).groups`` to avoid this warning",
FutureWarning,
stacklevel=find_stack_level(),
)
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 move this to GroupBy.groups? Then you won't need to modify Grouping (e.g. include extra arguments) at all.

@rhshadrach
Copy link
Member

@rhshadrach, could you please advise me on how to fix this test?

Sorry for the delay! I believe you've resolved this, let me know if not.

@natmokval natmokval marked this pull request as ready for review July 19, 2024 12:33
@natmokval
Copy link
Contributor Author

Sorry for the delay! I believe you've resolved this, let me know if not.

Thanks @rhshadrach for reviewing this PR. I moved the FutureWarning from Grouping to GroupBy.groups as you suggested and added a note to v3.0.0. Could you please take a look at my changes?

rhshadrach
rhshadrach requested changes Jul 21, 2024
View reviewed changes
pandas/tests/groupby/test_grouping.py Outdated Show resolved Hide resolved
pandas/tests/groupby/test_groupby.py Outdated Show resolved Hide resolved
rhshadrach
rhshadrach approved these changes Jul 23, 2024
View reviewed changes
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 the Deprecate Functionality to remove in pandas label Jul 23, 2024
@rhshadrach rhshadrach added this to the 3.0 milestone Jul 23, 2024
@rhshadrach rhshadrach merged commit e191a06 into pandas-dev:main Jul 23, 2024
49 checks passed
@rhshadrach
Copy link
Member

Thanks @natmokval!

@github-actions github-actions bot mentioned this pull request Jul 23, 2024
Open
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhshadrach rhshadrach rhshadrach approved these changes

Assignees
No one assigned
Labels
API - Consistency Internal Consistency of API/Behavior Deprecate Functionality to remove in pandas Groupby
Projects
None yet
Milestone
3.0
Development

Successfully merging this pull request may close these issues.

BUG: Inconsistent types for groupby group names
2 participants
@natmokval @rhshadrach