Use vctrs to perform input validation #961
Conversation
|
Also FYI you'll need 3344411 for checks to pass again |
R/pmap.R
Outdated
| @@ -100,12 +100,14 @@ pmap_chr <- function(.l, .f, ..., .progress = FALSE) { | |||
| } | |||
|
|
|||
| pmap_ <- function(.type, .l, .f, ..., .progress = FALSE, ..error_call = caller_env()) { | |||
| .l <- vctrs_list_compat(.l) | |||
There was a problem hiding this comment.
I guess we are ok with this?
pmap(NULL, identity)
#> list()It used to error
Maybe worth a test, I feel like it is reasonable behavior (and maybe a test for a typed variant like pmap_dbl(NULL, identity)
And use vctrs_vec_compat in more places
Create new vctrs compatibility helper, and use it in a few places. "Weird" vectors like pairlists, expressions, and calls have been deprecated.
| @@ -99,12 +99,31 @@ is_quantity <- function(x) { | |||
| x | |||
| } | |||
|
|
|||
| vctrs_list_compat <- function(x, error_call = caller_env(), error_arg = caller_arg(x)) { | |||
| force(error_arg) | |||
There was a problem hiding this comment.
Is this required? I thought part of the point of passing error_call and error_arg through in the specific way that we do is to avoid forcing the promises until they are actually required
There was a problem hiding this comment.
I had to do it to get a good error.
There was a problem hiding this comment.
Maybe preserve x by assigning the result of vctrs_vec_compat(x) to out instead of x?
I agree that the default pattern should be to evaluate these things lazily.
There was a problem hiding this comment.
Oh right, I couldn't figure out what the problem was before but now I see that what Lionel suggests is a nice simple fix that maintains the standard pattern
vctrs_list_compat <- function(x, error_call = caller_env(), error_arg = caller_arg(x)) {
out <- vctrs_vec_compat(x)
vec_check_list(out, call = error_call, arg = error_arg)
out
}There was a problem hiding this comment.
Oh yeah, that's better. I pushed a fix to main.
No description provided.