Better error handling for guess_max #590
Conversation
R/col_types.R
Outdated
| guess_types <- function(datasource, tokenizer, locale, n = 1000) { | ||
| guess_types_(datasource, tokenizer, locale, n = n) | ||
| guess_types <- function(datasource, tokenizer, locale, guess_max = 1000, max_limit = .Machine$integer.max %/% 100) { | ||
| if (is.na(guess_max) || guess_max < 0) { |
There was a problem hiding this comment.
Probably should check it's a numeric of length 1 first
And then you probably should pull out into check_guess_max()
| #' | ||
| #' # Type convert can be used to infer types from an entire dataset | ||
| #' type_convert( | ||
| #' read_csv(readr_example("mtcars.csv"), |
|
Ok addressed comments at 5f848cc |
R/col_types.R
Outdated
| guess_types_(datasource, tokenizer, locale, n = n) | ||
|
|
||
| check_guess_max <- function(guess_max, max_limit = .Machine$integer.max %/% 100) { | ||
| if (!(length(guess_max) == 1 && is.numeric(guess_max) && |
There was a problem hiding this comment.
I agree it is a bit confusing, any suggestions on a clearer way to express this?
There was a problem hiding this comment.
Two options:
stopifnot(is.numeric(guess_max), length(guess_max) == 1)
Or De Morgan's the whole thing, which I think gives you
if (length(guess_max) != 1 || !is.numeric(guess_max) ||
!is_integerish(guess_max)) || is.na(guess_max) || guess_max <= 0) {
There was a problem hiding this comment.
How about this, the assert definition is slightly complicated because we want to evaluate the ... lazily.
assert <- function(msg, ...) {
dots <- eval(substitute(alist(...)))
for (x in dots) {
if (!isTRUE(eval(x, parent.frame()))) {
stop(msg, call. = FALSE)
}
}
invisible(TRUE)
}
assert("`guess_max` must be a positive integer",
length(guess_max) == 1, is.numeric(guess_max), is_integerish(guess_max),
!is.na(guess_max), guess_max > 0)The assert definition is basically identical to what is in assertthat, not sure if this makes me feel good or bad 😄
There was a problem hiding this comment.
Hmmmmm, I think we can just stick to an if statement instead of introducing a new syntactic form 😉
Fixes #588
I added another example showing how to read a dataset as character and using
type_convert()to infer types based on the full dataset as well.