[I] [R] arrow R package: multiple replacement disclaimers for str_replace_all [arrow]

["baarthur (via GitHub)" <gi...@apache.org>]

baarthur opened a new issue, #45314:
URL: https://github.com/apache/arrow/issues/45314

   ### Describe the enhancement requested
   
   While PR https://github.com/apache/arrow/pull/39219 introduced an error message when the user passes pattern/replacement vectors with more than one element to `arrow_r_string_replace_function`, I'd like to suggest two enhancements. 
   
   * First and most fundamental: add a caveat on [this page](https://arrow.apache.org/docs/r/reference/acero.html#stringr), which, by the way, I use almost every day. On `r/R/dplyr-funcs-doc.R`: 
   ```
   #' * [`str_replace_all()`][stringr::str_replace_all()] Multiple replacements not yet supported,
   #' `pattern`/`replacement` must be a length 1 character vector
   ```
   
   * Second: make the `validation_error()` more explicit on what's happening. On `r/R/dplyr-funcs-string.R`:
   ```
   if (length(pattern) != 1) {
           validation_error(paste("Multiple replacements not supported,",
                                  "`pattern` must be a length 1 character vector"))
         }
         if (length(replacement) != 1) {
           validation_error(paste("Multiple replacements not supported,",
                                  "`replacement` must be a length 1 character vector"))
         }
   ```
   
   I'm about to submit a PR. But first, I would like to confirm some minor things: (1) does the test file update automatically when I run `devtools::test()`? (I can't test my changes right now), (2) is there any other test or change I should make before submitting the PR?
   
   ### Component(s)
   
   R


-- 
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: issues-unsubscribe@arrow.apache.org.apache.org

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

Re: [I] [R] arrow R package: multiple replacement disclaimers for str_replace_all [arrow]

["thisisnic (via GitHub)" <gi...@apache.org>]

thisisnic commented on issue #45314:
URL: https://github.com/apache/arrow/issues/45314#issuecomment-2614582598

   Hi @baarthur, thanks for opening this issue and great that you're going to submit a PR!  
   
   The test file doesn't automatically update when you run `devtools::test()`.  What's the reason you can't test your changes now?  Happy to help figure it out, but also happy for you to just submit the PR and we can figure it out from there.
   
   The function bindings act as a wrapper around the C++ library functions, and this feature isn't currently implemented in the C++ library and I don't think anyone is working on that right now, so "not supported" is great.
   


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

Re: [I] [R] arrow R package: multiple replacement disclaimers for str_replace_all [arrow]

["github-actions[bot] (via GitHub)" <gi...@apache.org>]

github-actions[bot] commented on issue #45314:
URL: https://github.com/apache/arrow/issues/45314#issuecomment-4440643913

   This issue has been marked as stale because it has had no activity in the past 365 days. Please remove the stale label or comment below, or this issue will be closed in 14 days. If this improvement is still desired but has no current owner, please add the 'Status: needs champion' label.


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