diff --git a/NEWS.md b/NEWS.md index 74d226027..bbf9ae702 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # vctrs (development version) +* New `.name_spec = "inner"` option for `vec_c()`, `list_unchop()`, and `vec_rbind()`. This efficiently ignores all outer names, while retaining any inner names (#1988). + * `list_unchop()` now works in an edge case with a single `NA` recycled to size 0 (#1989). * `list_unchop()` has gained new `size`, `default`, and `unmatched` arguments (#1982). diff --git a/R/bind.R b/R/bind.R index 296a689e3..f6117dc0f 100644 --- a/R/bind.R +++ b/R/bind.R @@ -44,16 +44,18 @@ #' #' `NULL` inputs are silently ignored. Empty (e.g. zero row) inputs #' will not appear in the output, but will affect the derived `.ptype`. -#' @param .names_to -#' This controls what to do with input names supplied in `...`. -#' * By default, input names are [zapped][rlang::zap]. +#' @param .names_to This controls what to do with names on `...`: #' -#' * If a string, specifies a column where the input names will be +#' * By default, names on `...` are [zapped][rlang::zap] and do not appear +#' anywhere in the output. +#' +#' * If a string, specifies a column where the names on `...` will be #' copied. These names are often useful to identify rows with #' their original input. If a column name is supplied and `...` is #' not named, an integer column is used instead. #' -#' * If `NULL`, the input names are used as row names. +#' * If `NULL`, the outer names on `...` are instead merged with inner +#' row names on each element of `...` and are subject to `.name_spec`. #' @param .name_repair One of `"unique"`, `"universal"`, `"check_unique"`, #' `"unique_quiet"`, or `"universal_quiet"`. See [vec_as_names()] for the #' meaning of these options. @@ -167,11 +169,13 @@ NULL #' @export -#' @param .name_spec A name specification (as documented in [vec_c()]) -#' for combining the outer inputs names in `...` and the inner row -#' names of the inputs. This only has an effect when `.names_to` is -#' set to `NULL`, which causes the input names to be assigned as row -#' names. +#' @param .name_spec A name specification (as documented in [vec_c()]) for +#' combining the outer names on `...` with the inner row names of each element +#' of `...`. An outer name will only ever be provided when `.names_to` is set +#' to `NULL`, which causes the outer name to be used as part of the row names +#' rather than as a new column, but it can still be useful to fix this to +#' either [rlang::zap()] to always ignore all names, or `"inner"` to always +#' ignore outer names, regardless of `.names_to`. #' @rdname vec_bind vec_rbind <- function(..., .ptype = NULL, diff --git a/R/names.R b/R/names.R index 6a5755f15..8f52e9b1c 100644 --- a/R/names.R +++ b/R/names.R @@ -552,6 +552,10 @@ vec_as_names_legacy <- function(names, prefix = "V", sep = "") { #' * An anonymous function as a purrr-style formula. #' #' * A glue specification of the form `"{outer}_{inner}"`. +#' +#' * `"inner"`, in which case outer names are ignored, and inner +#' names are used if they exist. Note that outer names may still +#' be used to provide informative error messages. #' #' * An [rlang::zap()] object, in which case both outer and inner #' names are ignored and the result is unnamed. @@ -577,6 +581,16 @@ vec_as_names_legacy <- function(names, prefix = "V", sep = "") { #' #' # Or purrr-style formulas for anonymous functions: #' vec_c(name = 1:3, other = 4:5, .name_spec = ~ paste0(.x, .y)) +#' +#' # Or the string `"inner"` to only use inner names +#' vec_c(name = 1:3, outer = 4:5, .name_spec = "inner") +#' vec_c(name = c(a = 1, b = 2, c = 3), outer = 4:5, .name_spec = "inner") +#' # This can be useful when you want outer names mentioned in error messages, +#' # but you don't want them interfering with the result +#' try(vec_c(x = c(a = 1), y = c(b = "2"), .name_spec = "inner")) +#' +#' # Or `rlang::zap()` to ignore both outer and inner names entirely +#' vec_c(name = c(a = 1, b = 2), outer = c(c = 3), .name_spec = rlang::zap()) #' @name name_spec NULL diff --git a/man/name_spec.Rd b/man/name_spec.Rd index be6da2e35..fdd02aeda 100644 --- a/man/name_spec.Rd +++ b/man/name_spec.Rd @@ -16,6 +16,9 @@ string to the first argument, and the inner names or positions are passed as second argument. \item An anonymous function as a purrr-style formula. \item A glue specification of the form \code{"{outer}_{inner}"}. +\item \code{"inner"}, in which case outer names are ignored, and inner +names are used if they exist. Note that outer names may still +be used to provide informative error messages. \item An \code{\link[rlang:zap]{rlang::zap()}} object, in which case both outer and inner names are ignored and the result is unnamed. } @@ -68,4 +71,14 @@ vec_c(name = 1:3, other = 4:5, .name_spec = my_spec) # Or purrr-style formulas for anonymous functions: vec_c(name = 1:3, other = 4:5, .name_spec = ~ paste0(.x, .y)) + +# Or the string `"inner"` to only use inner names +vec_c(name = 1:3, outer = 4:5, .name_spec = "inner") +vec_c(name = c(a = 1, b = 2, c = 3), outer = 4:5, .name_spec = "inner") +# This can be useful when you want outer names mentioned in error messages, +# but you don't want them interfering with the result +try(vec_c(x = c(a = 1), y = c(b = "2"), .name_spec = "inner")) + +# Or `rlang::zap()` to ignore both outer and inner names entirely +vec_c(name = c(a = 1, b = 2), outer = c(c = 3), .name_spec = rlang::zap()) } diff --git a/man/vec_bind.Rd b/man/vec_bind.Rd index fff4ae2d7..f3e272c91 100644 --- a/man/vec_bind.Rd +++ b/man/vec_bind.Rd @@ -47,14 +47,16 @@ Alternatively, you can supply \code{.ptype} to give the output known type. If \code{getOption("vctrs.no_guessing")} is \code{TRUE} you must supply this value: this is a convenient way to make production code demand fixed types.} -\item{.names_to}{This controls what to do with input names supplied in \code{...}. +\item{.names_to}{This controls what to do with names on \code{...}: \itemize{ -\item By default, input names are \link[rlang:zap]{zapped}. -\item If a string, specifies a column where the input names will be +\item By default, names on \code{...} are \link[rlang:zap]{zapped} and do not appear +anywhere in the output. +\item If a string, specifies a column where the names on \code{...} will be copied. These names are often useful to identify rows with their original input. If a column name is supplied and \code{...} is not named, an integer column is used instead. -\item If \code{NULL}, the input names are used as row names. +\item If \code{NULL}, the outer names on \code{...} are instead merged with inner +row names on each element of \code{...} and are subject to \code{.name_spec}. }} \item{.name_repair}{One of \code{"unique"}, \code{"universal"}, \code{"check_unique"}, @@ -69,11 +71,13 @@ repair function after all inputs have been concatenated together in a final data frame. Hence \code{vec_cbind()} allows the more permissive minimal names repair.} -\item{.name_spec}{A name specification (as documented in \code{\link[=vec_c]{vec_c()}}) -for combining the outer inputs names in \code{...} and the inner row -names of the inputs. This only has an effect when \code{.names_to} is -set to \code{NULL}, which causes the input names to be assigned as row -names.} +\item{.name_spec}{A name specification (as documented in \code{\link[=vec_c]{vec_c()}}) for +combining the outer names on \code{...} with the inner row names of each element +of \code{...}. An outer name will only ever be provided when \code{.names_to} is set +to \code{NULL}, which causes the outer name to be used as part of the row names +rather than as a new column, but it can still be useful to fix this to +either \code{\link[rlang:zap]{rlang::zap()}} to always ignore all names, or \code{"inner"} to always +ignore outer names, regardless of \code{.names_to}.} \item{.error_call}{The execution environment of a currently running function, e.g. \code{caller_env()}. The function will be diff --git a/man/vec_c.Rd b/man/vec_c.Rd index 4f5d2df5f..b30392dc4 100644 --- a/man/vec_c.Rd +++ b/man/vec_c.Rd @@ -36,6 +36,9 @@ string to the first argument, and the inner names or positions are passed as second argument. \item An anonymous function as a purrr-style formula. \item A glue specification of the form \code{"{outer}_{inner}"}. +\item \code{"inner"}, in which case outer names are ignored, and inner +names are used if they exist. Note that outer names may still +be used to provide informative error messages. \item An \code{\link[rlang:zap]{rlang::zap()}} object, in which case both outer and inner names are ignored and the result is unnamed. } diff --git a/man/vec_chop.Rd b/man/vec_chop.Rd index 30eece16e..4b5a40984 100644 --- a/man/vec_chop.Rd +++ b/man/vec_chop.Rd @@ -88,6 +88,9 @@ string to the first argument, and the inner names or positions are passed as second argument. \item An anonymous function as a purrr-style formula. \item A glue specification of the form \code{"{outer}_{inner}"}. +\item \code{"inner"}, in which case outer names are ignored, and inner +names are used if they exist. Note that outer names may still +be used to provide informative error messages. \item An \code{\link[rlang:zap]{rlang::zap()}} object, in which case both outer and inner names are ignored and the result is unnamed. } diff --git a/man/vec_interleave.Rd b/man/vec_interleave.Rd index bd4ce678f..26148005c 100644 --- a/man/vec_interleave.Rd +++ b/man/vec_interleave.Rd @@ -35,6 +35,9 @@ string to the first argument, and the inner names or positions are passed as second argument. \item An anonymous function as a purrr-style formula. \item A glue specification of the form \code{"{outer}_{inner}"}. +\item \code{"inner"}, in which case outer names are ignored, and inner +names are used if they exist. Note that outer names may still +be used to provide informative error messages. \item An \code{\link[rlang:zap]{rlang::zap()}} object, in which case both outer and inner names are ignored and the result is unnamed. } diff --git a/man/vec_unchop.Rd b/man/vec_unchop.Rd index 7dbf303f9..d4f7dc987 100644 --- a/man/vec_unchop.Rd +++ b/man/vec_unchop.Rd @@ -45,6 +45,9 @@ string to the first argument, and the inner names or positions are passed as second argument. \item An anonymous function as a purrr-style formula. \item A glue specification of the form \code{"{outer}_{inner}"}. +\item \code{"inner"}, in which case outer names are ignored, and inner +names are used if they exist. Note that outer names may still +be used to provide informative error messages. \item An \code{\link[rlang:zap]{rlang::zap()}} object, in which case both outer and inner names are ignored and the result is unnamed. } diff --git a/src/bind.c b/src/bind.c index e13cb4bbf..f22cf0752 100644 --- a/src/bind.c +++ b/src/bind.c @@ -94,12 +94,6 @@ r_obj* vec_rbind(r_obj* xs, r_ssize names_to_loc = 0; if (has_names_to) { - if (!assign_names) { - r_abort_lazy_call(error_call, - "Can't zap outer names when %s is supplied.", - r_c_str_format_error_arg(".names_to")); - } - r_obj* ptype_nms = KEEP(r_names(ptype)); names_to_loc = r_chr_find(ptype_nms, names_to); FREE(1); diff --git a/src/c.c b/src/c.c index 51f56f96b..2dbb5c798 100644 --- a/src/c.c +++ b/src/c.c @@ -320,6 +320,21 @@ r_obj* vec_c_fallback_invoke(r_obj* xs, r_chr_get_c_string(r_class(x), 0)); } + if (name_spec_is_inner(name_spec)) { + // We don't support most `name_spec` options in the fallback, + // but we do allow this one because it is extremely useful + // and easy to implement + name_spec = r_null; + + if (r_names(xs) != r_null) { + // Remove outer names, but remember we likely don't own `xs`! + xs = KEEP(r_clone_referenced(xs)); + r_attrib_poke_names(xs, r_null); + FREE(1); + } + } + KEEP(xs); + int err_type = vec_c_fallback_validate_args(x, name_spec); if (err_type) { stop_vec_c_fallback(xs, err_type, error_call); @@ -328,7 +343,7 @@ r_obj* vec_c_fallback_invoke(r_obj* xs, r_obj* ffi_call = KEEP(r_call2(r_sym("base_c_invoke"), xs)); r_obj* out = r_eval(ffi_call, vctrs_ns_env); - FREE(1); + FREE(2); return out; } diff --git a/src/names.c b/src/names.c index 5f0890849..e38341a36 100644 --- a/src/names.c +++ b/src/names.c @@ -552,6 +552,9 @@ r_obj* apply_name_spec(r_obj* name_spec, r_obj* outer, r_obj* inner, r_ssize n) if (r_inherits(name_spec, "rlang_zap")) { return r_null; } + if (name_spec_is_inner(name_spec)) { + return inner; + } if (outer == r_null) { return inner; @@ -633,6 +636,16 @@ r_obj* glue_as_name_spec(r_obj* spec) { syms_internal_spec, spec); } +bool name_spec_is_inner(r_obj* name_spec) { + if (!r_is_string(name_spec)) { + return false; + } + + const char* name_spec_c_string = r_chr_get_c_string(name_spec, 0); + + return !strcmp(name_spec_c_string, "inner"); +} + #define VCTRS_PASTE_BUFFER_MAX_SIZE 4096 char vctrs_paste_buffer[VCTRS_PASTE_BUFFER_MAX_SIZE]; diff --git a/src/names.h b/src/names.h index 7cb75ab13..1e413f940 100644 --- a/src/names.h +++ b/src/names.h @@ -14,6 +14,7 @@ r_obj* vec_unique_colnames(r_obj* x, bool quiet); r_obj* outer_names(r_obj* names, r_obj* outer, r_ssize n); r_obj* apply_name_spec(r_obj* name_spec, r_obj* outer, r_obj* inner, r_ssize n); +bool name_spec_is_inner(r_obj* name_spec); enum name_repair_type { NAME_REPAIR_none = 0, diff --git a/tests/testthat/_snaps/bind.md b/tests/testthat/_snaps/bind.md index 5ed40ae23..78d9fd817 100644 --- a/tests/testthat/_snaps/bind.md +++ b/tests/testthat/_snaps/bind.md @@ -402,22 +402,6 @@ Error in `vec_rbind()`: ! Can't combine `..1` and `..2` . -# can't zap names when `.names_to` is supplied - - Code - (expect_error(vec_rbind(foo = c(x = 1), .names_to = "id", .name_spec = zap()))) - Output - - Error in `vec_rbind()`: - ! Can't zap outer names when `.names_to` is supplied. - Code - (expect_error(vec_rbind(foo = c(x = 1), .names_to = "id", .name_spec = zap(), - .error_call = call("foo")))) - Output - - Error in `foo()`: - ! Can't zap outer names when `.names_to` is supplied. - # row-binding performs expected allocations Code diff --git a/tests/testthat/_snaps/c.md b/tests/testthat/_snaps/c.md index 7b98f68cc..5b58c6971 100644 --- a/tests/testthat/_snaps/c.md +++ b/tests/testthat/_snaps/c.md @@ -114,7 +114,7 @@ i The author of the class should implement vctrs methods. i See . -# vec_c() fallback doesn't support `name_spec` or `ptype` +# vec_c() fallback doesn't support (most) `name_spec` or `ptype` Code (expect_error(with_c_foobar(vec_c(foobar(1), foobar(2), .name_spec = "{outer}_{inner}")), @@ -152,6 +152,14 @@ Error in `vec_c()`: ! Can't combine `a` and `b` . +# can ignore outer names in `vec_c()` by providing an 'inner' name-spec (#1988) + + Code + vec_c(x = c(a = 1), y = c(b = "2"), .name_spec = "inner") + Condition + Error in `vec_c()`: + ! Can't combine `x` and `y` . + # concatenation performs expected allocations Code diff --git a/tests/testthat/_snaps/slice-chop.md b/tests/testthat/_snaps/slice-chop.md index 428bd96c0..52b0f0f3f 100644 --- a/tests/testthat/_snaps/slice-chop.md +++ b/tests/testthat/_snaps/slice-chop.md @@ -392,13 +392,13 @@ Error in `list_unchop()`: ! Can't combine `x[[1]]` and `x[[2]]` . -# list_unchop() fallback doesn't support `name_spec` or `ptype` +# list_unchop() fallback doesn't support (most) `name_spec` or `ptype` Code foo <- structure(foobar(1), foo = "foo") bar <- structure(foobar(2), bar = "bar") - (expect_error(with_c_foobar(list_unchop(list(foo, bar), name_spec = "{outer}_{inner}")), - "name specification")) + (expect_error(with_c_foobar(list_unchop(list(foo, bar), indices = list(1, 2), + name_spec = "{outer}_{inner}")), "name specification")) Output Error in `list_unchop()`: @@ -406,8 +406,9 @@ vctrs methods must be implemented for class `vctrs_foobar`. See . Code - (expect_error(with_c_foobar(list_unchop(list(foo, bar), name_spec = "{outer}_{inner}", - error_call = call("foo"))), "name specification")) + (expect_error(with_c_foobar(list_unchop(list(foo, bar), indices = list(1, 2), + name_spec = "{outer}_{inner}", error_call = call("foo"))), "name specification") + ) Output Error in `foo()`: @@ -416,8 +417,8 @@ See . Code x <- list(foobar(1)) - (expect_error(with_c_foobar(list_unchop(x, ptype = "")), class = "vctrs_error_incompatible_type") - ) + (expect_error(with_c_foobar(list_unchop(x, indices = list(1), ptype = "")), + class = "vctrs_error_incompatible_type")) Output Error in `list_unchop()`: @@ -478,6 +479,15 @@ Error in `list_unchop()`: ! Can't combine `x$a` and `x$b` . +# can ignore outer names in `list_unchop()` by providing a 'inner' name-spec (#1988) + + Code + list_unchop(list(x = c(a = 1), y = c(b = "2")), indices = list(1, 2), + name_spec = "inner") + Condition + Error in `list_unchop()`: + ! Can't combine `x$x` and `x$y` . + # list_unchop() fails if foreign classes are not homogeneous and there is no c() method Code diff --git a/tests/testthat/test-bind.R b/tests/testthat/test-bind.R index 3a08f956f..10352eb72 100644 --- a/tests/testthat/test-bind.R +++ b/tests/testthat/test-bind.R @@ -1056,20 +1056,45 @@ test_that("vec_rbind() zaps names when name-spec is zap() and names-to is NULL", ) }) -test_that("can't zap names when `.names_to` is supplied", { +test_that("can zap names even when `.names_to` is supplied", { expect_identical( vec_rbind(foo = c(x = 1), .names_to = zap(), .name_spec = zap()), data.frame(x = 1) ) + expect_identical( + vec_rbind(foo = data.frame(x = 1, row.names = "row"), .names_to = zap(), .name_spec = zap()), + data.frame(x = 1) + ) - expect_snapshot({ - (expect_error( - vec_rbind(foo = c(x = 1), .names_to = "id", .name_spec = zap()) - )) - (expect_error( - vec_rbind(foo = c(x = 1), .names_to = "id", .name_spec = zap(), .error_call = call("foo")) - )) - }) + # We previously didn't allow `.name_spec = zap()` when `.names_to = "id"`, + # but this does have a use case - zapping inner row names while also moving + # outer names into a new column + expect_identical( + vec_rbind(foo = c(x = 1), .names_to = "id", .name_spec = zap()), + data.frame(id = "foo", x = 1) + ) + expect_identical( + vec_rbind(foo = data.frame(x = 1, row.names = "row"), .names_to = "id", .name_spec = zap()), + data.frame(id = "foo", x = 1) + ) +}) + +test_that("can request 'inner' names when `.names_to` is supplied", { + # Note how it can be useful to lock `.name_spec` to `"inner"` in your API, but + # still expose `.names_to` to your users and allow all of its options. + # `purrr::list_rbind()` does this. + expect_identical( + vec_rbind(foo = data.frame(x = 1, row.names = "row"), .names_to = zap(), .name_spec = "inner"), + data.frame(x = 1, row.names = "row") + ) + expect_identical( + vec_rbind(foo = data.frame(x = 1, row.names = "row"), .names_to = "id", .name_spec = "inner"), + data.frame(id = "foo", x = 1, row.names = "row") + ) + expect_identical( + vec_rbind(foo = data.frame(x = 1, row.names = "row"), .names_to = NULL, .name_spec = "inner"), + data.frame(x = 1, row.names = "row") + ) }) test_that("can zap outer names from a name-spec (#1215)", { @@ -1085,6 +1110,16 @@ test_that("can zap outer names from a name-spec (#1215)", { vec_names(vec_rbind(a = df, df_named, .name_spec = zap_outer_spec)), c("...1", "...2", "foo") ) + + # These days it is more efficient to use a name-spec of "inner" (#1988) + expect_identical( + vec_rbind(a = df, .names_to = NULL, .name_spec = zap_outer_spec), + vec_rbind(a = df, .names_to = NULL, .name_spec = "inner") + ) + expect_identical( + vec_rbind(a = df, df_named, .name_spec = zap_outer_spec), + vec_rbind(a = df, df_named, .name_spec = "inner") + ) }) test_that("column names are treated consistently in vec_rbind()", { diff --git a/tests/testthat/test-c.R b/tests/testthat/test-c.R index dc1e69ea2..80a7f2959 100644 --- a/tests/testthat/test-c.R +++ b/tests/testthat/test-c.R @@ -120,6 +120,7 @@ test_that("can mix named and unnamed vectors (#271)", { test_that("preserves names when inputs are cast to a common type (#1690)", { expect_named(vec_c(c(a = 1), .ptype = integer()), "a") expect_named(vec_c(foo = c(a = 1), .ptype = integer(), .name_spec = "{outer}_{inner}"), "foo_a") + expect_named(vec_c(foo = c(a = 1), .ptype = integer(), .name_spec = "inner"), "a") }) test_that("vec_c() repairs names", { @@ -339,7 +340,7 @@ test_that("vec_c() falls back to c() if S4 method is available", { ) }) -test_that("vec_c() fallback doesn't support `name_spec` or `ptype`", { +test_that("vec_c() fallback doesn't support (most) `name_spec` or `ptype`", { expect_snapshot({ (expect_error( with_c_foobar(vec_c(foobar(1), foobar(2), .name_spec = "{outer}_{inner}")), @@ -363,6 +364,27 @@ test_that("vec_c() fallback doesn't support `name_spec` or `ptype`", { }) }) +test_that("vec_c() fallback does support `.name_spec = 'inner'`", { + # Because of how useful it is, and how easy it is to implement! + expect_identical( + with_c_foobar(vec_c(foobar(1), foobar(2), .name_spec = "inner")), + foobar(c(1, 2)) + ) + expect_identical( + with_c_foobar(vec_c(x = foobar(1), y = foobar(2), .name_spec = "inner")), + foobar(c(1, 2)) + ) + expect_identical( + with_c_foobar(vec_c( + x = foobar(c(a = 1)), + y = foobar(c(b = 2)), + z = foobar(3), + .name_spec = "inner" + )), + foobar(c(a = 1, b = 2, 3)) + ) +}) + test_that("vec_c() doesn't fall back when ptype2 is implemented", { new_quux <- function(x) structure(x, class = "vctrs_quux") @@ -427,6 +449,18 @@ test_that("can ignore names in `vec_c()` by providing a `zap()` name-spec (#232) }) }) +test_that("can ignore outer names in `vec_c()` by providing an 'inner' name-spec (#1988)", { + expect_identical( + vec_c(x = c(a = 1, 2), y = c(3, b = 4), .name_spec = "inner"), + c(a = 1, 2, 3, b = 4) + ) + + # Importantly, outer names are still used in error messages! + expect_snapshot(error = TRUE, { + vec_c(x = c(a = 1), y = c(b = "2"), .name_spec = "inner") + }) +}) + test_that("can concatenate subclasses of `vctrs_vctr` which don't have ptype2 methods", { x <- new_vctr(1, class = "vctrs_foo") expect_identical(vec_c(x, x), new_vctr(c(1, 1), class = "vctrs_foo")) @@ -475,6 +509,24 @@ test_that("can zap outer names from a name-spec (#1215)", { names(list_unchop(list(a = 1:2, c(foo = 3L)), indices = list(1:2, 3), name_spec = zap_outer_spec)), c("", "", "foo") ) + + # These days it is more efficient to use a name-spec of "inner" (#1988) + expect_identical( + vec_c(a = 1:2, .name_spec = zap_outer_spec), + vec_c(a = 1:2, .name_spec = "inner") + ) + expect_identical( + vec_c(a = 1:2, c(foo = 3L), .name_spec = zap_outer_spec), + vec_c(a = 1:2, c(foo = 3L), .name_spec = "inner") + ) + expect_identical( + list_unchop(list(a = 1:2), indices = list(1:2), name_spec = zap_outer_spec), + list_unchop(list(a = 1:2), indices = list(1:2), name_spec = "inner") + ) + expect_identical( + list_unchop(list(a = 1:2, c(foo = 3L)), indices = list(1:2, 3), name_spec = zap_outer_spec), + list_unchop(list(a = 1:2, c(foo = 3L)), indices = list(1:2, 3), name_spec = "inner") + ) }) test_that("named empty vectors force named output (#1263)", { diff --git a/tests/testthat/test-names.R b/tests/testthat/test-names.R index 908a1dda1..e411487d8 100644 --- a/tests/testthat/test-names.R +++ b/tests/testthat/test-names.R @@ -857,6 +857,11 @@ test_that("can pass glue string as name spec", { expect_error(vec_c(foo = c(a = 1, b = 2), .name_spec = c("a", "b")), "single string") }) +test_that("can pass 'inner' string as name spec", { + expect_named(vec_c(foo = c(a = 1, b = 2), .name_spec = "inner"), c("a", "b")) + expect_named(vec_c(foo = 1:2, .name_spec = "inner"), NULL) +}) + test_that("`outer` is recycled before name spec is invoked", { expect_identical(vec_c(outer = 1:2, .name_spec = "{outer}"), c(outer = 1L, outer = 2L)) }) diff --git a/tests/testthat/test-slice-chop.R b/tests/testthat/test-slice-chop.R index b0bf9540e..1ddb98734 100644 --- a/tests/testthat/test-slice-chop.R +++ b/tests/testthat/test-slice-chop.R @@ -742,6 +742,9 @@ test_that("preserves names when inputs are cast to a common type (#1689)", { c("foo_a", "b") ) + expect_named(list_unchop(list(foo = c(a = 1)), ptype = integer(), name_spec = "inner"), "a") + expect_named(list_unchop(list(foo = c(a = 1)), ptype = integer(), name_spec = "inner", indices = list(1)), "a") + # When `x` elements are recycled, names are also recycled x <- list(c(a = 1), c(b = 2)) indices <- list(1:2, 3:4) @@ -1182,28 +1185,69 @@ test_that("list_unchop() falls back for S4 classes with a registered c() method" ) }) -test_that("list_unchop() fallback doesn't support `name_spec` or `ptype`", { +test_that("list_unchop() fallback doesn't support (most) `name_spec` or `ptype`", { expect_snapshot({ foo <- structure(foobar(1), foo = "foo") bar <- structure(foobar(2), bar = "bar") (expect_error( - with_c_foobar(list_unchop(list(foo, bar), name_spec = "{outer}_{inner}")), + with_c_foobar(list_unchop( + list(foo, bar), + indices = list(1, 2), + name_spec = "{outer}_{inner}" + )), "name specification" )) # With error call (expect_error( - with_c_foobar(list_unchop(list(foo, bar), name_spec = "{outer}_{inner}", error_call = call("foo"))), + with_c_foobar(list_unchop( + list(foo, bar), + indices = list(1, 2), + name_spec = "{outer}_{inner}", + error_call = call("foo") + )), "name specification" )) # Used to be an error about `ptype` x <- list(foobar(1)) (expect_error( - with_c_foobar(list_unchop(x, ptype = "")), + with_c_foobar(list_unchop(x, indices = list(1), ptype = "")), class = "vctrs_error_incompatible_type" )) }) }) +test_that("list_unchop() fallback does support `name_spec = 'inner'`", { + # Because of how useful it is, and how easy it is to implement! + expect_identical( + with_c_foobar(list_unchop( + list(foobar(1), foobar(2)), + indices = list(1, 2), + name_spec = "inner" + )), + foobar(c(1, 2)) + ) + expect_identical( + with_c_foobar(list_unchop( + list(x = foobar(1), y = foobar(2)), + indices = list(1, 2), + name_spec = "inner" + )), + foobar(c(1, 2)) + ) + expect_identical( + with_c_foobar(list_unchop( + list( + x = foobar(c(a = 1)), + y = foobar(c(b = 2)), + z = foobar(3) + ), + indices = list(1, 2, 3), + name_spec = "inner" + )), + foobar(c(a = 1, b = 2, 3)) + ) +}) + test_that("list_unchop() supports numeric S3 indices", { local_methods( vec_ptype2.vctrs_foobar = function(x, y, ...) UseMethod("vec_ptype2.vctrs_foobar"), @@ -1275,6 +1319,26 @@ test_that("can ignore names in `list_unchop()` by providing a `zap()` name-spec }) }) +test_that("can ignore outer names in `list_unchop()` by providing a 'inner' name-spec (#1988)", { + expect_identical( + list_unchop( + list(x = c(a = 1, 2), y = c(3, b = 4)), + indices = list(c(3, 1), c(2, 4)), + name_spec = "inner" + ), + c(2, 3, a = 1, b = 4) + ) + + # Importantly, outer names are still used in error messages! + expect_snapshot(error = TRUE, { + list_unchop( + list(x = c(a = 1), y = c(b = "2")), + indices = list(1, 2), + name_spec = "inner" + ) + }) +}) + test_that("list_unchop() falls back to c() methods (#1120)", { expect_error( list_unchop(list(foobar(1), foobar(2, class = "foo"))),