Levels ignored in empty default factor

[jonthegeek]

My real use-case of this is more complicated, but this is enough to dig into it.

library(S7)
expected_levels <- levels(factor(levels = c("a", "b")))
class_testing <- new_class(
  name = "testing",
  properties = list(
    enum = new_property(
      class = class_factor,
      default = factor(levels = c("a", "b"))
    )
  )
)
default_levels <- levels(class_testing()@enum)
expected_levels
#> [1] "a" "b"
default_levels
#> character(0)
identical(default_levels, expected_levels)
#> [1] FALSE

^{Created on 2023-12-05 with reprex v2.0.2}

I traced it to the definition of %||% in utils.R (uses length(x) == 0, not is.null(x); it's more like the unexported %|0|% from rlang, rather than %||%).

`%||%` <- function(x, y) if (length(x) == 0) y else x

The given default (factor(levels = c("a", "b"))) is length 0, but it isn't NULL.

I'm not sure yet that this actually matters for what I'm doing, but it was surprising (and made me think I had some deeper issue).

I'd try to fix it myself, but I'm getting other errors when I run the tests, and this one seems like it could break things in surprising ways.

[mmaechler]

Of course, I'm strongly arguing that the length(x) == 0 version of %||% is non-sense in particular as I was the one who added %||% to base only a couple of weeks ago. But even if it was not in base (that R-devel version of base is only due to release next April) many other packages define %||%, others import it from rlang... and the "only truly correct" ;-) condition is is.null(.) ..

[hadley]

Yeah, I suspect this came into S7 at a time when the tidyverse team was exploring the length(x) == 0 condition, which we ultimately decided was a bad idea.