You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2025/05/27 00:47:27 UTC

Re: [PR] GH-46589: [C++] Fixed utf8_is_digit to support full Unicode digit range [arrow]

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", "123"])");
+  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", "123"])");
     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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org