Skip to content

Extends DataFrame.groupby overloads to recognize some scalar index types #679

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

Merged
merged 2 commits into from
May 6, 2023

Conversation

Daverball
Copy link
Contributor

@Daverball Daverball commented May 3, 2023

@Daverball
Copy link
Contributor Author

Initially I had also added the Series version of each Index to the type unions and added a separate new test for those, however it seems a little less useful, since you would have to use the series from the dataframe, which does not have a specific type, so you would need to manually cast to a more specific type to get the benefit there.

It certainly can be done, but it felt so awkward writing the test that I decided against it for now. It's also not a pattern I personally have ever used before on a groupby. This also decreases the surface area of the patch, so it should be a less controversial change overall.

@Daverball Daverball changed the title Extends groupby on dataframe to recognize some scalar index types Extends DataFrame.groupby overloads to recognize some scalar index types May 3, 2023
Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. Can you make the change to use Generic and then just have separate overloads for the different Index types you are including?

Add additional overloads to `DataFrame.groupby` to cover all index types
Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

This looks really good. Just some small changes to make.

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

Thanks @Daverball . Really nice contribution.

@Dr-Irv Dr-Irv merged commit bd817d2 into pandas-dev:main May 6, 2023
@Daverball Daverball deleted the groupby-scalar-index branch May 6, 2023 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataFrame.groupby should return _DataFrameGroupByScalar when grouping by a PeriodIndex
3 participants