Skip to content

Implement .name_spec = "inner" #1995

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

Open
wants to merge 1 commit into
base: origin-feature/list-unchop-na
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -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).
Expand Down
24 changes: 14 additions & 10 deletions R/bind.R
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 14 additions & 0 deletions R/names.R
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand Down
13 changes: 13 additions & 0 deletions man/name_spec.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 13 additions & 9 deletions man/vec_bind.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions man/vec_c.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions man/vec_chop.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions man/vec_interleave.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions man/vec_unchop.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 0 additions & 6 deletions src/bind.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
17 changes: 16 additions & 1 deletion src/c.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down
13 changes: 13 additions & 0 deletions src/names.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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];

Expand Down
1 change: 1 addition & 0 deletions src/names.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 0 additions & 16 deletions tests/testthat/_snaps/bind.md
Original file line number Diff line number Diff line change
Expand Up @@ -402,22 +402,6 @@
Error in `vec_rbind()`:
! Can't combine `..1` <vctrs_Counts> and `..2` <vctrs:::common_class_fallback>.

# 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/rlang_error>
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/rlang_error>
Error in `foo()`:
! Can't zap outer names when `.names_to` is supplied.

# row-binding performs expected allocations

Code
Expand Down
10 changes: 9 additions & 1 deletion tests/testthat/_snaps/c.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@
i The author of the class should implement vctrs methods.
i See <https://vctrs.r-lib.org/reference/faq-error-incompatible-attributes.html>.

# 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}")),
Expand Down Expand Up @@ -152,6 +152,14 @@
Error in `vec_c()`:
! Can't combine `a` <character> and `b` <double>.

# 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` <double> and `y` <character>.

# concatenation performs expected allocations

Code
Expand Down
24 changes: 17 additions & 7 deletions tests/testthat/_snaps/slice-chop.md
Original file line number Diff line number Diff line change
Expand Up @@ -392,22 +392,23 @@
Error in `list_unchop()`:
! Can't combine `x[[1]]` <vctrs_Counts> and `x[[2]]` <double>.

# 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/rlang_error>
Error in `list_unchop()`:
! Can't use a name specification with non-vctrs types.
vctrs methods must be implemented for class `vctrs_foobar`.
See <https://vctrs.r-lib.org/articles/s3-vector.html>.
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/rlang_error>
Error in `foo()`:
Expand All @@ -416,8 +417,8 @@
See <https://vctrs.r-lib.org/articles/s3-vector.html>.
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/vctrs_error_cast>
Error in `list_unchop()`:
Expand Down Expand Up @@ -478,6 +479,15 @@
Error in `list_unchop()`:
! Can't combine `x$a` <integer> and `x$b` <character>.

# 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` <double> and `x$y` <character>.

# list_unchop() fails if foreign classes are not homogeneous and there is no c() method

Code
Expand Down
Loading
Loading