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

Use column_views instead of column_device_views in binary operations. #10780

Merged

Conversation

bdice
Copy link
Contributor

@bdice bdice commented May 3, 2022

This PR changes the internal APIs used for binary operations to use column_view objects instead of column_device_view objects. This change is needed for the eventual support of structs in binary operations. See also: PR #9452.

@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label May 3, 2022
@bdice bdice added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 3, 2022
@bdice bdice self-assigned this May 3, 2022
@bdice bdice marked this pull request as ready for review May 3, 2022 23:08
@bdice bdice requested a review from a team as a code owner May 3, 2022 23:08
@bdice bdice requested review from devavret and hyperbolic2346 May 3, 2022 23:08
Copy link
Contributor

@rwlee rwlee left a comment

Choose a reason for hiding this comment

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

Small nits, otherwise looks good and matches functionality implemented and already reviewed in #9452

@codecov
Copy link

codecov bot commented May 4, 2022

Codecov Report

Merging #10780 (4f39145) into branch-22.06 (a9eb47c) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 4f39145 differs from pull request most recent head a444649. Consider uploading reports for the commit a444649 to get more accurate results

@@               Coverage Diff                @@
##           branch-22.06   #10780      +/-   ##
================================================
+ Coverage         86.40%   86.43%   +0.02%     
================================================
  Files               143      143              
  Lines             22448    22448              
================================================
+ Hits              19396    19402       +6     
+ Misses             3052     3046       -6     
Impacted Files Coverage Δ
python/cudf/cudf/testing/_utils.py 93.98% <100.00%> (ø)
python/cudf/cudf/core/dataframe.py 93.74% <0.00%> (+0.04%) ⬆️
python/cudf/cudf/core/column/string.py 89.21% <0.00%> (+0.12%) ⬆️
python/cudf/cudf/core/groupby/groupby.py 91.79% <0.00%> (+0.22%) ⬆️
python/cudf/cudf/core/tools/datetimes.py 84.49% <0.00%> (+0.30%) ⬆️
python/cudf/cudf/core/column/lists.py 92.91% <0.00%> (+0.83%) ⬆️

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 ad12606...a444649. Read the comment docs.

@vyasr
Copy link
Contributor

vyasr commented May 4, 2022

@bdice same request as #10776, could you write a more descriptive PR description. In this case (unlike in #10776) it's not immediately clear to me what the motivation for this change is.

@bdice
Copy link
Contributor Author

bdice commented May 4, 2022

@gpucibot merge

@bdice
Copy link
Contributor Author

bdice commented May 4, 2022

rerun tests

@rapids-bot rapids-bot bot merged commit 0d11591 into rapidsai:branch-22.06 May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants