Skip to content

Fix construction of Grouping objects - #13932

Merged
rapids-bot[bot] merged 8 commits into
NVIDIA:branch-23.10from
galipremsagar:13923
Aug 23, 2023
Merged

Fix construction of Grouping objects#13932
rapids-bot[bot] merged 8 commits into
NVIDIA:branch-23.10from
galipremsagar:13923

Conversation

@galipremsagar

@galipremsagar galipremsagar commented Aug 22, 2023 •
edited by wence-
Loading

Copy link
Copy Markdown
Contributor

Description

This PR fixes multiple issues with groupby where the consequent operations on a groupby object always return incorrect results due to an inplace modification that happens in the Groupby constructor. To fix this, take a copy of the by argument whenever it is an (internal) _Grouping object.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@galipremsagar
galipremsagar requested a review from a team as a code owner August 22, 2023 01:51
@galipremsagar
galipremsagar marked this pull request as draft August 22, 2023 01:51
@github-actions github-actions Bot added the Python Affects Python cuDF API. label Aug 22, 2023
galipremsagar
galipremsagar commented Aug 22, 2023
View reviewed changes
Comment thread python/cudf/cudf/core/groupby/groupby.py
@galipremsagar galipremsagar self-assigned this Aug 23, 2023
@galipremsagar
galipremsagar marked this pull request as ready for review August 23, 2023 00:33
@galipremsagar galipremsagar added bug Something isn't working non-breaking Non-breaking change 3 - Ready for Review Ready for review by team 4 - Needs cuDF (Python) Reviewer labels Aug 23, 2023
@galipremsagar
galipremsagar requested a review from bdice August 23, 2023 00:35
wence-
wence- requested changes Aug 23, 2023
View reviewed changes
Comment thread python/cudf/cudf/core/groupby/groupby.py Outdated
Comment thread python/cudf/cudf/core/resample.py
Comment thread python/cudf/cudf/core/series.py Outdated
Comment thread python/cudf/cudf/core/groupby/groupby.py Outdated
Comment thread python/cudf/cudf/core/groupby/groupby.py
@galipremsagar
galipremsagar requested a review from wence- August 23, 2023 15:15
wence-
wence- approved these changes Aug 23, 2023
View reviewed changes
Comment thread python/cudf/cudf/core/groupby/groupby.py
@wence-

wence- commented Aug 23, 2023

Copy link
Copy Markdown
Contributor

Serialization test looks like it fails https://github.com/rapidsai/cudf/actions/runs/5953195610/job/16147414724?pr=13932

@galipremsagar

galipremsagar commented Aug 23, 2023

Copy link
Copy Markdown
Contributor Author

Serialization test looks like it fails https://github.com/rapidsai/cudf/actions/runs/5953195610/job/16147414724?pr=13932

Fixed it 👍

@galipremsagar galipremsagar added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Aug 23, 2023
@galipremsagar

galipremsagar commented Aug 23, 2023

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit abac227 into NVIDIA:branch-23.10 Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wence- wence- wence- approved these changes
@bdice bdice bdice approved these changes
@mroeschke mroeschke Awaiting requested review from mroeschke mroeschke was automatically assigned from rapidsai/cudf-python-codeowners

Assignees

@galipremsagar galipremsagar

Labels

5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working non-breaking Non-breaking change Python Affects Python cuDF API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Groupby object returns erroneous results for subsequent nth calls [BUG] groupby object is unusable after calling .cumcount

4 participants

@galipremsagar @wence- @bdice @mroeschke