-
Notifications
You must be signed in to change notification settings - Fork 915
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
Refactor purge_nonempty_nulls
#12111
Refactor purge_nonempty_nulls
#12111
Conversation
@@ -80,6 +80,24 @@ bool has_nonempty_nulls(cudf::column_view const& input, rmm::cuda_stream_view st | |||
|
|||
return false; | |||
} | |||
|
|||
std::unique_ptr<column> purge_nonempty_nulls(column_view const& input, |
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.
This is moved from the deleted file copy.cuh
.
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.
LGTM
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.
One minor comment on docstrings, otherwise LGTM.
Someday I'd like to see the "pass-through" version of this which accepts a column &&
(takes ownership) and returns the existing data when possible instead of copying.
Co-authored-by: Bradley Dice <[email protected]>
Rerun tests. |
Rerun tests. |
Codecov ReportBase: 87.47% // Head: 88.15% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## branch-22.12 #12111 +/- ##
================================================
+ Coverage 87.47% 88.15% +0.67%
================================================
Files 133 135 +2
Lines 21826 22144 +318
================================================
+ Hits 19093 19520 +427
+ Misses 2733 2624 -109
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
@gpucibot merge |
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.
Approving ops-codeowner
file changes
This refactor combines the discrete interfaces of
purge_nonempty_nulls
that requirestructs/strings/lists_column_view
input into just one interface accepting justcolumn_view
. This facilitates easier usage of this function. It is also a necessary step for subsequent work in fixingstructs::superimpose_parent_nulls
.cudf::detail
interface for this new API is also added.