Implement argument checking for exported functions #114
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
skadauke
requested changes
Dec 16, 2022
|
Also, in case you haven't seen it, there is a really good section in the tidyverse style guide on error messages: https://style.tidyverse.org/error-messages.html |
|
Made the following updates:
Error Message Examplesdevtools::load_all()
#> ℹ Loading REDCapTidieR
options(rlang_backtrace_on_error_report = "none")
# read_redcap
classic_token <- "123456789ABCDEF"
redcap_uri <- "www.google.com"
## redcap_uri
read_redcap(123, classic_token)
#> Error in `read_redcap()`:
#> ✖ You've supplied an invalid value to `redcap_uri`
#> ! Must be of type 'character', not 'double'
read_redcap(letters[1:3], classic_token)
#> Error in `read_redcap()`:
#> ✖ You've supplied an invalid value to `redcap_uri`
#> ! Must have length 1, but has length 3
## token
read_redcap(redcap_uri, 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied an invalid value to `token`
#> ! Must be of type 'character', not 'double'
read_redcap(redcap_uri, letters[1:3])
#> Error in `read_redcap()`:
#> ✖ You've supplied an invalid value to `token`
#> ! Must have length 1, but has length 3
## raw_or_label
read_redcap(redcap_uri, classic_token, raw_or_label = "bad option")
#> Error in `read_redcap()`:
#> ✖ You've supplied an invalid value to `raw_or_label`
#> ! Must be element of set {'label','raw'}, but is 'bad option'
## forms
read_redcap(redcap_uri, classic_token, forms = 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied an invalid value to `forms`
#> ! Must be of type 'character' (or 'NULL'), not 'double'
## export_survey_fields
read_redcap(redcap_uri, classic_token, export_survey_fields = 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied an invalid value to `export_survey_fields`
#> ! Must be of type 'logical', not 'double'
read_redcap(redcap_uri, classic_token, export_survey_fields = c(TRUE, TRUE))
#> Error in `read_redcap()`:
#> ✖ You've supplied an invalid value to `export_survey_fields`
#> ! Must have length 1, but has length 2
## suppress_redcapr_messages
read_redcap(redcap_uri, classic_token, suppress_redcapr_messages = 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied an invalid value to `suppress_redcapr_messages`
#> ! Must be of type 'logical', not 'double'
read_redcap(redcap_uri, classic_token, suppress_redcapr_messages = c(TRUE, TRUE))
#> Error in `read_redcap()`:
#> ✖ You've supplied an invalid value to `suppress_redcapr_messages`
#> ! Must have length 1, but has length 2
# bind_tibbles
bind_tibbles(123)
#> Error in `bind_tibbles()`:
#> ✖ You've supplied an invalid value to `supertbl`
#> ! Must be of type 'data.frame', not 'double'
supertbl <- tibble(redcap_data = list())
bind_tibbles(supertbl, environment = "abc")
#> Error in `bind_tibbles()`:
#> ✖ You've supplied an invalid value to `environment`
#> ! Must be an environment, not 'character'
bind_tibbles(supertbl, tbls = 123)
#> Error in `bind_tibbles()`:
#> ✖ You've supplied an invalid value to `tbls`
#> ! Must be of type 'character' (or 'NULL'), not 'double'
# extract_tibbles
extract_tibbles(123)
#> Error in `extract_tibbles()`:
#> ✖ You've supplied an invalid value to `supertbl`
#> ! Must be of type 'data.frame', not 'double'
# extract_tibble
extract_tibble(123, "my_tibble")
#> Error in `extract_tibble()`:
#> ✖ You've supplied an invalid value to `supertbl`
#> ! Must be of type 'data.frame', not 'double'
supertbl <- tibble(redcap_data = list())
extract_tibble(supertbl, tbl = 123)
#> Error in `extract_tibble()`:
#> ✖ You've supplied an invalid value to `tbl`
#> ! Must be of type 'character', not 'double'
extract_tibble(supertbl, tbl = letters[1:3])
#> Error in `extract_tibble()`:
#> ✖ You've supplied an invalid value to `tbl`
#> ! Must have length 1, but has length 3
# make_labelled
make_labelled(123)
#> Error in `make_labelled()`:
#> ✖ You've supplied an invalid value to `supertbl`
#> ! Must be of type 'data.frame', not 'double'
missing_col_supertbl <- tibble(redcap_data = list())
make_labelled(missing_col_supertbl)
#> Error in `make_labelled()`:
#> ✖ You've supplied an invalid value to `supertbl`
#> ! Must contain columns `redcap_data` and `redcap_metadata`
#> ! You are missing `redcap_metadata`
missing_list_col_supertbl <- tibble(redcap_data = list(), redcap_metadata = 123)
make_labelled(missing_list_col_supertbl)
#> Error in `make_labelled()`:
#> ✖ You've supplied an invalid value to `supertbl`
#> ! `redcap_metadata` must be of type 'list'Created on 2022-12-16 with reprex v2.0.2 |
|
I think these returns largely look great. I especially like "element set" message for the One thing to note, which may not be implementable here but for the future, is if For > make_labelled(data.frame(mtcars))
Error in `check_req_labelled_fields()` at REDCapTidieR/R/labelled.R:58:2:
! `supertbl` must contain `redcap_data` and `redcap_metadata`
✖ You are missing `redcap_data` and `redcap_metadata`
Run `rlang::last_error()` to see where the error occurred.For fun I modified mtcars to have these colnames, and (while I this should never in a million years happen) here's what could happen: mtcars_mod <- data.frame(mtcars) |>
mutate(
redcap_data = cyl,
redcap_metadata = hp
)
> make_labelled(data.frame(mtcars_mod))
Error in `check_req_labelled_metadata_fields()` at REDCapTidieR/R/labelled.R:59:2:
! All elements of `supertbl$redcap_metadata` must contain
`field_name` and `field_label`
✖ `supertbl$redcap_metadata[[1]]` is missing `field_name` and
`field_label`
✖ `supertbl$redcap_metadata[[2]]` is missing `field_name` and
`field_label`
.... |
|
This looks great! A few comments:
How about:
|
I've been messing around with this but haven't quite gotten it. If we want the actual evaluated value passed by the user it's easy: "You've supplied {x} for {.arg {arg}} which is not a valid value."But this won't work for things like If we want the actual unevaluated text the user entered it doesn't seem like there's an easy way to do this without capturing the value in the function that calls the Works but uglylibrary(rlang)
exported_function <- function(param) {
check_arg_function(param, val = caller_arg(param))
}
check_arg_function <- function(x, val, arg = caller_arg(x)) {
cat("The value of", arg, "was", val)
}
x <- 123
exported_function(x)
#> The value of param was x
exported_function(123)
#> The value of param was 123
If we're good to add the S3 class I think this will all work in a modified version of
Line 53 in 7fc296b Any reason to change it? |
Got it. I think we could handle this like so:
|
|
To summarize the additional pieces of this PR in the works today:
|
Done
Done with the addition of adding an example
Done. Using
Agreed to hold off on this Error Message Examplesdevtools::load_all()
#> ℹ Loading REDCapTidieR
options(rlang_backtrace_on_error_report = "none")
# read_redcap
classic_token <- "123456789ABCDEF123456789ABCDEF01"
redcap_uri <- "www.google.com"
## redcap_uri
read_redcap(123, classic_token)
#> Error in `read_redcap()`:
#> ✖ You've supplied `123` for `redcap_uri` which is not a valid value
#> ! Must be of type 'character', not 'double'
read_redcap(letters[1:3], classic_token)
#> Error in `read_redcap()`:
#> ✖ You've supplied `a`, `b`, `c` for `redcap_uri` which is not a valid
#> value
#> ! Must have length 1, but has length 3
## token
read_redcap(redcap_uri, 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied `123` for `token` which is not a valid value
#> ! Must be of type 'character', not 'double'
read_redcap(redcap_uri, letters[1:3])
#> Error in `read_redcap()`:
#> ✖ You've supplied `a`, `b`, `c` for `token` which is not a valid value
#> ! Must have length 1, but has length 3
## raw_or_label
read_redcap(redcap_uri, classic_token, raw_or_label = "bad option")
#> Error in `read_redcap()`:
#> ✖ You've supplied `bad option` for `raw_or_label` which is not a valid
#> value
#> ! Must be element of set {'label','raw'}, but is 'bad option'
## forms
read_redcap(redcap_uri, classic_token, forms = 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied `123` for `forms` which is not a valid value
#> ! Must be of type 'character' (or 'NULL'), not 'double'
## export_survey_fields
read_redcap(redcap_uri, classic_token, export_survey_fields = 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied `123` for `export_survey_fields` which is not a valid
#> value
#> ! Must be of type 'logical', not 'double'
read_redcap(redcap_uri, classic_token, export_survey_fields = c(TRUE, TRUE))
#> Error in `read_redcap()`:
#> ✖ You've supplied `TRUE`, `TRUE` for `export_survey_fields` which is not
#> a valid value
#> ! Must have length 1, but has length 2
## suppress_redcapr_messages
read_redcap(redcap_uri, classic_token, suppress_redcapr_messages = 123)
#> Error in `read_redcap()`:
#> ✖ You've supplied `123` for `suppress_redcapr_messages` which is not a
#> valid value
#> ! Must be of type 'logical', not 'double'
read_redcap(redcap_uri, classic_token, suppress_redcapr_messages = c(TRUE, TRUE))
#> Error in `read_redcap()`:
#> ✖ You've supplied `TRUE`, `TRUE` for `suppress_redcapr_messages` which
#> is not a valid value
#> ! Must have length 1, but has length 2
# bind_tibbles
bind_tibbles(123)
#> Error in `bind_tibbles()`:
#> ✖ You've supplied `123` for `supertbl` which is not a valid value
#> ! Must be of class <redcaptidier_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#> `read_redcap()`
supertbl <- tibble(redcap_data = list())
bind_tibbles(supertbl, environment = "abc")
#> Error in `bind_tibbles()`:
#> ✖ You've supplied `<tibble[,1]>` for `supertbl` which is not a valid
#> value
#> ! Must be of class <redcaptidier_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#> `read_redcap()`
bind_tibbles(supertbl, tbls = 123)
#> Error in `bind_tibbles()`:
#> ✖ You've supplied `<tibble[,1]>` for `supertbl` which is not a valid
#> value
#> ! Must be of class <redcaptidier_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#> `read_redcap()`
# extract_tibbles
extract_tibbles(letters[1:10])
#> Error in `extract_tibbles()`:
#> ✖ You've supplied `a`, `b`, `c`, …, `i`, `j` for `supertbl` which is not
#> a valid value
#> ! Must be of class <redcaptidier_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#> `read_redcap()`
# extract_tibble
extract_tibble(123, "my_tibble")
#> Error in `extract_tibble()`:
#> ✖ You've supplied `123` for `supertbl` which is not a valid value
#> ! Must be of class <redcaptidier_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#> `read_redcap()`
supertbl <- tibble(redcap_data = list()) %>%
as_supertbl()
extract_tibble(supertbl, tbl = 123)
#> Error in `extract_tibble()`:
#> ✖ You've supplied `123` for `tbl` which is not a valid value
#> ! Must be of type 'character', not 'double'
extract_tibble(supertbl, tbl = letters[1:3])
#> Error in `extract_tibble()`:
#> ✖ You've supplied `a`, `b`, `c` for `tbl` which is not a valid value
#> ! Must have length 1, but has length 3
# make_labelled
make_labelled(123)
#> Error in `make_labelled()`:
#> ✖ You've supplied `123` for `supertbl` which is not a valid value
#> ! Must be of class <redcaptidier_supertbl>
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#> `read_redcap()`
missing_col_supertbl <- tibble(redcap_data = list()) %>%
as_supertbl()
make_labelled(missing_col_supertbl)
#> Error in `make_labelled()`:
#> ✖ You've supplied `<redcaptidier_supertbl[,1]>` for `supertbl` which is
#> not a valid value
#> ! Must contain `supertbl$redcap_metadata`
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#> `read_redcap()`
missing_list_col_supertbl <- tibble(redcap_data = list(), redcap_metadata = 123) %>%
as_supertbl()
make_labelled(missing_list_col_supertbl)
#> Error in `make_labelled()`:
#> ✖ You've supplied `<redcaptidier_supertbl[,2]>` for `supertbl` which is
#> not a valid value
#> ! `supertbl$redcap_metadata` must be of type 'list'
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#> `read_redcap()`Created on 2022-12-19 with reprex v2.0.2 |
rsh52
reviewed
Dec 20, 2022
rsh52
approved these changes
Dec 20, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR implements error checking for arguments to exported functions with the following exceptions:
extract_tibbles(): No checking fortblssince this takes an unevaluated expression andeval_select()does checking on our behalf withcli-generated messagesfmt_*format helpers: No checking on arguments since these are all thin wrappers forstringrfunctions.stringrdoes a nice job of trying to coerce inputs tocharacterand I don't think we want to override this behavior.Proposed Changes
check_arg_is_dataframe(),check_arg_is_env(),check_arg_is_character(),check_arg_is_logical(),check_arg_choices()that are wrappers forcheckmatechecks.wrap_checkmate(), so we can easily add new checking functions in the futuretest-checks.Rto make sure check functions workScreenshots
Example error message:
The first line is added by
REDCapTidieRand the second line is the message fromcheckmate.Issue Addressed
Closes #109
PR Checklist
Before submitting this PR, please check and verify below that the submission meets the below criteria:
.RDS) updated underinst/testdata/create_test_data.Rhttptest::with_mock_apiand any new mocks were added totests/testthat/fixtures/create_httptest_mocks.RCode Review
This section to be used by the reviewer and developers during Code Review after PR submission
Code Review Checklist