diff --git a/NEWS.md b/NEWS.md index f811e59d..ebe53a0b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # S7 (development version) +* In `new_class()`, properties can either be named by naming the element + of the list or by supplying the `name` argument to `new_property()` (#371). + * `super()` now works with Ops methods (#357). * `method()` now generates an informative message when dispatch fails (#387). diff --git a/R/property.R b/R/property.R index 6d5cdf63..a7852deb 100644 --- a/R/property.R +++ b/R/property.R @@ -33,9 +33,10 @@ #' @param default When an object is created and the property is not supplied, #' what should it default to? If `NULL`, defaults to the "empty" instance #' of `class`. -#' @param name Property name, primarily used for error messages. Used -#' primrarily for testing as it is set automatically when using a list of -#' properties. +#' @param name Property name, primarily used for error messages. Generally +#' don't need to set this here, as it's more convenient to supply as a +#' the element name when defining a list of properties. If both `name` +#' and a list-name are supplied, the list-name will be used. #' @returns An S7 property, i.e. a list with class `S7_property`. #' @export #' @examples @@ -393,7 +394,7 @@ as_properties <- function(x) { } out <- Map(as_property, x, names2(x), seq_along(x)) - names(out) <- names2(x) + names(out) <- vapply(out, function(x) x$name, FUN.VALUE = character(1)) if (anyDuplicated(names(out))) { stop("`properties` names must be unique", call. = FALSE) @@ -403,15 +404,23 @@ as_properties <- function(x) { } as_property <- function(x, name, i) { - if (name == "") { - msg <- sprintf("`property[[%i]]` is missing a name", i) - stop(msg, call. = FALSE) - } if (is_property(x)) { - x$name <- name + if (name == "") { + if (is.null(x$name)) { + msg <- sprintf("`properties[[%i]]` must have a name or be named.", i) + stop(msg, call. = FALSE) + } + } else { + x$name <- name + } x } else { + if (name == "") { + msg <- sprintf("`properties[[%i]]` must be named.", i) + stop(msg, call. = FALSE) + } + class <- as_class(x, arg = paste0("property$", name)) new_property(x, name = name) } diff --git a/man/new_property.Rd b/man/new_property.Rd index b5c6af06..70e7cd58 100644 --- a/man/new_property.Rd +++ b/man/new_property.Rd @@ -43,9 +43,10 @@ your code can assume that \code{value} has known type.} what should it default to? If \code{NULL}, defaults to the "empty" instance of \code{class}.} -\item{name}{Property name, primarily used for error messages. Used -primrarily for testing as it is set automatically when using a list of -properties.} +\item{name}{Property name, primarily used for error messages. Generally +don't need to set this here, as it's more convenient to supply as a +the element name when defining a list of properties. If both \code{name} +and a list-name are supplied, the list-name will be used.} } \value{ An S7 property, i.e. a list with class \code{S7_property}. diff --git a/tests/testthat/_snaps/property.md b/tests/testthat/_snaps/property.md index 2df3a7f5..4f5ea5e7 100644 --- a/tests/testthat/_snaps/property.md +++ b/tests/testthat/_snaps/property.md @@ -160,12 +160,12 @@ as_properties(list(1)) Condition Error: - ! `property[[1]]` is missing a name + ! `properties[[1]]` must be named. Code as_properties(list(new_property(class_character))) Condition Error: - ! `property[[1]]` is missing a name + ! `properties[[1]]` must have a name or be named. Code as_properties(list(x = 1)) Condition diff --git a/tests/testthat/test-property.R b/tests/testthat/test-property.R index 1ee8e71a..94bc983e 100644 --- a/tests/testthat/test-property.R +++ b/tests/testthat/test-property.R @@ -287,6 +287,16 @@ test_that("as_properties normalises properties", { as_properties(list(x = new_property(class = class_numeric))), list(x = new_property(class_numeric, name = "x") )) + expect_equal( + as_properties(list(new_property(name = "y"))), + list(y = new_property(name = "y") + )) + + # list name wins + expect_equal( + as_properties(list(x = new_property(name = "y"))), + list(x = new_property(name = "x") + )) }) test_that("as_properties() gives useful error messages", {