Skip to content

Commit

Permalink
Add a names<- method for vctrs_rcrd
Browse files Browse the repository at this point in the history
This is a no-op for `NULL` names, but otherwise errors. We should reconsider this in the future once we are ready to try and support names on rcrds, but for now this prevents `vec_set_names()` from corrupting rcrd types.
  • Loading branch information
DavisVaughan committed Aug 4, 2021
1 parent 599fa9a commit 41e68fb
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 0 deletions.
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ S3method("length<-",vctrs_rcrd)
S3method("length<-",vctrs_vctr)
S3method("levels<-",vctrs_sclr)
S3method("levels<-",vctrs_vctr)
S3method("names<-",vctrs_rcrd)
S3method("names<-",vctrs_sclr)
S3method("names<-",vctrs_vctr)
S3method("|",vctrs_vctr)
Expand Down
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)

* `vec_set_names()` no longer corrupts `vctrs_rcrd` types (#1419).

* `vec_detect_complete()` now computes completeness for `vctrs_rcrd` types in
the same way as data frames, which means that if any field is missing, the
entire record is considered incomplete (#1386).
Expand Down
9 changes: 9 additions & 0 deletions R/type-rcrd.R
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,15 @@ names.vctrs_rcrd <- function(x) {
NULL
}

#' @export
`names<-.vctrs_rcrd` <- function(x, value) {
if (is_null(value)) {
x
} else {
abort("Can't assign names to a <vctrs_rcrd>.")
}
}

#' @export
format.vctrs_rcrd <- function(x, ...) {
if (inherits(x, "vctrs_foobar")) {
Expand Down
22 changes: 22 additions & 0 deletions tests/testthat/test-type-rcrd.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,28 @@ test_that("vec_proxy() transforms records to data frames", {
)
})

# base methods ------------------------------------------------------------

test_that("has no names", {
x <- new_rcrd(list(a = 1, b = 2L))

expect_null(names(x))
expect_null(vec_names(x))
})

test_that("removing names with `NULL` is a no-op (#1419)", {
x <- new_rcrd(list(a = 1, b = 2L))

expect_identical(`names<-`(x, NULL), x)
expect_identical(vec_set_names(x, NULL), x)
})

test_that("setting character names is an error (#1419)", {
x <- new_rcrd(list(a = 1, b = 2L))

expect_error(`names<-`(x, "x"), "Can't assign names")
expect_error(vec_set_names(x, "x"), "Can't assign names")
})

# coercion ----------------------------------------------------------------

Expand Down

0 comments on commit 41e68fb

Please sign in to comment.