Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Expose streams in public replace APIs #14010

Merged
merged 5 commits into from
Sep 1, 2023
Merged

Expose streams in public replace APIs #14010

merged 5 commits into from
Sep 1, 2023

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Aug 30, 2023

Description

Contributes to #925

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@vyasr vyasr added feature request New feature or request 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change labels Aug 30, 2023
@vyasr vyasr added this to the Enable streams milestone Aug 30, 2023
@vyasr vyasr self-assigned this Aug 30, 2023
@vyasr vyasr requested a review from a team as a code owner August 30, 2023 19:08
@vyasr vyasr requested review from bdice and mythrocks August 30, 2023 19:08
@github-actions github-actions bot added the CMake CMake build issue label Aug 30, 2023
@vyasr vyasr force-pushed the feat/streams_replace branch from c00a9d3 to 63544f6 Compare August 30, 2023 20:47
@GregoryKimball GregoryKimball mentioned this pull request Aug 31, 2023
Closed
@vyasr vyasr force-pushed the feat/streams_replace branch from f85cbf0 to e99e5ae Compare September 1, 2023 17:22
@vyasr vyasr requested a review from davidwendt September 1, 2023 17:23
bdice
bdice approved these changes Sep 1, 2023
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good reviews @davidwendt and @mythrocks -- I think this is ready.

@bdice
Copy link
Contributor

bdice commented Sep 1, 2023

/merge

@rapids-bot rapids-bot bot merged commit 2b7294b into rapidsai:branch-23.10 Sep 1, 2023
@vyasr
Copy link
Contributor Author

vyasr commented Sep 5, 2023

Can't find the comment anymore, but for the record the reason the tests include explicit casts from column_wrapper to cudf::column_view and cudf::mutable_column_view when calling normalize_nans_and_zeros is that both casts operators are defined and the function also has overloads for both, so the calls are ambiguous.

@vyasr vyasr mentioned this pull request Sep 7, 2023
Merged
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Sep 9, 2023
@vyasr
Remove the mr from the base fixture (#14057)
886e189 
This mr is just an alias for the current memory resource, so we don't really need it. This came up in #14010 (comment). This PR removes all uses of it, but does not actually remove the mr yet. That will be done in a follow-up (see #14057 (comment)).

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Nghia Truong (https://github.com/ttnghia)

URL: #14057
@vyasr vyasr deleted the feat/streams_replace branch January 23, 2024 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdice bdice bdice approved these changes

@mythrocks mythrocks mythrocks approved these changes

@davidwendt davidwendt davidwendt approved these changes

Assignees

@vyasr vyasr

Labels
3 - Ready for Review Ready for review by team CMake CMake build issue feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Milestone
Enable streams
Development

Successfully merging this pull request may close these issues.
4 participants
@vyasr @bdice @mythrocks @davidwendt