From f555ba451ed9d5fb5fc0390fa445cee82cb0a388 Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Thu, 15 Aug 2024 19:21:43 +0100 Subject: [PATCH 01/10] add sep argument to str_dup closes #487 Adds a sep argument to str_dup. Becasue this is not supported by stri_dup, do separately and use lapply to go through strings. --- R/dup.R | 19 +++++++++++++++++-- man/str_dup.Rd | 4 +++- tests/testthat/test-dup.R | 17 +++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/R/dup.R b/R/dup.R index d90e62fb..c9f6f8fe 100644 --- a/R/dup.R +++ b/R/dup.R @@ -5,6 +5,7 @@ #' #' @inheritParams str_detect #' @param times Number of times to duplicate each string. +#' @param sep String to insert between each duplicate. #' @return A character vector the same length as `string`/`times`. #' @export #' @examples @@ -12,7 +13,21 @@ #' str_dup(fruit, 2) #' str_dup(fruit, 1:3) #' str_c("ba", str_dup("na", 0:5)) -str_dup <- function(string, times) { +str_dup <- function(string, times, sep = NULL) { vctrs::vec_size_common(string = string, times = times) - stri_dup(string, times) + if(is.null(sep)){ + stri_dup(string, times) + } else { + # stri_dup does not currently support sep + check_string(sep) + lapply(seq_along(string), function(i) { + if (length(times) == 1) { + paste(rep(string[i], times), collapse = sep) + } else { + paste(rep(string[i], times[i]), collapse = sep) + } + }) |> + rlang::flatten_chr() + } + } diff --git a/man/str_dup.Rd b/man/str_dup.Rd index fb722f2c..a2e2e162 100644 --- a/man/str_dup.Rd +++ b/man/str_dup.Rd @@ -4,13 +4,15 @@ \alias{str_dup} \title{Duplicate a string} \usage{ -str_dup(string, times) +str_dup(string, times, sep = NULL) } \arguments{ \item{string}{Input vector. Either a character vector, or something coercible to one.} \item{times}{Number of times to duplicate each string.} + +\item{sep}{String to insert between each duplicate.} } \value{ A character vector the same length as \code{string}/\code{times}. diff --git a/tests/testthat/test-dup.R b/tests/testthat/test-dup.R index 2872ae37..108a0307 100644 --- a/tests/testthat/test-dup.R +++ b/tests/testthat/test-dup.R @@ -13,3 +13,20 @@ test_that("0 duplicates equals empty string", { test_that("uses tidyverse recycling rules", { expect_error(str_dup(1:2, 1:3), class = "vctrs_error_incompatible_size") }) + +test_that("uses sep argument", { + expect_equal(str_dup("a", 3, sep = ";"), "a;a;a") + expect_equal(str_dup("abc", 2, sep = "-"), "abc-abc") + expect_equal(str_dup("a", 3, sep = ";"), "a;a;a") + expect_equal(str_dup("abc", 2, sep = "-"), "abc-abc") + expect_equal(str_dup(c("a", "b"), 2, sep = "x"), c("axa", "bxb")) + expect_equal(str_dup(c("aa", "bb", "ccc"), c(2, 3, 4), sep = ";"), + c("aa;aa", "bb;bb;bb", "ccc;ccc;ccc;ccc")) + + expect_error(str_dup("a", 3, sep = 1)) + expect_error(str_dup("a", 3, sep = c("-", ";"))) + expect_error(str_dup(c("aa", "bb"), c(2, 3, 4), sep = ";")) + expect_error(str_dup(c("aa", "bb", "cc"), c(2, 3), sep = ";")) + +}) + From 6c40b757154fb9ea78c5da88865874c7896eb291 Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Thu, 15 Aug 2024 19:24:23 +0100 Subject: [PATCH 02/10] update news --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 365fd68b..e24fa4d4 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # stringr (development version) +* Add sep argument to `str_dup()` (@edward-burn, #564). + * In `str_replace_all()`, a `replacement` function now receives all values in a single vector. This radically improves performance at the cost of breaking some existing uses (#462). From 342508d886bf4a3f8bde9ab7a17b48aa20f9c09d Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Thu, 15 Aug 2024 19:25:52 +0100 Subject: [PATCH 03/10] news --- NEWS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index e24fa4d4..7898f266 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,7 @@ # stringr (development version) -* Add sep argument to `str_dup()` (@edward-burn, #564). +* Add sep argument to `str_dup()` so that it is possible to repeat a string and + add a separator between every repeated value. (@edward-burn, #564). * In `str_replace_all()`, a `replacement` function now receives all values in a single vector. This radically improves performance at the cost of breaking From a4b45b897d6d74b02b927538745af2f8151470ad Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Thu, 15 Aug 2024 20:45:35 +0100 Subject: [PATCH 04/10] magrittr pipe --- R/dup.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/dup.R b/R/dup.R index c9f6f8fe..9420d636 100644 --- a/R/dup.R +++ b/R/dup.R @@ -26,7 +26,7 @@ str_dup <- function(string, times, sep = NULL) { } else { paste(rep(string[i], times[i]), collapse = sep) } - }) |> + }) %>% rlang::flatten_chr() } From efefb586de1afd8b02128af9fd1e6e543eed655d Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Thu, 15 Aug 2024 21:56:45 +0100 Subject: [PATCH 05/10] updates after review --- NEWS.md | 4 ++-- R/dup.R | 26 ++++++++++++++------------ tests/testthat/test-dup.R | 13 ++++++------- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/NEWS.md b/NEWS.md index 7898f266..9f575079 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,7 @@ # stringr (development version) -* Add sep argument to `str_dup()` so that it is possible to repeat a string and - add a separator between every repeated value. (@edward-burn, #564). +* Add `sep` argument to `str_dup()` so that it is possible to repeat a string and + add a separator between every repeated value (@edward-burn, #564). * In `str_replace_all()`, a `replacement` function now receives all values in a single vector. This radically improves performance at the cost of breaking diff --git a/R/dup.R b/R/dup.R index 9420d636..93a1d724 100644 --- a/R/dup.R +++ b/R/dup.R @@ -14,20 +14,22 @@ #' str_dup(fruit, 1:3) #' str_c("ba", str_dup("na", 0:5)) str_dup <- function(string, times, sep = NULL) { - vctrs::vec_size_common(string = string, times = times) - if(is.null(sep)){ + size <- vctrs::vec_size_common(string = string, times = times) + if (is.null(sep)) { stri_dup(string, times) } else { - # stri_dup does not currently support sep - check_string(sep) - lapply(seq_along(string), function(i) { - if (length(times) == 1) { - paste(rep(string[i], times), collapse = sep) - } else { - paste(rep(string[i], times[i]), collapse = sep) - } - }) %>% - rlang::flatten_chr() + # stri_dup does not currently support sep + check_string(sep) + lapply(seq_along(string), function(i) { + if (size == 1) { + paste(rep(string[i], times), collapse = sep) + } else { + paste(rep(string[i], times[i]), collapse = sep) + } + }) %>% + rlang::flatten_chr() } } + + diff --git a/tests/testthat/test-dup.R b/tests/testthat/test-dup.R index 108a0307..d4b831c9 100644 --- a/tests/testthat/test-dup.R +++ b/tests/testthat/test-dup.R @@ -15,18 +15,17 @@ test_that("uses tidyverse recycling rules", { }) test_that("uses sep argument", { - expect_equal(str_dup("a", 3, sep = ";"), "a;a;a") - expect_equal(str_dup("abc", 2, sep = "-"), "abc-abc") - expect_equal(str_dup("a", 3, sep = ";"), "a;a;a") expect_equal(str_dup("abc", 2, sep = "-"), "abc-abc") expect_equal(str_dup(c("a", "b"), 2, sep = "x"), c("axa", "bxb")) expect_equal(str_dup(c("aa", "bb", "ccc"), c(2, 3, 4), sep = ";"), c("aa;aa", "bb;bb;bb", "ccc;ccc;ccc;ccc")) - expect_error(str_dup("a", 3, sep = 1)) - expect_error(str_dup("a", 3, sep = c("-", ";"))) - expect_error(str_dup(c("aa", "bb"), c(2, 3, 4), sep = ";")) - expect_error(str_dup(c("aa", "bb", "cc"), c(2, 3), sep = ";")) + expect_identical(character(), str_dup(character(), 1, sep = ":")) + + expect_snapshot(str_dup("a", 3, sep = 1), error = TRUE) + expect_snapshot(str_dup("a", 3, sep = c("-", ";")), error = TRUE) + expect_snapshot(str_dup(c("aa", "bb"), c(2, 3, 4), sep = ";"), error = TRUE) + expect_snapshot(str_dup(c("aa", "bb", "cc"), c(2, 3), sep = ";"), error = TRUE) }) From 1834be3603b12c89659454e4d271a138d0d3b9c4 Mon Sep 17 00:00:00 2001 From: edward-burn <9583964+edward-burn@users.noreply.github.com> Date: Thu, 15 Aug 2024 22:04:32 +0100 Subject: [PATCH 06/10] size of times --- R/dup.R | 4 ++-- tests/testthat/_snaps/dup.md | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 tests/testthat/_snaps/dup.md diff --git a/R/dup.R b/R/dup.R index 93a1d724..da0af901 100644 --- a/R/dup.R +++ b/R/dup.R @@ -14,14 +14,14 @@ #' str_dup(fruit, 1:3) #' str_c("ba", str_dup("na", 0:5)) str_dup <- function(string, times, sep = NULL) { - size <- vctrs::vec_size_common(string = string, times = times) + vctrs::vec_size_common(string = string, times = times) if (is.null(sep)) { stri_dup(string, times) } else { # stri_dup does not currently support sep check_string(sep) lapply(seq_along(string), function(i) { - if (size == 1) { + if (vctrs::vec_size(times) == 1) { paste(rep(string[i], times), collapse = sep) } else { paste(rep(string[i], times[i]), collapse = sep) diff --git a/tests/testthat/_snaps/dup.md b/tests/testthat/_snaps/dup.md new file mode 100644 index 00000000..25c4d11b --- /dev/null +++ b/tests/testthat/_snaps/dup.md @@ -0,0 +1,32 @@ +# uses sep argument + + Code + str_dup("a", 3, sep = 1) + Condition + Error in `str_dup()`: + ! `sep` must be a single string, not the number 1. + +--- + + Code + str_dup("a", 3, sep = c("-", ";")) + Condition + Error in `str_dup()`: + ! `sep` must be a single string, not a character vector. + +--- + + Code + str_dup(c("aa", "bb"), c(2, 3, 4), sep = ";") + Condition + Error in `str_dup()`: + ! Can't recycle `string` (size 2) to match `times` (size 3). + +--- + + Code + str_dup(c("aa", "bb", "cc"), c(2, 3), sep = ";") + Condition + Error in `str_dup()`: + ! Can't recycle `string` (size 3) to match `times` (size 2). + From b95ee123015d58605da23295027a83710be5e6f6 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Thu, 15 Aug 2024 14:17:41 -0700 Subject: [PATCH 07/10] Polish implementation a little --- R/dup.R | 22 +++++++--------------- tests/testthat/_snaps/dup.md | 4 ++-- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/R/dup.R b/R/dup.R index da0af901..2cf3ddab 100644 --- a/R/dup.R +++ b/R/dup.R @@ -14,22 +14,14 @@ #' str_dup(fruit, 1:3) #' str_c("ba", str_dup("na", 0:5)) str_dup <- function(string, times, sep = NULL) { - vctrs::vec_size_common(string = string, times = times) + input <- vctrs::vec_recycle_common(string = string, times = times) + check_string(sep, allow_null = TRUE) + if (is.null(sep)) { - stri_dup(string, times) + stri_dup(input$string, input$times) } else { - # stri_dup does not currently support sep - check_string(sep) - lapply(seq_along(string), function(i) { - if (vctrs::vec_size(times) == 1) { - paste(rep(string[i], times), collapse = sep) - } else { - paste(rep(string[i], times[i]), collapse = sep) - } - }) %>% - rlang::flatten_chr() + map_chr(seq_along(input$string), function(i) { + paste(rep(string[i], input$times[i]), collapse = sep) + }) } - } - - diff --git a/tests/testthat/_snaps/dup.md b/tests/testthat/_snaps/dup.md index 25c4d11b..ded4842e 100644 --- a/tests/testthat/_snaps/dup.md +++ b/tests/testthat/_snaps/dup.md @@ -4,7 +4,7 @@ str_dup("a", 3, sep = 1) Condition Error in `str_dup()`: - ! `sep` must be a single string, not the number 1. + ! `sep` must be a single string or `NULL`, not the number 1. --- @@ -12,7 +12,7 @@ str_dup("a", 3, sep = c("-", ";")) Condition Error in `str_dup()`: - ! `sep` must be a single string, not a character vector. + ! `sep` must be a single string or `NULL`, not a character vector. --- From b4e5e3a8ed35b5e3f1b433d3d80cd5d547ae492f Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Thu, 15 Aug 2024 14:21:11 -0700 Subject: [PATCH 08/10] Polish tests --- tests/testthat/_snaps/dup.md | 21 +-------------------- tests/testthat/test-dup.R | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 29 deletions(-) diff --git a/tests/testthat/_snaps/dup.md b/tests/testthat/_snaps/dup.md index ded4842e..08a752c2 100644 --- a/tests/testthat/_snaps/dup.md +++ b/tests/testthat/_snaps/dup.md @@ -1,32 +1,13 @@ -# uses sep argument +# separator must be a single string Code str_dup("a", 3, sep = 1) Condition Error in `str_dup()`: ! `sep` must be a single string or `NULL`, not the number 1. - ---- - Code str_dup("a", 3, sep = c("-", ";")) Condition Error in `str_dup()`: ! `sep` must be a single string or `NULL`, not a character vector. ---- - - Code - str_dup(c("aa", "bb"), c(2, 3, 4), sep = ";") - Condition - Error in `str_dup()`: - ! Can't recycle `string` (size 2) to match `times` (size 3). - ---- - - Code - str_dup(c("aa", "bb", "cc"), c(2, 3), sep = ";") - Condition - Error in `str_dup()`: - ! Can't recycle `string` (size 3) to match `times` (size 2). - diff --git a/tests/testthat/test-dup.R b/tests/testthat/test-dup.R index d4b831c9..169f9cd0 100644 --- a/tests/testthat/test-dup.R +++ b/tests/testthat/test-dup.R @@ -15,17 +15,19 @@ test_that("uses tidyverse recycling rules", { }) test_that("uses sep argument", { + expect_equal(str_dup("abc", 1, sep = "-"), "abc") expect_equal(str_dup("abc", 2, sep = "-"), "abc-abc") - expect_equal(str_dup(c("a", "b"), 2, sep = "x"), c("axa", "bxb")) - expect_equal(str_dup(c("aa", "bb", "ccc"), c(2, 3, 4), sep = ";"), - c("aa;aa", "bb;bb;bb", "ccc;ccc;ccc;ccc")) - expect_identical(character(), str_dup(character(), 1, sep = ":")) - - expect_snapshot(str_dup("a", 3, sep = 1), error = TRUE) - expect_snapshot(str_dup("a", 3, sep = c("-", ";")), error = TRUE) - expect_snapshot(str_dup(c("aa", "bb"), c(2, 3, 4), sep = ";"), error = TRUE) - expect_snapshot(str_dup(c("aa", "bb", "cc"), c(2, 3), sep = ";"), error = TRUE) + expect_equal(str_dup(c("a", "b"), 2, sep = "-"), c("a-a", "b-b")) + expect_equal(str_dup(c("a", "b"), c(1, 2), sep = "-"), c("a", "b-b")) + expect_equal(str_dup(character(), 1, sep = ":"), character()) + expect_equal(str_dup(character(), 2, sep = ":"), character()) }) +test_that("separator must be a single string", { + expect_snapshot(error = TRUE, { + str_dup("a", 3, sep = 1) + str_dup("a", 3, sep = c("-", ";")) + }) +}) From 244376b9049dd6aef41d23b4b9c6012917015f53 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Thu, 15 Aug 2024 14:24:13 -0700 Subject: [PATCH 09/10] Use same sep --- tests/testthat/test-dup.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-dup.R b/tests/testthat/test-dup.R index 169f9cd0..ca7ddbb9 100644 --- a/tests/testthat/test-dup.R +++ b/tests/testthat/test-dup.R @@ -21,8 +21,8 @@ test_that("uses sep argument", { expect_equal(str_dup(c("a", "b"), 2, sep = "-"), c("a-a", "b-b")) expect_equal(str_dup(c("a", "b"), c(1, 2), sep = "-"), c("a", "b-b")) - expect_equal(str_dup(character(), 1, sep = ":"), character()) - expect_equal(str_dup(character(), 2, sep = ":"), character()) + expect_equal(str_dup(character(), 1, sep = "-"), character()) + expect_equal(str_dup(character(), 2, sep = "-"), character()) }) test_that("separator must be a single string", { From 089abeae11332110c3d277b9cf4425a0500acd4e Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 20 Aug 2024 14:39:07 -0500 Subject: [PATCH 10/10] Add example & use `[[` --- R/dup.R | 3 ++- man/str_dup.Rd | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/R/dup.R b/R/dup.R index 2cf3ddab..3a82ffe0 100644 --- a/R/dup.R +++ b/R/dup.R @@ -11,6 +11,7 @@ #' @examples #' fruit <- c("apple", "pear", "banana") #' str_dup(fruit, 2) +#' str_dup(fruit, 2, sep = " ") #' str_dup(fruit, 1:3) #' str_c("ba", str_dup("na", 0:5)) str_dup <- function(string, times, sep = NULL) { @@ -21,7 +22,7 @@ str_dup <- function(string, times, sep = NULL) { stri_dup(input$string, input$times) } else { map_chr(seq_along(input$string), function(i) { - paste(rep(string[i], input$times[i]), collapse = sep) + paste(rep(string[[i]], input$times[[i]]), collapse = sep) }) } } diff --git a/man/str_dup.Rd b/man/str_dup.Rd index a2e2e162..48a7867b 100644 --- a/man/str_dup.Rd +++ b/man/str_dup.Rd @@ -24,6 +24,7 @@ A character vector the same length as \code{string}/\code{times}. \examples{ fruit <- c("apple", "pear", "banana") str_dup(fruit, 2) +str_dup(fruit, 2, sep = " ") str_dup(fruit, 1:3) str_c("ba", str_dup("na", 0:5)) }