-
Notifications
You must be signed in to change notification settings - Fork 902
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
Add detail interface for split
and slice(table_view)
, refactors both function with host_span
#9226
Conversation
Codecov Report
@@ Coverage Diff @@
## branch-21.12 #9226 +/- ##
================================================
- Coverage 10.79% 10.75% -0.04%
================================================
Files 116 116
Lines 18869 19481 +612
================================================
+ Hits 2036 2096 +60
- Misses 16833 17385 +552
Continue to review full report at Codecov.
|
I think |
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.
If the tests passed it would lgtm.
rerun tests |
Changed a couple more places with use of This does not count nulls, so no need to use cudf/cpp/include/cudf/detail/copy.hpp Line 45 in 015f15c
This one does. cudf/cpp/include/cudf/detail/copy.hpp Lines 74 to 76 in 015f15c
And I only refactor usages of the latter. |
Since you're already touching it, can you change |
split
and slice(table_view)
split
and slice(table_view)
, refactors both function with host_span
The PR became a bit larger because of refactoring the indices specification with Vscode regex find and replace helped. |
…Add_detail_split
Added an overload of split/slice interface with |
@gpucibot merge |
cudf::detail::slice
performs asegmented_count_unset_bits
that requires stream ordering. The dependingsplit
interface does not have an internal version that accepts astream
argument. Similarly forslice(table_view)
. This PR fixes that.Besides, slice/split interface is refactored to accept
host_span
to specify indices/splits, and is overloaded withstd::initializer_list
. This allows specifying the argument with both a container and a brace-init-list.