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

Remove detail from device_span #7533

Merged
merged 4 commits into from
Mar 9, 2021
Merged

Remove detail from device_span #7533

merged 4 commits into from
Mar 9, 2021

Conversation

rwlee
Copy link
Contributor

@rwlee rwlee commented Mar 8, 2021

Fix for issue caused by stale PR issue from #7406

@rwlee rwlee added bug Something isn't working 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. labels Mar 8, 2021
@rwlee rwlee requested a review from a team as a code owner March 8, 2021 20:01
@rwlee rwlee requested review from trxcllnt and jrhemstad March 8, 2021 20:01
@rwlee rwlee added the non-breaking Non-breaking change label Mar 8, 2021
galipremsagar
galipremsagar requested changes Mar 8, 2021
View reviewed changes
Copy link
Contributor

@galipremsagar galipremsagar left a comment

Choose a reason for hiding this comment

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

I think we need to fix this line too: https://github.com/rapidsai/cudf/blob/branch-0.19/cpp/src/structs/structs_column_factories.cu#L53

galipremsagar
galipremsagar approved these changes Mar 8, 2021
View reviewed changes
Copy link
Contributor

@galipremsagar galipremsagar left a comment

Choose a reason for hiding this comment

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

Thanks @rwlee !

@rwlee
Copy link
Contributor Author

rwlee commented Mar 8, 2021

I think we need to fix this line too: https://github.com/rapidsai/cudf/blob/branch-0.19/cpp/src/structs/structs_column_factories.cu#L53

grep through the codebase for detail::device_span comes up clean now, had missed the reference without a cudf:: prefix.

@ttnghia
Copy link
Contributor

ttnghia commented Mar 8, 2021

Rerun tests.

@kkraus14
Copy link
Collaborator

kkraus14 commented Mar 8, 2021

rerun tests

@kkraus14
Copy link
Collaborator

kkraus14 commented Mar 8, 2021

@gpucibot merge

@codecov
Copy link

codecov bot commented Mar 8, 2021 •
edited
Loading

Codecov Report

Merging #7533 (67805f0) into branch-0.19 (7871e7a) will increase coverage by 0.41%.
The diff coverage is 93.75%.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.19    #7533      +/-   ##
===============================================
+ Coverage        81.86%   82.27%   +0.41%     
===============================================
  Files              101      101              
  Lines            16884    17261     +377     
===============================================
+ Hits             13822    14202     +380     
+ Misses            3062     3059       -3
Impacted Files Coverage Δ
python/cudf/cudf/core/column/column.py 87.80% <75.00%> (+0.04%) ⬆️
python/cudf/cudf/core/column/decimal.py 95.83% <100.00%> (+0.96%) ⬆️
python/cudf/cudf/core/column/string.py 86.76% <100.00%> (+0.26%) ⬆️
python/cudf/cudf/utils/gpu_utils.py 53.65% <0.00%> (-4.88%) ⬇️
python/cudf/cudf/core/abc.py 87.23% <0.00%> (-1.14%) ⬇️
python/cudf/cudf/io/feather.py 100.00% <0.00%> (ø)
python/cudf/cudf/comm/serialize.py 0.00% <0.00%> (ø)
python/cudf/cudf/_fuzz_testing/io.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/struct.py 100.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/_version.py 0.00% <0.00%> (ø)
... and 43 more

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 8b2ae92...8940de8. Read the comment docs.

@kkraus14
Copy link
Collaborator

kkraus14 commented Mar 9, 2021

rerun tests

@galipremsagar galipremsagar added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Mar 9, 2021
@kkraus14 kkraus14 mentioned this pull request Mar 9, 2021
Merged
@kkraus14
Copy link
Collaborator

kkraus14 commented Mar 9, 2021

rerun tests

@hyperbolic2346
Copy link
Contributor

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 53929eb into rapidsai:branch-0.19 Mar 9, 2021
@kkraus14 kkraus14 mentioned this pull request Mar 10, 2021
Closed
hyperbolic2346 pushed a commit to hyperbolic2346/cudf that referenced this pull request Mar 25, 2021
@rwlee @hyperbolic2346
Remove detail from device_span (rapidsai#7533)
0784a31 
Fix for issue caused by stale PR issue from rapidsai#7406

Authors:
  - @rwlee
  - Keith Kraus (@kkraus14)

Approvers:
  - Keith Kraus (@kkraus14)
  - Mike Wilson (@hyperbolic2346)
  - GALI PREM SAGAR (@galipremsagar)
  - Jake Hemstad (@jrhemstad)
  - Vukasin Milovanovic (@vuule)
  - Paul Taylor (@trxcllnt)

URL: rapidsai#7533
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trxcllnt trxcllnt trxcllnt approved these changes

@hyperbolic2346 hyperbolic2346 hyperbolic2346 approved these changes

@kkraus14 kkraus14 kkraus14 approved these changes

@galipremsagar galipremsagar galipremsagar approved these changes

@jrhemstad jrhemstad jrhemstad approved these changes

@vuule vuule vuule approved these changes

Assignees
No one assigned
Labels
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@rwlee @ttnghia @kkraus14 @hyperbolic2346 @trxcllnt @galipremsagar @jrhemstad @vuule