Skip to content

Update lists::contains to experimental row comparator#13810

Merged
rapids-bot[bot] merged 10 commits intorapidsai:branch-23.10from
divyegala:row-comp-list-contains
Aug 3, 2023
Merged

Update lists::contains to experimental row comparator#13810
rapids-bot[bot] merged 10 commits intorapidsai:branch-23.10from
divyegala:row-comp-list-contains

Conversation

@divyegala
Copy link
Copy Markdown
Member

@divyegala divyegala commented Aug 2, 2023

Description

Part of #11844

Checklist

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

@divyegala divyegala requested a review from a team as a code owner August 2, 2023 21:45
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Aug 2, 2023
@divyegala divyegala self-assigned this Aug 2, 2023
@divyegala divyegala added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 2, 2023
bdice
bdice requested changes Aug 2, 2023
View reviewed changes
Copy link
Copy Markdown
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.

Let's wait to merge until #13805 is merged, then the diff should get better here. Ideally we should rebase over branch-23.10 once #13805 merges.

@divyegala divyegala requested a review from bdice August 2, 2023 21:58
bdice
bdice reviewed Aug 2, 2023
View reviewed changes
Copy link
Copy Markdown
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.

Code looks fine. I'd like to wait to approve until #13805 is merged, so I can see the final diff again.

ttnghia
ttnghia reviewed Aug 3, 2023
View reviewed changes
bdice
bdice approved these changes Aug 3, 2023
View reviewed changes
Copy link
Copy Markdown
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.

Nice work. 🧘‍♂️ The zen of non-type-dispatching (043ae29) is now yours to cherish. Let it blossom throughout the code base.

@divyegala @bdice
Update cpp/src/lists/contains.cu
ba6ff6e 
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
@divyegala
Copy link
Copy Markdown
Member Author

divyegala commented Aug 3, 2023

/merge

@rapids-bot rapids-bot bot merged commit 15cc501 into rapidsai:branch-23.10 Aug 3, 2023
@bdice bdice mentioned this pull request Aug 21, 2023
Closed
@GregoryKimball GregoryKimball mentioned this pull request Aug 29, 2023
Closed
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

@ttnghia ttnghia ttnghia approved these changes

@PointKernel PointKernel PointKernel approved these changes

@nvdbaranec nvdbaranec Awaiting requested review from nvdbaranec nvdbaranec is a code owner automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees

@divyegala divyegala

Labels

improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@divyegala @bdice @ttnghia @PointKernel