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

Prompt for missing parameters #115

Merged
merged 4 commits into from
Oct 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Package: orderly2
Title: Orderly Next Generation
Version: 1.99.8
Version: 1.99.9
Authors@R: c(person("Rich", "FitzJohn", role = c("aut", "cre"),
email = "[email protected]"),
person("Robert", "Ashton", role = "aut"),
Expand Down
58 changes: 58 additions & 0 deletions R/interactive.R
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,61 @@ detect_orderly_interactive_path <- function(path = getwd()) {
orderly_interactive_set_search_options <- function(options = NULL) {
.interactive$search_options <- options
}


get_parameter_interactive <- function(name, call = NULL) {
value <- readline(sprintf("%s > ", name))
if (!nzchar(value)) {
cli::cli_abort("Expected a value for parameter '{name}'", call = call)
}
parsed <- tryCatch(parse(text = value)[[1L]], error = identity)
explain_string <- paste(
"If entering a string, you must use quotes, this helps",
'disambiguate numbers and booleans. For example, TRUE and "TRUE" will',
"parse as a boolean and a string, respectively")
if (inherits(parsed, "error")) {
cli::cli_abort(
c("Failed to parse value for parameter '{name}'",
x = "Was given: {value}",
i = explain_string),
parent = parsed, call = call)
}
if (is.name(parsed)) {
cli::cli_abort(
c("Invalid input for parameter '{name}'",
i = 'Did you mean: {.strong "{as.character(parsed)}"} (in quotes)?',
i = explain_string),
call = call)
}
if (!is_simple_scalar_atomic(parsed)) {
cli::cli_abort(
c("Invalid input for parameter '{name}': {value}",
i = "Must be a simple boolean, number or string"),
call = call)
}
parsed[[1]]
}


get_missing_parameters_interactive <- function(required, envir, call = NULL) {
msg <- setdiff(required, names(envir))
if (length(msg) == 0) {
return()
}
if (getOption("orderly_interactive_parameters_missing_error", FALSE)) {
cli::cli_abort(
c("Missing parameters: {squote(msg)}",
i = paste("Erroring because option",
"'orderly_interactive_parameters_missing_error'",
"is 'TRUE'")),
call = call)
}
n <- length(msg)
cli::cli_alert(
"Please enter values for {n} missing {cli::qty(n)}parameter{?s}:")
found <- list()
for (nm in msg) {
found[[nm]] <- get_parameter_interactive(nm, call)
}
list2env(found, envir)
}
16 changes: 16 additions & 0 deletions R/metadata.R
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,22 @@ static_orderly_strict_mode <- function(args) {
##' not come from a variable itself). Provide `NULL` if you do not
##' have a default, in which case this parameter will be required.
##'
##' @section Behaviour in interactive sessions:
##'
##' When running interactively (i.e., via `source()` or running an
##' `orderly.R` session by copy/paste or in Rstudio), the
##' `orderly_parameters()` function has different behaviour.
##'
##' First, we look in the current environment (most likely the global
##' environment) for values of your parameters - that is, variables
##' bound to the names of your parameters. For any parameters that
##' are not found we will look at the default values and use these
##' if possible, but if not possible then we will either error or
##' prompt based on the global option
##' `orderly_interactive_parameters_missing_error`. If this is
##' `TRUE`, then we will ask you to enter a value for the parameters
##' (strings will need to be entered with quotes).
##'
##' @title Declare orderly parameters
##'
##' @param ... Any number of parameters
Expand Down
15 changes: 2 additions & 13 deletions R/run.R
Original file line number Diff line number Diff line change
Expand Up @@ -408,19 +408,8 @@ check_parameters_interactive <- function(envir, spec, call) {
return()
}

is_required <- vlapply(spec, is.null)

msg <- setdiff(names(spec)[is_required], names(envir))
if (length(msg) > 0L) {
## This will change, but we'll need some interactive prompting
## better done in another ticket. See
## https://github.com/r-lib/cli/issues/228 and
## https://github.com/r-lib/cli/issues/488 for context here.
##
## There will be other "interactive mode" functions too that we'll
## try and get a unified interface on.
stop("Missing parameters: ", paste(squote(msg), collapse = ", "))
}
required <- names(spec)[vlapply(spec, is.null)]
get_missing_parameters_interactive(required, envir, call)

## Set any missing values into the environment:
list2env(spec[setdiff(names(spec), names(envir))], envir)
Expand Down
18 changes: 18 additions & 0 deletions man/orderly_parameters.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 40 additions & 0 deletions tests/testthat/test-interactive.R
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,43 @@ test_that("can detect orderly directory", {
root <- detect_orderly_interactive_path(file.path(path, "src", "explicit"))
expect_equal(path, root)
})


test_that("can validate interactive parameters", {
mock_readline <- mockery::mock("TRUE", "100", "1.23", '"string"')
mockery::stub(get_parameter_interactive, "readline", mock_readline)
expect_equal(get_parameter_interactive("foo"), TRUE)
expect_equal(get_parameter_interactive("foo"), 100)
expect_equal(get_parameter_interactive("foo"), 1.23)
expect_equal(get_parameter_interactive("foo"), "string")
})


test_that("can error when interactive parameters are invlid", {
mock_readline <- mockery::mock("", "hey ho", "string", "c(1, 2)")
mockery::stub(get_parameter_interactive, "readline", mock_readline)
err <- expect_error(
get_parameter_interactive("foo"),
"Expected a value for parameter 'foo'")
expect_null(err$body)

err <- expect_error(
get_parameter_interactive("foo"),
"Failed to parse value for parameter 'foo'")
expect_length(err$body, 2)
expect_equal(err$body[1], c(x = "Was given: hey ho"))
expect_match(err$body[[2]], "If entering a string, you must use quotes")

err <- expect_error(
get_parameter_interactive("foo"),
"Invalid input for parameter 'foo'")
expect_equal(
cli::ansi_strip(err$body[[1]]),
'Did you mean: "string" (in quotes)?')
expect_match(err$body[[2]], "If entering a string, you must use quotes")

err <- expect_error(
get_parameter_interactive("foo"),
"Invalid input for parameter 'foo'")
expect_equal(err$body[1], c(i = "Must be a simple boolean, number or string"))
})
33 changes: 32 additions & 1 deletion tests/testthat/test-parameters.R
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,45 @@ test_that("set defaults into environment if missing", {
})


test_that("fill in missing parameters", {
envir <- list2env(list(b = 3, c = 4), parent = new.env())
mock_get <- mockery::mock(envir$a <- 1)
mockery::stub(check_parameters_interactive,
"get_missing_parameters_interactive",
mock_get)
check_parameters_interactive(envir, list(a = NULL, b = NULL, c = NULL), NULL)
mockery::expect_called(mock_get, 1)
expect_equal(mockery::mock_args(mock_get)[[1]],
list(c("a", "b", "c"), envir, NULL))
expect_mapequal(as.list(envir), list(a = 1, b = 3, c = 4))
})


test_that("require non-default parameters are present in environment", {
withr::local_options(orderly_interactive_parameters_missing_error = TRUE)
envir <- list2env(list(b = 3, c = 4), parent = new.env())
expect_error(
check_parameters_interactive(envir, list(a = NULL, b = NULL, c = NULL)),
get_missing_parameters_interactive(c("a", "b", "c"), envir),
"Missing parameters: 'a'")
})


test_that("prompt for missing parameters", {
withr::local_options(orderly_interactive_parameters_missing_error = NULL)
envir <- list2env(list(c = 4), parent = new.env())
mock_get <- mockery::mock(1, 2)
mockery::stub(get_missing_parameters_interactive, "get_parameter_interactive",
mock_get)
expect_message(
get_missing_parameters_interactive(c("a", "b", "c"), envir),
"Please enter values for 2 missing parameters:")
mockery::expect_called(mock_get, 2)
expect_equal(mockery::mock_args(mock_get),
list(list("a", NULL), list("b", NULL)))
expect_mapequal(as.list(envir), list(a = 1, b = 2, c = 4))
})


test_that("parameters must be atomic scalars", {
err <- expect_error(
check_parameters(list(a = NULL, b = 2), list(a = NULL, b = NULL)),
Expand Down
2 changes: 1 addition & 1 deletion vignettes/introduction.Rmd
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ r_output(readLines(file.path(path, "src/random/orderly.R")))
This creates a report that has a single parameter `n_samples` with a default value of 10. We could have used

```r
orderly2::orderly_parameters(n_samples = NULL)`
orderly2::orderly_parameters(n_samples = NULL)
```

to define a parameter with no default, or defined multiple parameters with
Expand Down