Pass calls for improved error messages #1340
Conversation
Conflicts: R/rectangle.R tests/testthat/_snaps/expand.md tests/testthat/_snaps/rectangle.md
|
I'll look at these in some detail next week, thanks @mgirlich |
|
Let's wait a week or two for vctrs and tidyselect to be out, and then I'll finish off. |
#Conflicts: # DESCRIPTION # R/rectangle.R # tests/testthat/_snaps/nest.md # tests/testthat/_snaps/rectangle.md
|
I think this should be good for review. I worked through all the snapshot files so while there are probably more places that need updates, this feels like a good place to start. |
R/chop.R
Outdated
| df_unchop <- function(x, ..., ptype = NULL, keep_empty = FALSE) { | ||
| df_unchop <- function(x, ..., ptype = NULL, keep_empty = FALSE, call = caller_env()) { |
There was a problem hiding this comment.
Our emerging convention when passing error calls through as arguments seems to be:
-
Name them
callandargwhen the function's main purpose is for some kind of input checking, i.e.vec_assert(),check_*()functions,abort(), etc. -
Name them
error_callanderror_argotherwise.
Since df_unchop()'s main purpose is unchopping, it should use error_call. The rest of the PR probably needs a pass to do this updating too. I'll try and mark them as I see them.
We have started doing this in vctrs, tidyselect, internally in dplyr, and I've done it in ivs, so probably best to take the time to be consistent here since call improvements is what this PR is all about.
There was a problem hiding this comment.
FWIW I don't like this convention because it means there are 4 names it could be: call, .call, error_call, or .error_call. I'd prefer to use error_call everywhere.
There was a problem hiding this comment.
This means we need to rename a lot of already exported functions? Isn't it a bit late to change this now?
By the way I don't consider a dotted variant to represent a different name. I see it as different syntax for the same name.
There was a problem hiding this comment.
Right, I don't think of them as different names either, but pragmatically you have to remember which of 4 options it is.
There was a problem hiding this comment.
Only in the terminal right? In IDEs you immediately see if you have a function that requires dotted arguments. It also shows you whether you need call or error_call.
Pass call to
abort()and some vctrs functions for nicer error messages.Fixes #1313.