Handle case of scan aggregation in groupby-transform #15450
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When performing a groupby-transform with a scan aggregation, the intermediate result obtained from calling groupby-agg is already the correct shape and does not need to be broadcast to align with the grouping keys. To handle this, make sure that if the requested transform is a scan that we don't try and broadcast. While here, tighten up the input checking: transform only applies to a single aggregation, rather than the more general interface offered by agg. - Closes rapidsai#12621 - Closes rapidsai#15448
wence-
added
bug
bdice
approved these changes
Apr 3, 2024
|
|
||
| def test_transform_invalid(): | ||
| df = cudf.DataFrame({"key": [1, 1], "values": [4, 5]}) | ||
| with pytest.raises(ValueError): |
There was a problem hiding this comment.
Do we care to raise the same error as pandas? This ends up hitting a TypeError in pandas for a few of the "bad" inputs that I tried like {"values": "cumprod"} or the tuple ("cumprod",) or the integer 3. Only invalid function names (strings) raise ValueError from what I can tell.
There was a problem hiding this comment.
Oh probably, I didn't check exhaustively what pandas produces.
It's been here forever, so arguably 24.06 is OK. |
|
/merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When performing a groupby-transform with a scan aggregation, the intermediate result obtained from calling groupby-agg is already the correct shape and does not need to be broadcast to align with the grouping keys.
To handle this, make sure that if the requested transform is a scan that we don't try and broadcast.
While here, tighten up the input checking: transform only applies to a single aggregation, rather than the more general interface offered by agg.
Checklist