GH-45314: [R] arrow R package: multiple replacement disclaimers for str_replace_all - #51168
GH-45314: [R] arrow R package: multiple replacement disclaimers for str_replace_all#51168thisisnic wants to merge 2 commits into
Conversation
|
|
github-actions
Bot
added
Component: R
awaiting committer review
There was a problem hiding this comment.
🟡 Changes recommended
The updated pattern-length validation error is misleading for str_remove*() (it mentions “replacements”), and the new error wording isn’t currently protected by a targeted test assertion.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Improves the Arrow R dplyr function-mapping UX by making the “single pattern/replacement only” limitation explicit in both runtime validation errors and the generated “Functions available in Arrow dplyr queries” documentation.
Changes:
- Update
arrow_r_string_replace_function()validation errors to mention that multiple replacements aren’t supported. - Attach
notesto thesub()/gsub()/str_replace*()/str_remove*()bindings so doc generation includes the limitation text. - Regenerate the function-mapping docs output so the
acerohelp page lists the new disclaimers.
File summaries
| File | Description |
|---|---|
| r/R/dplyr-funcs-string.R | Adjusts validation error text and adds notes metadata for replace/remove bindings. |
| r/R/dplyr-funcs-doc.R | Regenerated doc list to include multiple pattern/replacement disclaimers. |
| r/man/acero.Rd | Regenerated Rd output to reflect updated function-mapping notes. |
Review details
Files not reviewed (1)
- r/man/acero.Rd: Generated file
- Files reviewed: 2/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
The updated error messaging still allows a generic “missing replacement” error path for stringr-style named pattern inputs, which undermines the goal of consistently informative validation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- r/man/acero.Rd: Generated file
- Files reviewed: 3/4 changed files
- Comments generated: 1
- Review effort level: Lite
| if (length(replacement) != 1) { | ||
| validation_error("`replacement` must be a length 1 character vector") | ||
| validation_error( | ||
| "Multiple replacements not supported: `replacement` must be a length 1 character vector" | ||
| ) |
Rationale for this change
sub(),gsub(),str_replace(),str_replace_all(),str_remove(), andstr_remove_all()only accept a single pattern/replacement, but neither the docs nor the error message said why.What changes are included in this PR?
The validation error now says multiple replacements are not supported, and the limitation is documented against each of those functions on the "Functions available in Arrow dplyr queries" page.
Are these changes tested?
No
Are there any user-facing changes?
No
🤖 Generated with Claude Code, checked manually by @thisisnic
https://claude.ai/code/session_01Y273BE7ZELeBhwRRQH2xrv