[Data] Support Non-Hashable Types for Unique Aggregator and encode_lists flag
#58916
+85
−7
Conversation
This file contains hidden or 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
Signed-off-by: Daniel Shin <[email protected]>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds support for non-hashable types to the Unique aggregator by using pickling, and introduces an encode_lists flag for more flexible handling of list data. The implementation is sound and includes a comprehensive set of tests. My review includes one suggestion to refactor the aggregate_block method for improved performance and readability.
ray-gardener
bot
added
python
Signed-off-by: Daniel Shin <[email protected]>
…ion; previous ignore_nulls was broken Signed-off-by: Daniel Shin <[email protected]>
Signed-off-by: Daniel Shin <[email protected]>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Signed-off-by: Daniel Shin <[email protected]>
kyuds
commented
Nov 23, 2025
| Std("B", alias_name="std_b", ignore_nulls=ignore_nulls), | ||
| Quantile("B", alias_name="quantile_b", ignore_nulls=ignore_nulls), | ||
| Unique("B", alias_name="unique_b"), | ||
| Unique("B", alias_name="unique_b", ignore_nulls=False), |
Contributor
Author
There was a problem hiding this comment.
this is to fix tests, because ignore_nulls was broken before, so we don't filter for nulls at all
Signed-off-by: Daniel Shin <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Basically the same idea as #58659
So
Uniqueaggregator usespyarrow.compute.uniquefunction internally. This doesn't work with non-hashable types like lists. Similar to what I did forApproximateTopK, we now use pickle to serialize and deserialize elements.Other improvements:
ignore_nullsflag didn't work at all. This flag now properly worksignore_nulls=Falsefor datasetsuniqueapi for backwards compatibility (we setignore_nullstoTrueby default, so behavior for datasetsuniqueapi will change now thatignore_nullsactually works)Related issues
This PR replaces #58538
Additional information
N/A