-
Notifications
You must be signed in to change notification settings - Fork 8
Prevent object serialization in slide error messages #429
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I'm guessing the culprit here is (function() {
comp_value = list(letters)
assert(check_atomic(comp_value, any.missing = TRUE),
check_data_frame(comp_value), combine = "or", .var.name = vname(comp_value))
})()
#> Error: Assertion on 'list(c("a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z"))' failed: One of the following must apply:
#> * check_atomic(list(c("a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l",
#> * "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z"))): Must be
#> * of type 'atomic', not 'list'
#> * check_data_frame(list(c("a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k",
#> * "l", "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z"))):
#> * Must be of type 'data.frame', not 'list'. We can improve this a bit with a custom name (function() {
comp_value = list(letters)
assert(check_atomic(comp_value, any.missing = TRUE),
check_data_frame(comp_value), combine = "or", .var.name = "slide computation output")
})()
#> Error: Assertion on 'slide computation output' failed: One of the following must apply:
#> * check_atomic(slide computation output): Must be of type 'atomic', not 'list'
#> * check_data_frame(slide computation output): Must be of type 'data.frame', not
#> * 'list'. though it still looks a little awkward, and might not look as good if we try to include some context about where the error occurred (see #351 point 2). @dshemetov any ideas here to make error message output a bit friendlier? |
So seems like we need to add |
Accepting [unnamed] lists [& rejecting named lists] would probably need some more effort verifying/tweaking things to work. If we want to just fix the error message we could focus on that. |
Sorry, I forgot that you were working on this. Probably going to clobber your work with my eliminate-slide-annoyances pass. |
I haven't really worked on this, so not a problem. |
When I try outputting an unnamed, length-1 list result from a slide computation, I get an unintelligible, incomplete error message.
[In interactive mode the actual description of what's wrong seems to be cut off? Trying in
reprex
instead seems to output a whole lot more of the structure; I didn't look to see if it also cuts off the important part.]The "problem" we're catching here is that an unnamed, length-1 list isn't atomic. (Aside: I think we could probably accept these, and maybe even unnamed lists of other lengths [though it might require some implementation tweaks, not just validation changes]. Named lists are likely a mistake or would need extra processing, though.) This was previously the job of
but now is
and checkmate's not giving a great message.
The text was updated successfully, but these errors were encountered: