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

[REVIEW] Add dictionary support to cudf::clamp #6373

Merged
merged 12 commits into from
Oct 2, 2020

Conversation

davidwendt
Copy link
Contributor

Reference #5963

Add specialization logic for dictionary type to the cudf::clamp API.
This required introducing a cudf::dictionary::detail::get_insert_index function to retrieve an appropriate scalar index from an input scalar key value.
This PR includes gtests for both the dictionary specialization and the new get_insert_index function.

@davidwendt davidwendt requested a review from a team as a code owner September 30, 2020 17:19
@davidwendt davidwendt self-assigned this Sep 30, 2020
@davidwendt davidwendt added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. labels Sep 30, 2020
@codecov
Copy link

codecov bot commented Sep 30, 2020

Codecov Report

Merging #6373 into branch-0.16 will increase coverage by 0.33%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.16    #6373      +/-   ##
===============================================
+ Coverage        82.82%   83.15%   +0.33%     
===============================================
  Files               94       94              
  Lines            14432    14779     +347     
===============================================
+ Hits             11953    12290     +337     
- Misses            2479     2489      +10     
Impacted Files Coverage Δ
python/cudf/cudf/io/feather.py 100.00% <0.00%> (ø)
python/cudf/cudf/testing/io.py 0.00% <0.00%> (ø)
python/cudf/cudf/comm/serialize.py 0.00% <0.00%> (ø)
python/custreamz/custreamz/_version.py 0.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/_version.py 0.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/io/tests/test_csv.py 100.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/io/tests/test_orc.py 100.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/io/tests/test_json.py 100.00% <0.00%> (ø)
...ython/custreamz/custreamz/tests/test_dataframes.py 100.00% <0.00%> (ø)
...ython/dask_cudf/dask_cudf/io/tests/test_parquet.py 100.00% <0.00%> (ø)
... and 34 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 dbe496c...a544ffe. Read the comment docs.

Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

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

Looks good. Just DRYing

cpp/src/replace/clamp.cu Outdated Show resolved Hide resolved
cpp/src/replace/clamp.cu Outdated Show resolved Hide resolved
@davidwendt
Copy link
Contributor Author

rerun tests

cpp/src/replace/clamp.cu Outdated Show resolved Hide resolved
cpp/src/replace/clamp.cu Show resolved Hide resolved
Copy link
Contributor

@trxcllnt trxcllnt left a comment

Choose a reason for hiding this comment

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

Left a question on dispatch_clamp null handling.

@davidwendt davidwendt merged commit c4f7495 into rapidsai:branch-0.16 Oct 2, 2020
@davidwendt davidwendt deleted the dictionary-clamp branch October 2, 2020 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants