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

[Java] Added method to remove null_masks if the column has no nulls #9061

Merged
merged 10 commits into from
Aug 20, 2021

Conversation

razajafri
Copy link
Contributor

This PR adds a method to remove the validity vector in cases where there are columns in a Table with no nulls but still have a validity vector.

@razajafri razajafri requested a review from a team as a code owner August 18, 2021 10:04
@github-actions github-actions bot added the Java Affects Java cuDF API. label Aug 18, 2021
@razajafri razajafri added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change and removed Java Affects Java cuDF API. labels Aug 18, 2021
@github-actions github-actions bot added the Java Affects Java cuDF API. label Aug 18, 2021
@codecov
Copy link

codecov bot commented Aug 18, 2021

Codecov Report

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

❗ Current head ec5da1e differs from pull request most recent head 139cd17. Consider uploading reports for the commit 139cd17 to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.10    #9061   +/-   ##
===============================================
  Coverage                ?   10.73%           
===============================================
  Files                   ?      114           
  Lines                   ?    19055           
  Branches                ?        0           
===============================================
  Hits                    ?     2046           
  Misses                  ?    17009           
  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 eb85d77...139cd17. Read the comment docs.

java/src/main/java/ai/rapids/cudf/ColumnVector.java Outdated Show resolved Hide resolved
java/src/test/java/ai/rapids/cudf/TableTest.java Outdated Show resolved Hide resolved
java/src/test/java/ai/rapids/cudf/TableTest.java Outdated Show resolved Hide resolved
java/src/test/java/ai/rapids/cudf/TableTest.java Outdated Show resolved Hide resolved
java/src/test/java/ai/rapids/cudf/TableTest.java Outdated Show resolved Hide resolved
@razajafri
Copy link
Contributor Author

Thanks for the review guys, is there anything else that you want me to change?

@razajafri
Copy link
Contributor Author

Rerun tests

@razajafri
Copy link
Contributor Author

@revans2 I have addressed your concerns. I have removed deprecation as it doesn't hurt to keep this API it also makes more sense and is less confusing to users.

java/src/main/java/ai/rapids/cudf/ColumnVector.java Outdated Show resolved Hide resolved
java/src/test/java/ai/rapids/cudf/TableTest.java Outdated Show resolved Hide resolved
java/src/test/java/ai/rapids/cudf/TableTest.java Outdated Show resolved Hide resolved
java/src/test/java/ai/rapids/cudf/TableTest.java Outdated Show resolved Hide resolved
@razajafri
Copy link
Contributor Author

Can I ask for one more review?

@razajafri
Copy link
Contributor Author

rerun tests

@dillon-cullinan
Copy link
Contributor

rerun tests

@razajafri
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 6cd0167 into rapidsai:branch-21.10 Aug 20, 2021
@razajafri razajafri deleted the remove_null_mask_if_needed branch August 20, 2021 22:54
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 Java Affects Java cuDF API. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants