Refine assert_function_expects error handling and tests#144
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54c5a86e6e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| func_required_arg_names <- function(func){ | ||
| args <- formals(args(func)) | ||
| if(length(args) == 0){ | ||
| return(character(0)) | ||
| } | ||
| required_args <- args[vapply(args, is.symbol, FUN.VALUE = TRUE)] | ||
| required_args <- names(required_args) |
There was a problem hiding this comment.
func_required_arg_names() uses is.symbol to decide which formals are “required.” In R, a default like x = mean or x = y is stored as a symbol too, so this logic treats those defaulted args as required and assert_function_expects() will incorrectly report them missing when callers omit them. This only shows up for functions whose defaults are symbols, but it makes the new assertion reject valid functions. Consider checking for a missing default (e.g., identical(arg, quote(expr=))) rather than is.symbol.
Useful? React with 👍 / 👎.
|
@codex add examples to the assert_function_expects roxygen comment |
|
Summary
Testing
|
selkamand
deleted the
codex/add-assert_function_expects-to-assertion-package
branch
1 participant
Motivation
Description
function_expects_advanced()and return a formatted message that includes the missing count and a backticked list of missing names.R/assert_functions.Rto embed the computedmissing_countandmissing_argsinto the returned error string instead of relying on deferred glue placeholders.tests/testthat/test-assert_functions.Rto cover defaults, functions with only..., and non-function inputs, replacing several snapshots with explicitexpect_error()checks forassert_function_expects()while keeping snapshot coverage forassert_function_expects_n_arguments().tests/testthat/_snaps/assert_functions.mdto retain existing snapshot coverage for the n-arguments assertion.Testing
devtools::document()which completed and updated documentation successfully.devtools::test()and all tests passed (PASS 978,FAIL 0,WARN 0,SKIP 0).devtools::check()which completed but reported one WARNING (qpdfmissing for PDF size checks) and one NOTE (future timestamps);R CMD checksurfaced the WARNING which caused the check command to exit non-zero.Codex Task