Skip to content
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

improving check_pop_data() #207

Merged
merged 4 commits into from
Jul 18, 2024
Merged

improving check_pop_data() #207

merged 4 commits into from
Jul 18, 2024

Conversation

d-morrison
Copy link
Member

improving error messages, added test

@d-morrison d-morrison self-assigned this Jul 18, 2024
@d-morrison d-morrison added API application programming interface (exported functions, etc) unit testing labels Jul 18, 2024
@d-morrison d-morrison added this to the v2.0 release milestone Jul 18, 2024
Copy link

codecov bot commented Jul 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
R/check_pop_data.R 100.00% <100.00%> (+53.84%) ⬆️

@d-morrison d-morrison requested a review from chrisorwa July 18, 2024 16:46
Copy link
Collaborator

@chrisorwa chrisorwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! How do we communicate the error classes available in the package?

}

missing_age <- is.element(attributes(pop_data)$age_var, names(pop_data))

if (!missing_age) {
cli::cli_abort(message = paste("Argument `pop_data` is missing column", attributes(pop_data)$age_var, "(age, in years)"))
"Argument {.arg pop_data} is missing column {.var {attributes(pop_data)$age_var}} (age, in years)" %>%
cli::cli_abort(class = "missing-var")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an exhaustive list of error classes?

"Provide a `data.frame()` with cross-sectional serology data per antigen isotype."
))
cli::cli_abort(
class = "not a data.frame",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we communicate all types of error classes in the package?

Copy link
Member Author

@d-morrison d-morrison Jul 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisorwa see my other comment; I don't think so, since they aren't typically visible to users of our package.

@chrisorwa chrisorwa merged commit dbd2ea8 into main Jul 18, 2024
10 checks passed
@d-morrison
Copy link
Member Author

Looks good! How do we communicate the error classes available in the package?

@chrisorwa good question; I'm still coming up to speed on error classes, but as far as I can tell, they are not part of the API - we create them ourselves for our own internal use in unit tests, instead of using the regexp argument for testthat::expect_error(). So there won't be an exhaustive list; they are more like the inputId arguments in Shiny inputs.
Take a look here for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API application programming interface (exported functions, etc) unit testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants