Fix cast_string_to_float with trailing whitespaces for inf and nan string #2063
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: fejiang <fejiang@nvidia.com>
…ni into branch-24.06
thirtiseven
reviewed
May 22, 2024
| tb2.column("INFINITY ", "inf", "+inf ", " -INF "); | ||
|
|
||
| Table.TestBuilder tb = new Table.TestBuilder(); | ||
| tb.column(Float.POSITIVE_INFINITY, Float.POSITIVE_INFINITY, Float.POSITIVE_INFINITY, Float.NEGATIVE_INFINITY); |
There was a problem hiding this comment.
Let's test double type too.
There was a problem hiding this comment.
Some negative tests would be nice. Some strings like INFINITY AND BEYOND.
There was a problem hiding this comment.
INFINITY AND BEYOND added.
with aother one test string INF.
More test cases suggestions are appreciated.
Signed-off-by: fejiang <fejiang@nvidia.com>
hyperbolic2346
requested changes
May 28, 2024
| tb2.column("INFINITY ", "inf", "+inf ", " -INF "); | ||
|
|
||
| Table.TestBuilder tb = new Table.TestBuilder(); | ||
| tb.column(Float.POSITIVE_INFINITY, Float.POSITIVE_INFINITY, Float.POSITIVE_INFINITY, Float.NEGATIVE_INFINITY); |
There was a problem hiding this comment.
Some negative tests would be nice. Some strings like INFINITY AND BEYOND.
jlowe
changed the title
hyperbolic2346
previously approved these changes
May 29, 2024
Thank you for reviewing, appreciate it. |
|
build |
hyperbolic2346
previously approved these changes
May 30, 2024
Feng-Jiang28
dismissed stale reviews from hyperbolic2346 and thirtiseven
via
26f2a75
|
build |
1 similar comment
|
build |
thirtiseven
previously approved these changes
May 31, 2024
hyperbolic2346
approved these changes
Jun 3, 2024
This was referenced Jun 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix the issue listed in #10794.
In RapidsCastSuit of Spark UT, "Process Infinity, -Infinity, NaN in case insensitive manner" case would fail, the reason is that in jni's
cast_to_float.cu, the functions for checking inf and nan were considering "inf"("infinity") and "nan" strings with additional characters as illegal. I added the logic for checking if the additional characters are whitespaces.