kou commented on code in PR #46590:
URL: https://github.com/apache/arrow/pull/46590#discussion_r2107950177
##########
cpp/src/arrow/compute/kernels/scalar_string_test.cc:
##########
@@ -2640,4 +2640,10 @@ TEST(TestStringKernels, UnicodeLibraryAssumptions) {
}
#endif
+TEST(TestUtf8IsDigit, SupportsUnicodeDigits) {
+ auto input = ArrayFromJSON(utf8(), R"(["٤", "³", "५", "Ⅷ", "٣٣", "१२३",
"abc", "１２３"])");
+ auto expected = ArrayFromJSON(boolean(), "[true, true, true, false, true,
true, false, true]");
+ CheckScalarUnary("utf8_is_digit", input, expected);
+}
Review Comment:
```suggestion
TEST(TestUtf8IsDigit, SupportsUnicodeDigits) {
auto input = ArrayFromJSON(utf8(), R"(["٤", "³", "५", "Ⅷ", "٣٣", "१२३",
"abc", "１２３"])");
auto expected = ArrayFromJSON(boolean(), "[true, true, true, false, true,
true, false, true]");
CheckScalarUnary("utf8_is_digit", input, expected);
}
#endif
```
##########
cpp/src/arrow/compute/kernels/scalar_string_utf8.cc:
##########
@@ -140,7 +140,7 @@ static inline bool IsDecimalCharacterUnicode(uint32_t
codepoint) {
static inline bool IsDigitCharacterUnicode(uint32_t codepoint) {
// Python defines this as Numeric_Type=Digit or Numeric_Type=Decimal.
// utf8proc has no support for this, this is the best we can do:
Review Comment:
Should we update this comment?
##########
cpp/src/arrow/compute/kernels/scalar_string_test.cc:
##########
@@ -2640,4 +2640,10 @@ TEST(TestStringKernels, UnicodeLibraryAssumptions) {
}
#endif
+TEST(TestUtf8IsDigit, SupportsUnicodeDigits) {
Review Comment:
```suggestion
TEST(TestStringKernels, Utf8IsDigit) {
```
BTW, should we update existing (but empty) one instead of adding a new test?
https://github.com/apache/arrow/blob/19439118ce25620b9773fe3616aa1e4dab4fd098/cpp/src/arrow/compute/kernels/scalar_string_test.cc#L1386-L1391
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
