Repository navigation
Should size 1 vectors be allowed to have "" as names? #780
Description
Activity
library(vctrs) z <- new_vctr(c(foo = 1), class = "foobar", inherit_base_type = FALSE) vec_slice(z, c(NA_integer_, NA_integer_)) #> <foobar[2]> #> #> NA NA vec_restore.foobar <- function(x, to, ..., n = NULL) { new_vctr(x, class = "foobar", inherit_base_type = FALSE) } vec_slice(z, c(NA_integer_, NA_integer_)) #> Error: If any elements of `.data` are named, all must be named
I guess this isn't specifically a size 1 thing. It happens any time you have a named vector and you slice all
NA_integer_elementsShould having any
""names be allowed innames_all_or_nothing()?Actually I'm wondering if we need
validate_names()at all? Maybe we should disallowNA_character_names but otherwise allow anything elseMaybe the rule forvec_slice()is "if all elements ofiareNA_integer_, remove all names"I don't think we want
vec_slice(c(x = 1), NA_integer_)to return a vector with any names.I'm not so sure. It depends if you consider the presence of a character vector in the names attribute to be part of the type.
Taking a step back, having
NULLnames for vectors is just an optimisation. In the ALTREP world this optimisation is no longer necessary. So returningNULLnames instead of""feels like a step in the wrong direction.I think the
vctrs_vctrclass should treat repetitions of""in the same way asNULLnames.Oh I didn't see the PR was more liberal. I think it's more liberal than the suggestion above because it allows
names == c("", "a")instead of only allowing vectors of all empty names.I don't have a strong opinion about this. It forces the user to use a different default name for unnamed elements, which may make sense. @hadley should chime in as he implemented this behaviour.
The quick rationale for allowing
names == c("", "a")is because they can be generated by, say,vec_slice(z, c(NA_integer_, 1)), and if you have a restore method that goes through thenew_vctr()constructor then you get stuck. Agreed about getting Hadley's input here!That rationale makes sense to me. Actually that's probably the rationale for setting names to
NArather than""here:set_names(letters)[c(1, NA)] #> a <NA> #> "a" NA
The names are really missing rather than non-existent.
I will say that one of the high-level impressions I got from my deep dive into names is that
NAis the weirdest and most pathological option of all. FWIW here's some relevant text from?names:The name
""is special: it is used to indicate that there is no name associated with an element of a (atomic or generic) vector. Subscripting by""will match nothing (not even elements which have no name).A name can be character
NA, but such a name will never be matched and is likely to lead to confusion.This is substantially why the
"minimal"level of name repair forbidsNAs.I realize that vctrs is at least partially motivated by the need to rethink and improve some fundamental stuff and maybe that applies here. But I'd carefully consider this before making intentional use ofNAin names.I really wasn't considering using
NA, as it makes the types and hence the code more complex. Just providing some context on an old design decision.Reacted by Jennifer (Jenny) Bryan
We have a small inconsistency with this edge case.
vec_slice(x, NA_integer_)can generate a named vector of size 1 with""as names. Butnew_vctr()doesn't like this, as it errors if "all names are the empty string" (seenames_all_or_nothing())Should
vec_slice()be tweaked to not restore any names for this edge case, or shouldnames_all_or_nothing()be more relaxed?I think tweaking
vec_slice()is correct. I don't think we wantvec_slice(c(x = 1), NA_integer_)to return a vector with any names.Created on 2020-01-27 by the reprex package (v0.3.0.9000)