nealrichardson commented on a change in pull request #11232:
URL: https://github.com/apache/arrow/pull/11232#discussion_r717860993



##########
File path: r/R/dplyr-functions.R
##########
@@ -330,6 +330,38 @@ arrow_string_join_function <- function(null_handling, 
null_replacement = NULL) {
   }
 }
 
+# Arrow does not supports a locale option for string case conversion functions,
+# contrast to stringr's API, so the `locale` argument is only valid for the
+# standard/default ones: "en", "C", and "POSIX". The following are string
+# functions that take a `locale` option as its second argument:
+#   str_to_lower
+#   str_to_upper
+#   str_to_title
+.valid_locales_for_string_functions <- list("en", "C", "POSIX")
+
+arrow_string_function_with_locale_arg <- function(func, string, locale) {
+  assert_that(
+    locale %in% .valid_locales_for_string_functions,
+    msg = paste(
+      "`locale` must be any of: ",
+      paste(.valid_locales_for_string_functions, collapse=",")
+    )
+  )
+  Expression$create(func, string)
+}
+
+nse_funcs$str_to_lower <- function(string, locale = "en") {
+  arrow_string_function_with_locale_arg("utf8_lower", string, locale)

Review comment:
       I would ditch the function above and rewrite these like this:
   
   ```suggestion
     if (!identical(locale, "en")) {
       stop("Providing 'locale' to 'str_to_lower' not supported in Arrow; to 
change locale, use 'Sys.setlocale()'", call. = FALSE)
     }
     Expression$create("utf8_lower", string)
   ```
   
   Thoughts @jonkeane @thisisnic ? 
   
   (I'd also leave a comment referencing the jira issue that would support 
passing locale to the string kernels)




-- 
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]


Reply via email to