Added javadocs to CastStrings.fromIntegersWithBase method [skip ci] #1452
Conversation
Signed-off-by: Raza Jafri <[email protected]>
razajafri
force-pushed
the
JNI-1451-javadocs
branch
from
acd7a05
to
b0a54eb
Compare
| @@ -109,6 +109,21 @@ public static ColumnVector toIntegersWithBase(ColumnView cv, int base, | |||
| type.getTypeId().getNativeId())); | |||
| } | |||
|
|
|||
| /** | |||
| * Converts an integer column to a strings column with either decimal or hexadecimal values | |||
There was a problem hiding this comment.
Eventually, the full base range [2;36] will be supported. I think the current limitation to decimal/hexadecimal should be just a note towards the end but not the opening sentence
There was a problem hiding this comment.
Right now we only support 2 bases and it's ok to have it in the main description. We can make it more generic when time comes and we support more bases
There was a problem hiding this comment.
Thank you for taking time out to review this. If you still feel strongly I can make the change. PTAL
| * based on the base provided. The hexadecimal value will be returned with the leading | ||
| * zeros dropped |
There was a problem hiding this comment.
Again maybe in a "Note:" say that there are no leading zeros or padding of digits to a particular bit width
|
build |
razajafri
force-pushed
the
JNI-1451-javadocs
branch
from
7575d28
to
07ede64
Compare
|
build |
|
@gerashegalov I have updated the PR based on our discussion offline |
|
build |
| * Example: | ||
| * input = [123, -1, 0, 27, 342718233] | ||
| * s = fromIntegersWithBase(input, 16) | ||
| * s is [ '4D2', 'FFFFFFFF', '0', '1B', '146D7719'] |
There was a problem hiding this comment.
is this accurate? 123 should map to "7B"
There was a problem hiding this comment.
Yes, you are right. I copied this from cudf doc here.
I have created a ticket in cudf rapidsai/cudf#14232
|
build |
razajafri
changed the title
|
build |
1 similar comment
|
build |
This PR just adds some javadocs to the method
fixes #1451