Add JNI support for the join_strings API - #11309
Conversation
revans2
added
3 - Ready for Review
jlowe
left a comment
jlowe
left a comment
There was a problem hiding this comment.
There should be a unit test to at least smoke-test the binding is working properly and the functionality isn't surprisingly changed underneath in libcudf.
|
@jlowe added the test please take another look |
| return new ColumnVector(joinStrings(getNativeView(), separator.getScalarHandle(), | ||
| narep.getScalarHandle())); | ||
| } | ||
| throw new IllegalArgumentException("The input, spearator, and narep all need to be the STRINGs"); |
There was a problem hiding this comment.
minor nit,
| throw new IllegalArgumentException("The input, spearator, and narep all need to be the STRINGs"); | |
| throw new IllegalArgumentException("The input, spearator, and narep all need to be STRINGs"); |
|
@abellina I have fixed the error message |
| @@ -0,0 +1,2 @@ | |||
| package ai.rapids.cudf;public class TableWithMeta { | |||
There was a problem hiding this comment.
| package ai.rapids.cudf;public class TableWithMeta { | |
| package ai.rapids.cudf; | |
| public class TableWithMeta { |
There was a problem hiding this comment.
Wait, I don't see this class is used anywhere in this PR? And it is an empty class without anything inside? And this file has just this class, nothing else like copyright header?
So was this file/class added by mistake?
|
rerun tests |
Codecov Report
@@ Coverage Diff @@
## branch-22.08 #11309 +/- ##
================================================
+ Coverage 86.34% 86.37% +0.03%
================================================
Files 144 144
Lines 22826 22826
================================================
+ Hits 19708 19715 +7
+ Misses 3118 3111 -7
Continue to review full report at Codecov.
|
|
@gpucibot merge |
vyasr
added
4 - Needs Review
6 participants
This just adds in a simple JNI binding for the join_strings cudf function.