Skip to content

Commit

Permalink
Prompt for missing parameters
Browse files Browse the repository at this point in the history
  • Loading branch information
richfitz committed Oct 13, 2023
1 parent 9ee980b commit 865edca
Show file tree
Hide file tree
Showing 7 changed files with 167 additions and 15 deletions.
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.6
Version: 1.99.7
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

0 comments on commit 865edca

Please sign in to comment.