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

Expose OutOfBoundsPolicy in JNI for Table.gather #9406

Merged
merged 6 commits into from
Oct 11, 2021
Merged

Expose OutOfBoundsPolicy in JNI for Table.gather #9406

merged 6 commits into from
Oct 11, 2021

Conversation

abellina
Copy link
Contributor

@abellina abellina commented Oct 8, 2021

This change just makes it clearer what the second argument for Table.gather means. Hopefully it is slightly clearer than the old boolean value.

@abellina abellina added the Java Affects Java cuDF API. label Oct 8, 2021
@abellina abellina requested a review from a team as a code owner October 8, 2021 18:45
@abellina abellina added 4 - Needs cuDF (Java) Reviewer non-breaking Non-breaking change improvement Improvement / enhancement to an existing function breaking Breaking change and removed non-breaking Non-breaking change labels Oct 8, 2021
@abellina
Copy link
Contributor Author

abellina commented Oct 8, 2021

@jlowe @revans2 I set this to breaking since it's a public API I am changing.

jlowe
jlowe reviewed Oct 8, 2021
View reviewed changes
Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

Comment nits and we should avoid the breaking change, but otherwise lgtm.

java/src/main/java/ai/rapids/cudf/Table.java Show resolved Hide resolved
java/src/main/java/ai/rapids/cudf/Table.java Outdated Show resolved Hide resolved
java/src/main/java/ai/rapids/cudf/OutOfBoundsPolicy.java Show resolved Hide resolved
@abellina abellina added non-breaking Non-breaking change and removed breaking Breaking change labels Oct 8, 2021
@codecov
Copy link

codecov bot commented Oct 8, 2021 •
edited
Loading

Codecov Report

Merging #9406 (9b87e65) into branch-21.12 (ab4bfaa) will decrease coverage by 0.04%.
The diff coverage is 0.00%.

❗ Current head 9b87e65 differs from pull request most recent head 8c25802. Consider uploading reports for the commit 8c25802 to get more accurate results
Impacted file tree graph

@@               Coverage Diff                @@
##           branch-21.12    #9406      +/-   ##
================================================
- Coverage         10.79%   10.74%   -0.05%     
================================================
  Files               116      116              
  Lines             18869    19082     +213     
================================================
+ Hits               2036     2051      +15     
- Misses            16833    17031     +198
Impacted Files Coverage Δ
python/cudf/cudf/__init__.py 0.00% <0.00%> (ø)
python/cudf/cudf/_lib/__init__.py 0.00% <ø> (ø)
python/cudf/cudf/core/_base_index.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/categorical.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/column.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/datetime.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/lists.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/numerical.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/string.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/struct.py 0.00% <0.00%> (ø)
... and 26 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 e6caaf5...8c25802. Read the comment docs.

@abellina
Copy link
Contributor Author

abellina commented Oct 8, 2021

I see a number of unrelated python tests failing, around parquet/orc writing.

@abellina
Copy link
Contributor Author

abellina commented Oct 8, 2021

Filed: #9408 to track the CI issues.

@abellina abellina mentioned this pull request Oct 11, 2021
Closed
@firestarman @abellina
JNI data writer sink supports device_write_async.
4fcd452 
Now call the sync version directly.

Signed-off-by: Firestarman <[email protected]>
@abellina
Copy link
Contributor Author

Stealing this 4fcd452, from @firestarman's PR.

@abellina abellina mentioned this pull request Oct 11, 2021
Merged
@abellina
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 56eb91a into rapidsai:branch-21.12 Oct 11, 2021
@abellina abellina mentioned this pull request Oct 12, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@razajafri razajafri razajafri approved these changes

@jlowe jlowe jlowe approved these changes

Assignees
No one assigned
Labels
improvement Improvement / enhancement to an existing function Java Affects Java cuDF API. non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@abellina @jlowe @razajafri @firestarman