You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In col_factor(), levels of the factor must be specified in the first argument levels, supplied as a vector:
x<- c("M", "F", NA)
collector<- col_factor(c("M", "F"))
parse_vector(x, collector)
#> [1] M F <NA>#> Levels: M F
In practice, users may accidentally spread out the levels across arguments of col_factor(). In current implementation, the collector is still successfully constructed, and the error is only caught at the point of parsing, with a rather cryptic message.
collector<- col_factor("M", "F")
parse_vector(x, collector)
#> Error: Expected single logical value
In so far as the second argument of col_factor() (ordered) expects a scalar logical, I think it'd be helpful for col_factor() to check for this early at its creation. Additionally, it would be helpful for col_factor() to inform the likely source of the error if ordered receives a scalar character instead (the user mistakenly treated levels as ...).
Proposed solution
A behavior like this would be helpful for debugging:
col_factor("M", "F")
#> Error in `col_factor()`:#> ! `ordered` must be a logical, not a string.#> ℹ Did you intend to pass a vector of levels to `levels`?
And here's the toy implementation for the above (inserts an if block to check ordered):
col_factor<-function(levels=NULL, ordered=FALSE, include_na=FALSE) {
if (!(is.null(levels) || is.character(levels))) {
stop(sprintf("`levels` must be `NULL` or a character vector:\n- `levels` is a '%s'", class(levels)), call.=FALSE)
}
if (!rlang::is_scalar_logical(ordered)) {
cli::cli_abort(c(
"{.arg ordered} must be a logical, not {.obj_type_friendly {ordered}}.",
if (rlang::is_scalar_character(ordered)) {
c("i"="Did you intend to pass a vector of levels to {.arg levels}?")
}
))
}
collector("factor", levels=levels, ordered=ordered, include_na=include_na)
}
Happy to PR this if this is within scope!
The text was updated successfully, but these errors were encountered:
Issue
In
col_factor()
, levels of the factor must be specified in the first argumentlevels
, supplied as a vector:In practice, users may accidentally spread out the levels across arguments of
col_factor()
. In current implementation, the collector is still successfully constructed, and the error is only caught at the point of parsing, with a rather cryptic message.In so far as the second argument of
col_factor()
(ordered
) expects a scalar logical, I think it'd be helpful forcol_factor()
to check for this early at its creation. Additionally, it would be helpful forcol_factor()
to inform the likely source of the error ifordered
receives a scalar character instead (the user mistakenly treatedlevels
as...
).Proposed solution
A behavior like this would be helpful for debugging:
And here's the toy implementation for the above (inserts an
if
block to checkordered
):Happy to PR this if this is within scope!
The text was updated successfully, but these errors were encountered: