Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 4.3k
ARROW-11513: [R] Bindings for sub/gsub#9878
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Uh oh!
There was an error while loading. Please reload this page.
Closed
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
8116fef
Implement gsub
ianmcook 9c27f99
Respect ignore.case
ianmcook e2805b6
Break out gsub and sub into separate function that returns a function
ianmcook 9639057
Add tests
ianmcook 8bc82a6
Implement str_replace and str_replace_all
ianmcook e40c38b
Add tests for str_replace and str_replace_all
ianmcook 55d6f2f
Fixes, renaming, cleanup
ianmcook c61b13b
Better way to do case-insensitive fixed matches
ianmcook 107efe2
Add uppercase to test data
ianmcook a170c14
More elegant way to handle stringr pattern modifier functions
ianmcook 7745345
Warn on unsupported pattern modifier arguments
ianmcook a47ffe7
Handle edge cases for re2 case-insensitive fixed string replacement
ianmcook 7e7e748
Add and improve tests
ianmcook 49d38fb
Fixes
ianmcook e83d940
Test errors and warnings
ianmcook 7aee83c
Improve tests
ianmcook db828be
Add comments explaining case-insensitive literal string replace
ianmcook 25b869d
Improve tests
ianmcook afdde70
Update r/tests/testthat/test-dplyr-string-functions.R
nealrichardson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -422,6 +422,10 @@ build_function_list <- function(FUN) { | ||
| both = FUN("utf8_trim_whitespace", string) | ||
| ) | ||
| }, | ||
| sub = arrow_r_string_replace_function(FUN, 1L), | ||
| gsub = arrow_r_string_replace_function(FUN, -1L), | ||
| str_replace = arrow_stringr_string_replace_function(FUN, 1L), | ||
| str_replace_all = arrow_stringr_string_replace_function(FUN, -1L), | ||
| between = function(x, left, right) { | ||
| x >= left & x <= right | ||
| }, | ||
| @@ -434,6 +438,88 @@ build_function_list <- function(FUN) { | ||
| ) | ||
| } | ||
| arrow_r_string_replace_function <- function(FUN, max_replacements) { | ||
| function(pattern, replacement, x, ignore.case = FALSE, fixed = FALSE) { | ||
| if (ignore.case) { | ||
| # Prepend "(?i)" to the regex for case insensitivity | ||
| if (fixed) { | ||
| # Arrow lacks native support for case-insensitive literal string | ||
| # replacement, so we use the regular expression engine (RE2) to do this. | ||
| # https://github.com/google/re2/wiki/Syntax | ||
| # | ||
| # Everything between "\Q" and "\E" is treated as literal text. | ||
| # | ||
| # If the search text contains any literal "\E" strings, make them | ||
| # lowercase so they won't signal the end of the literal text: | ||
| pattern <- gsub("\\E", "\\e", pattern, fixed = TRUE) | ||
| pattern <- paste0("(?i)\\Q", pattern, "\\E") | ||
| # Escape single backslashes in the regex replacement text so they are | ||
| # interpreted as literal backslashes: | ||
| replacement <- gsub("\\", "\\\\", replacement, fixed = TRUE) | ||
| } else { | ||
| pattern <- paste0("(?i)", pattern) | ||
| } | ||
| } | ||
| FUN( | ||
| ifelse(fixed && !ignore.case, "replace_substring", "replace_substring_regex"), | ||
| x, | ||
| options = list( | ||
| pattern = pattern, | ||
| replacement = replacement, | ||
| max_replacements = max_replacements | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
| arrow_stringr_string_replace_function <- function(FUN, max_replacements) { | ||
| function(string, pattern, replacement) { | ||
| # Assign stringr pattern modifier functions locally | ||
nealrichardson marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| fixed <- function(pattern, ignore_case = FALSE, ...) { | ||
| check_dots(...) | ||
| list(pattern = pattern, fixed = TRUE, ignore_case = ignore_case) | ||
| } | ||
| regex <- function(pattern, ignore_case = FALSE, ...) { | ||
| check_dots(...) | ||
| list(pattern = pattern, fixed = FALSE, ignore_case = ignore_case) | ||
| } | ||
| coll <- boundary <- function(...) { | ||
| stop( | ||
| "Pattern modifier `", | ||
| match.call()[[1]], | ||
| "()` is not supported in Arrow", | ||
| call. = FALSE | ||
| ) | ||
| } | ||
| check_dots <- function(...) { | ||
| dots <- list(...) | ||
| if (length(dots)) { | ||
| warning( | ||
| "Ignoring pattern modifier ", | ||
| ngettext(length(dots), "argument ", "arguments "), | ||
| "not supported in Arrow: ", | ||
| oxford_paste(names(dots)), | ||
| call. = FALSE | ||
| ) | ||
| } | ||
| } | ||
| ensure_opts <- function(opts) { | ||
| if (is.character(opts)) { | ||
| opts <- list(pattern = opts, fixed = TRUE, ignore_case = FALSE) | ||
| } | ||
| opts | ||
| } | ||
| opts <- ensure_opts(eval(enexpr(pattern))) | ||
| arrow_r_string_replace_function(FUN, max_replacements)( | ||
| pattern = opts$pattern, | ||
| replacement = replacement, | ||
| x = string, | ||
| ignore.case = opts$ignore_case, | ||
| fixed = opts$fixed | ||
| ) | ||
| } | ||
| } | ||
| # We'll populate these at package load time. | ||
| dplyr_functions <- NULL | ||
| init_env <- function () { | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,180 @@ | ||
| # Licensed to the Apache Software Foundation (ASF) under one | ||
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The ASF licenses this file | ||
| # to you under the Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, | ||
| # software distributed under the License is distributed on an | ||
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
| library(dplyr) | ||
| library(stringr) | ||
| test_that("sub and gsub", { | ||
| df <- tibble(x = c("Foo", "bar")) | ||
| for(fixed in c(TRUE, FALSE)) { | ||
| expect_dplyr_equal( | ||
| input %>% | ||
| transmute(x = sub("Foo", "baz", x, fixed = fixed)) %>% | ||
| collect(), | ||
| df | ||
| ) | ||
| expect_dplyr_equal( | ||
| input %>% | ||
| transmute(x = sub("^B.+", "baz", x, ignore.case = FALSE, fixed = fixed)) %>% | ||
| collect(), | ||
| df | ||
| ) | ||
| expect_dplyr_equal( | ||
| input %>% | ||
| transmute(x = sub("Foo", "baz", x, ignore.case = FALSE, fixed = fixed)) %>% | ||
| collect(), | ||
| df | ||
| ) | ||
| } | ||
| }) | ||
ianmcook marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| test_that("sub and gsub with ignore.case = TRUE and fixed = TRUE", { | ||
| df <- tibble(x = c("Foo", "bar")) | ||
| # base::sub() and base::gsub() ignore ignore.case = TRUE with a warning when | ||
| # fixed = TRUE, so we can't use expect_dplyr_equal() for these tests | ||
| expect_equal( | ||
| df %>% | ||
| Table$create() %>% | ||
| transmute(x = sub("O", "u", x, ignore.case = TRUE, fixed = TRUE)) %>% | ||
| collect(), | ||
| tibble(x = c("Fuo", "bar")) | ||
| ) | ||
| expect_equal( | ||
| df %>% | ||
| Table$create() %>% | ||
| transmute(x = gsub("o", "u", x, ignore.case = TRUE, fixed = TRUE)) %>% | ||
| collect(), | ||
| tibble(x = c("Fuu", "bar")) | ||
| ) | ||
| expect_equal( | ||
| df %>% | ||
| Table$create() %>% | ||
| transmute(x = sub("^B.+", "baz", x, ignore.case = TRUE, fixed = TRUE)) %>% | ||
| collect(), | ||
| df # unchanged | ||
| ) | ||
| }) | ||
| test_that("str_replace and str_replace_all", { | ||
| df <- tibble(x = c("Foo", "bar")) | ||
| library(stringr) | ||
| expect_dplyr_equal( | ||
| input %>% | ||
| transmute(x = str_replace_all(x, regex("^F"), "baz")) %>% | ||
| collect(), | ||
| df | ||
| ) | ||
| expect_dplyr_equal( | ||
| input %>% | ||
| transmute(x = str_replace(x, regex("^f[A-Z]{2}", ignore_case = TRUE), "baz")) %>% | ||
| collect(), | ||
| df | ||
| ) | ||
| expect_dplyr_equal( | ||
| input %>% | ||
| transmute(x = str_replace_all(x, fixed("o"), "u")) %>% | ||
| collect(), | ||
| df | ||
| ) | ||
| expect_dplyr_equal( | ||
| input %>% | ||
| transmute(x = str_replace(x, fixed("O"), "u")) %>% | ||
| collect(), | ||
| df | ||
| ) | ||
| expect_dplyr_equal( | ||
| input %>% | ||
| transmute(x = str_replace(x, fixed("O", ignore_case = TRUE), "u")) %>% | ||
| collect(), | ||
| df | ||
| ) | ||
| }) | ||
| test_that("backreferences", { | ||
| df <- tibble(x = c("Foo", "bar")) | ||
| expect_dplyr_equal( | ||
| input %>% | ||
| transmute(desc = sub( | ||
| "(?:https?|ftp)://([^/\r\n]+)(/[^\r\n]*)?", | ||
| "path `\\2` on server `\\1`", | ||
| url | ||
| ) | ||
| ) %>% | ||
| collect(), | ||
| tibble(url = "https://arrow.apache.org/docs/r/") | ||
| ) | ||
| expect_dplyr_equal( | ||
| input %>% | ||
| transmute(x = str_replace(x, regex("^(\\w)o(.*)", ignore_case = TRUE), "\\1\\2p")) %>% | ||
| collect(), | ||
| df | ||
| ) | ||
| }) | ||
| test_that("edge cases", { | ||
ianmcook marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| # in case-insensitive fixed replace, test that "\\E" in the search string and | ||
| # backslashes in the replacement string are interpreted literally. | ||
| # this test does not use expect_dplyr_equal() because base::sub() does not | ||
| # support ignore.case = TRUE when fixed = TRUE. | ||
| expect_equal( | ||
| tibble(x = c("\\Q\\e\\D")) %>% | ||
| Table$create() %>% | ||
| transmute(x = sub("\\E", "\\L", x, ignore.case = TRUE, fixed = TRUE)) %>% | ||
| collect(), | ||
| tibble(x = c("\\Q\\L\\D")) | ||
| ) | ||
| # test that a user's "(?i)" prefix does not break the "(?i)" prefix that's | ||
| # added in case-insensitive regex replace | ||
| expect_dplyr_equal( | ||
| input %>% | ||
| transmute(x = sub("(?i)^[abc]{3}$", "123", x, ignore.case = TRUE, fixed = FALSE)) %>% | ||
| collect(), | ||
| tibble(x = c("ABC")) | ||
| ) | ||
| }) | ||
| test_that("errors and warnings", { | ||
| df <- tibble(x = c("Foo", "bar")) | ||
| # This condition generates an error, but abandon_ship() catches the error, | ||
| # issues a warning, and pulls the data into R | ||
| expect_warning( | ||
| df %>% | ||
| Table$create() %>% | ||
| mutate(x = str_replace_all(x, coll("o", locale = "en"), "ó")) %>% | ||
| collect(), | ||
| "not supported" | ||
ianmcook marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| ) | ||
| expect_warning( | ||
| df %>% | ||
| Table$create() %>% | ||
| transmute(x = str_replace_all(x, regex("o", multiline = TRUE), "u")), | ||
| "Ignoring pattern modifier argument not supported in Arrow: \"multiline\"" | ||
| ) | ||
| }) | ||
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.