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.

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

Fix typo in list_device_view::pair_rep_end() #7423

Merged
merged 1 commit into from
Feb 23, 2021

Conversation

mythrocks
Copy link
Contributor

#7349 introduced rep-iterators for list_device_view. In implementation for list_device_view::pair_rep_end() has a typo, and is incorrect.

The change in this PR should fix the problem, and exercise the corrected implementation from lists::contains().

@mythrocks mythrocks requested a review from a team as a code owner February 22, 2021 19:31
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label Feb 22, 2021
@mythrocks mythrocks self-assigned this Feb 22, 2021
@mythrocks mythrocks added 4 - Needs Review Waiting for reviewer to review or respond bug Something isn't working non-breaking Non-breaking change labels Feb 22, 2021
@mythrocks
Copy link
Contributor Author

The error is regretted. I noticed the problem a moment before #7349 was merged. (I was going over it one last time before commit.)

@mythrocks mythrocks added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 5 - Ready to Merge Testing and reviews complete, ready to merge labels Feb 22, 2021
@codecov
Copy link

codecov bot commented Feb 22, 2021

Codecov Report

❗ No coverage uploaded for pull request base (branch-0.19@b887e58). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##             branch-0.19    #7423   +/-   ##
==============================================
  Coverage               ?   81.80%           
==============================================
  Files                  ?      101           
  Lines                  ?    16695           
  Branches               ?        0           
==============================================
  Hits                   ?    13658           
  Misses                 ?     3037           
  Partials               ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b887e58...1c07feb. Read the comment docs.

@harrism
Copy link
Member

harrism commented Feb 23, 2021

Is there no test that exercises this path?

@mythrocks
Copy link
Contributor Author

All the contains tests are to exercise this path. It was circumvented by an oversight of mine in #7349.

@mythrocks mythrocks added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Feb 23, 2021
@mythrocks
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit c3b3b96 into rapidsai:branch-0.19 Feb 23, 2021
@mythrocks mythrocks deleted the list-contains-fix-typo branch February 23, 2021 18:07
@mythrocks
Copy link
Contributor Author

Thanks for the reviews, @vuule, @trxcllnt!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - Needs Review Waiting for reviewer to review or respond 5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants