Skip to content

Conversation

@ianmcook
Copy link
Member

@ianmcook ianmcook commented Apr 2, 2021

Also stringr::str_replace() and stringr::str_replace_all()

@github-actions
Copy link

github-actions bot commented Apr 2, 2021

@ianmcook ianmcook changed the title ARROW-11513: [R] Bindings for sub/gsub [WIP] ARROW-11513: [R] Bindings for sub/gsub Apr 5, 2021
@ianmcook ianmcook marked this pull request as ready for review April 5, 2021 19:34
Comment on lines 443 to 451
if (ignore.case) {
if (fixed) {
pattern <- gsub("\\E", "\\e", pattern, fixed = TRUE)
pattern <- paste0("(?i)\\Q", pattern, "\\E")
replacement <- gsub("\\", "\\\\", replacement, fixed = TRUE)
} else {
pattern <- paste0("(?i)", pattern)
}
}
Copy link
Member Author

@ianmcook ianmcook Apr 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maartenbreddels since ARROW-10306 did not directly support case-insensitive fixed string replacement (non-regex), this is the workaround we used to support this in the R bindings. Feedback welcome, and thanks for your work on the underlying C++ code!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments added explaining this code in db828be

Copy link
Member

@nealrichardson nealrichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking close, just a few more questions

Copy link
Member

@nealrichardson nealrichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, nice work! Will merge when CI is green-ish.

pachadotdev pushed a commit to pachadotdev/arrow that referenced this pull request Apr 6, 2021
Also `stringr::str_replace()` and `stringr::str_replace_all()`

Closes apache#9878 from ianmcook/ARROW-11513

Lead-authored-by: Ian Cook <ianmcook@gmail.com>
Co-authored-by: Neal Richardson <neal.p.richardson@gmail.com>
Signed-off-by: Neal Richardson <neal.p.richardson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants