Repository navigation
Conversation
There was a problem hiding this comment.
📋 Review Summary
This PR registers _FilteringIterable with FastPrimitivesCoderImpl.register_iterable_like_type, allowing FastPrimitivesCoder (including its deterministic variant) to natively encode side input iterables without falling back to pickling or failing deterministic encoding checks.
🔍 General Feedback
- The implementation in
sideinputs.pyis clean and aligns with how other lazy iterable wrappers (such as_ConcatSequenceand_AbstractIterable) are registered withFastPrimitivesCoderImpl. - Consider adding a unit test in
sideinputs_test.pyorcoders_test_common.pyto explicitly verify encoding and deterministic encoding of_FilteringIterable(e.g., passing anAsIterside input directly into a transform requiring deterministic encoding).
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40888 +/- ##
===========================================
+ Coverage 0 79.83% +79.83%
===========================================
Files 0 494 +494
Lines 0 83575 +83575
===========================================
+ Hits 0 66719 +66719
- Misses 0 16856 +16856
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Assigning reviewers: R: @jrmccluskey for label python. This pull request likely touches a core component ("core" label). Please review with scrutiny. 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). |
No description provided.