Skip to content
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.

Sign up for GitHub

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

Jump to bottom

GH-39191: [R] throw error when string_replace is passed vector of values in pattern #39219

Merged
merged 7 commits into from
Dec 19, 2023
Merged

GH-39191: [R] throw error when string_replace is passed vector of values in pattern #39219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0db8c63
add error and tests (failing)
abfleishman Dec 13, 2023
7e8df1d
remove test that doesn't actualy test for something I am fixing; use …
abfleishman Dec 14, 2023
223602f
remove comment; add test for replacement length 2
abfleishman Dec 14, 2023
5efb699
remove check of replacement
abfleishman Dec 14, 2023
fd05f07
add error and tests for if replacement is >1 length; use `styler::sty…
abfleishman Dec 18, 2023
b094f69
styler::style_file
abfleishman Dec 18, 2023
db2ee6a
use Expression$field_ref("x") in tests
abfleishman Dec 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion r/R/dplyr-funcs-string.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ get_stringr_pattern_options <- function(pattern) {
}

ensure_opts <- function(opts) {

# default options for the simple cases
if (is.character(opts)) {
opts <- list(pattern = opts, fixed = FALSE, ignore_case = FALSE)
Expand Down Expand Up @@ -352,6 +351,12 @@ register_bindings_string_regex <- function() {
# Encapsulate some common logic for sub/gsub/str_replace/str_replace_all
arrow_r_string_replace_function <- function(max_replacements) {
function(pattern, replacement, x, ignore.case = FALSE, fixed = FALSE) {
if (length(pattern) != 1) {
stop("`pattern` must be a length 1 character vector")
}
if (length(replacement) != 1) {
stop("`replacement` must be a length 1 character vector")
}
Expression$create(
ifelse(fixed && !ignore.case, "replace_substring", "replace_substring_regex"),
x,
Expand Down
17 changes: 17 additions & 0 deletions r/tests/testthat/test-dplyr-funcs-string.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,23 @@ test_that("sub and gsub with namespacing", {
})

test_that("str_replace and str_replace_all", {
x <- Expression$field_ref("x")

expect_error(
call_binding("str_replace_all", x, c("F" = "_", "b" = "")),
regexp = "`pattern` must be a length 1 character vector"
)

expect_error(
call_binding("str_replace_all", x, c("F", "b"), c("_", "")),
regexp = "`pattern` must be a length 1 character vector"
)

expect_error(
call_binding("str_replace_all", x, c("F"), c("_", "")),
regexp = "`replacement` must be a length 1 character vector"
)

df <- tibble(x = c("Foo", "bar"))

compare_dplyr_binding(
Expand Down
Loading