Add Sample.Any to the Python SDK to match Java's Sample.any - #39442
Add Sample.Any to the Python SDK to match Java's Sample.any#39442SreeramaYeshwanthGowd wants to merge 3 commits into
Conversation
Python only had Sample.FixedSizeGlobally, which runs a uniform reservoir sample and returns a single list. Add Sample.Any, the equivalent of Java's Sample.any, which returns up to n arbitrary elements as a PCollection without the random sampling cost. If the input has fewer than n elements, all are returned. Includes unit tests on the DirectRunner and a CHANGES.md entry. Fixes apache#18552
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Assigning reviewers: R: @claudevdm for label python. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
Reminder, please take a look at this pr: @claudevdm |
bvolpato
left a comment
There was a problem hiding this comment.
LGTM. Clean bounded implementation and focused coverage. Non-blocking follow-up: align Sample.Any(-1) with Java Sample.any by rejecting negative n, and add a regression test.
Match Java's Sample.any, which rejects a negative limit at construction time. Add a regression test.
|
Thanks @bvolpato. Done, Sample.Any now rejects negative n with a ValueError matching Java's Sample.any, and added a regression test. |
Problem
Java has
Sample.any(n), which returns up to n arbitrary elements from a PCollection without the cost of a uniform random sample. The Python SDK has no equivalent. Its only global option isSample.FixedSizeGlobally(n), which runs a uniform reservoir sample and returns a singlelist[T]. This gap was reported in #18552.Solution
Add
Sample.Any, the Python equivalent of Java'sSample.any. It keeps up to n arbitrary elements using a smallCombineFnand returns them as a PCollection of individual elements. It makes no uniformity guarantee and is cheaper thanFixedSizeGloballybecause it skips the per element random sampling. If the input has fewer than n elements, all of them are returned. This mirrors Java's semantics and its element returning result type.Testing
Added unit tests in
combiners_test.pythat run on the DirectRunner and cover the normal case, an input smaller than n, a windowed input, an empty input, and n equal to zero. Also extended the existing display data test to cover the new transform.Fixes #18552
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.