Ignore names when an rlang::zap() is supplied as .name_spec (#1091)

Closes #232

Part of tidyverse/dplyr#5099

NEWS.md

@@ -190,6 +190,11 @@ The following errors are caused by breaking changes.
 
 * `new_data_frame()` infers size from row names when `n = NULL` (#894).
 
+* `vec_c()` now accepts `rlang::zap()` as `.name_spec` input. The
+  returned vector is then always unnamed, and the names do not cause
+  errors when they can't be combined. They are still used to create
+  more informative messages when the inputs have incompatible types (#232).
+
 
 ## Classes
 

R/c.R

@@ -7,7 +7,10 @@
 #' * `vec_ptype(vec_c(x, y)) == vec_ptype_common(x, y)`.
 #'
 #' @param ... Vectors to coerce.
-#' @param .name_repair How to repair names, see `repair` options in [vec_as_names()].
+#' @param .name_repair How to repair names, see `repair` options in
+#'   [vec_as_names()].  Can also be [rlang::zap()] to ignore names
+#'   during concatenation. The names are still used to give
+#'   informative error messages, e.g. with coercion errors.
 #' @return A vector with class given by `.ptype`, and length equal to the
 #'   sum of the `vec_size()` of the contents of `...`.
 #'

src/c.c

@@ -63,7 +63,8 @@ SEXP vec_c(SEXP xs,
   int* idx_ptr = INTEGER(idx);
 
   SEXP xs_names = PROTECT(r_names(xs));
-  bool has_names = xs_names != R_NilValue || list_has_inner_vec_names(xs, n);
+  bool assign_names = !Rf_inherits(name_spec, "rlang_zap");
+  bool has_names = assign_names && (xs_names != R_NilValue || list_has_inner_vec_names(xs, n));
   has_names = has_names && !is_data_frame(ptype);
 
   PROTECT_INDEX out_names_pi;
@@ -74,7 +75,7 @@ SEXP vec_c(SEXP xs,
   R_len_t counter = 0;
 
   const struct vec_assign_opts c_assign_opts = {
-    .assign_names = true
+    .assign_names = assign_names
   };
 
   for (R_len_t i = 0; i < n; ++i) {
@@ -83,7 +84,6 @@ SEXP vec_c(SEXP xs,
       continue;
     }
 
-    // TODO
     SEXP x = VECTOR_ELT(xs, i);
     SEXP elt = PROTECT(vec_cast(x, ptype, args_empty, args_empty));
 
@@ -115,8 +115,12 @@ SEXP vec_c(SEXP xs,
   if (has_names) {
     out_names = PROTECT(vec_as_names(out_names, name_repair));
     out = vec_set_names(out, out_names);
-    REPROTECT(out, out_pi);
     UNPROTECT(1);
+  } else if (!assign_names) {
+    // FIXME: `vec_ptype2()` doesn't consistently zaps names, so `out`
+    // might have been initialised with names. This branch can be
+    // removed once #1020 is resolved.
+    out = vec_set_names(out, R_NilValue);
   }
 
   UNPROTECT(7);

src/names.c

@@ -515,6 +515,10 @@ static SEXP glue_as_name_spec(SEXP spec);
 
 // [[ include("utils.h") ]]
 SEXP apply_name_spec(SEXP name_spec, SEXP outer, SEXP inner, R_len_t n) {
+  if (Rf_inherits(name_spec, "rlang_zap")) {
+    return R_NilValue;
+  }
+
   if (outer == R_NilValue) {
     return inner;
   }

src/slice-chop.c

@@ -439,8 +439,12 @@ static SEXP vec_unchop(SEXP x,
   x = PROTECT(vec_cast_common(x, ptype));
 
   SEXP x_names = PROTECT(r_names(x));
-  const bool has_outer_names = (x_names != R_NilValue);
-  const bool has_names = !is_data_frame(ptype) &&
+
+  bool has_outer_names = (x_names != R_NilValue);
+  bool assign_names = !Rf_inherits(name_spec, "rlang_zap");
+  bool has_names =
+    assign_names &&
+    !is_data_frame(ptype) &&
     (has_outer_names || list_has_inner_vec_names(x, x_size));
 
   // Element sizes are only required for applying the `name_spec`
@@ -489,7 +493,7 @@ static SEXP vec_unchop(SEXP x,
   PROTECT_WITH_INDEX(out_names, &out_names_pi);
 
   const struct vec_assign_opts unchop_assign_opts = {
-    .assign_names = true
+    .assign_names = assign_names
   };
 
   for (R_len_t i = 0; i < x_size; ++i) {
@@ -526,6 +530,11 @@ static SEXP vec_unchop(SEXP x,
     out_names = vec_as_names(out_names, name_repair);
     REPROTECT(out_names, out_names_pi);
     out = vec_set_names(out, out_names);
+  } else if (!assign_names) {
+    // FIXME: `vec_ptype2()` doesn't consistently zaps names, so `out`
+    // might have been initialised with names. This branch can be
+    // removed once #1020 is resolved.
+    out = vec_set_names(out, R_NilValue);
   }
 
   UNPROTECT(9);

tests/testthat/error/test-c.txt

@@ -34,3 +34,10 @@ See <https://vctrs.r-lib.org/articles/s3-vector.html>.
 > with_c_foobar(vec_c(foobar(1), foobar(2), .ptype = ""))
 Error: Can't convert <vctrs_foobar> to <character>.
 
+
+can ignore names by providing a `zap()` name-spec (#232)
+========================================================
+
+> vec_c(a = c(b = letters), b = 1, .name_spec = zap())
+Error: Can't combine `a` <character> and `b` <double>.
+

tests/testthat/error/test-unchop.txt

@@ -72,3 +72,14 @@ Error: Must subset elements with a valid subscript vector.
 x Subscript has the wrong type `vctrs_foobar`.
 i It must be numeric.
 
+
+can ignore names in `vec_unchop()` by providing a `zap()` name-spec (#232)
+==========================================================================
+
+> vec_unchop(list(a = c(b = letters), b = 3L), name_spec = zap())
+Error: Can't combine `a` <character> and `b` <integer>.
+
+> vec_unchop(list(a = c(foo = 1:2), b = c(bar = "")), indices = list(2:1, 3),
++ name_spec = zap())
+Error: Can't combine `a` <integer> and `b` <character>.
+

tests/testthat/test-c.R

@@ -324,6 +324,17 @@ test_that("vec_implements_ptype2() and vec_c() fallback are compatible with old
   expect_identical(vec_c(bar), bar)
 })
 
+test_that("can ignore names in `vec_c()` by providing a `zap()` name-spec (#232)", {
+  expect_error(vec_c(a = c(b = 1:2)))
+  expect_identical(vec_c(a = c(b = 1:2), b = 3L, .name_spec = zap()), 1:3)
+  verify_errors({
+    expect_error(
+      vec_c(a = c(b = letters), b = 1, .name_spec = zap()),
+      class = "vctrs_error_incompatible_type"
+    )
+  })
+})
+
 test_that("vec_c() has informative error messages", {
   verify_output(test_path("error", "test-c.txt"), {
     "# vec_c() fails with complex foreign S3 classes"
@@ -339,5 +350,8 @@ test_that("vec_c() has informative error messages", {
     "# vec_c() fallback doesn't support `name_spec` or `ptype`"
     with_c_foobar(vec_c(foobar(1), foobar(2), .name_spec = "{outer}_{inner}"))
     with_c_foobar(vec_c(foobar(1), foobar(2), .ptype = ""))
+
+    "# can ignore names by providing a `zap()` name-spec (#232)"
+    vec_c(a = c(b = letters), b = 1, .name_spec = zap())
   })
 })

tests/testthat/test-slice-chop.R

@@ -687,6 +687,37 @@ test_that("vec_unchop() does not support non-numeric S3 indices", {
   })
 })
 
+test_that("can ignore names in `vec_unchop()` by providing a `zap()` name-spec (#232)", {
+  expect_error(vec_unchop(list(a = c(b = 1:2))))
+  expect_identical(
+    vec_unchop(list(a = c(b = 1:2), b = 3L), name_spec = zap()),
+    1:3
+  )
+  expect_identical(
+    vec_unchop(
+      list(a = c(foo = 1:2), b = c(bar = 3L)),
+      indices = list(2:1, 3),
+      name_spec = zap()
+    ),
+    c(2L, 1L, 3L)
+  )
+
+  verify_errors({
+    expect_error(
+      vec_unchop(list(a = c(b = letters), b = 3L), name_spec = zap()),
+      class = "vctrs_error_incompatible_type"
+    )
+    expect_error(
+      vec_unchop(
+        list(a = c(foo = 1:2), b = c(bar = "")),
+        indices = list(2:1, 3),
+        name_spec = zap()
+      ),
+      class = "vctrs_error_incompatible_type"
+    )
+  })
+})
+
 test_that("vec_unchop() has informative error messages", {
   verify_output(test_path("error", "test-unchop.txt"), {
     "# vec_unchop() errors on unsupported location values"
@@ -717,5 +748,13 @@ test_that("vec_unchop() has informative error messages", {
     "# vec_unchop() does not support non-numeric S3 indices"
     vec_unchop(list(1), list(factor("x")))
     vec_unchop(list(1), list(foobar(1L)))
+
+    "# can ignore names in `vec_unchop()` by providing a `zap()` name-spec (#232)"
+    vec_unchop(list(a = c(b = letters), b = 3L), name_spec = zap())
+    vec_unchop(
+        list(a = c(foo = 1:2), b = c(bar = "")),
+        indices = list(2:1, 3),
+        name_spec = zap()
+      )
   })
 })