-
Notifications
You must be signed in to change notification settings - Fork 908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[REVIEW] Define and implement new stream compaction APIs copy_if
, drop_nulls
, apply_boolean_mask
, drop_duplicate
and unique_count
.
#3303
[REVIEW] Define and implement new stream compaction APIs copy_if
, drop_nulls
, apply_boolean_mask
, drop_duplicate
and unique_count
.
#3303
Conversation
…948_adding_copy_if
Codecov Report
@@ Coverage Diff @@
## branch-0.11 #3303 +/- ##
============================================
Coverage 87.36% 87.36%
============================================
Files 49 49
Lines 9295 9295
============================================
Hits 8121 8121
Misses 1174 1174
Continue to review full report at Codecov.
|
copy_if
and drop_null
copy_if
and drop_null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, except for the mystery of the missing apply_boolean_mask
!
@harrism Ouch, I thought it had different kernel and was planning a PR along with drop_duplicate. I will update this PR with that one as well. |
SHouldn't be too hard -- it's a wrapper around copy_if like drop_nulls is. |
*/ | ||
std::unique_ptr<experimental::table> | ||
drop_nulls(table_view const& input, | ||
table_view const& keys, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused. Jake asked you to changed this to a vector of indices, and you agreed, but the change hasn't been made and the conversation is resolved. Unresolving to confirm.
rerun tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic work, Ram!
Define and implement new stream compaction APIs
copy_if
,drop_nulls
,apply_boolean_mask
,drop_duplicate
andunique_count
.NOTE: All the APIs supports string
close #2948
close #3415