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

Evaluate print in evaluation environment #202

Merged
merged 5 commits into from
Jul 11, 2024
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
Expand Up @@ -36,4 +36,4 @@ Config/Needs/website: tidyverse/tidytemplate
Config/testthat/edition: 3
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.3.1
RoxygenNote: 7.3.2
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# evaluate (development version)

* The default `value` handler now evaluates print in a child environment of the evaluation environment. This largely makes evaluate easier to test, but should make defining S3 methods for print a little easier (#192).
* `parse_all()` adds a `\n` to the end of every line, even the last one if it didn't have one in the input.
* Setting `ACTIONS_STEP_DEBUG=1` (as in a failing GHA workflow) will automatically set `log_echo` and `log_warning` to `TRUE` (#175).
* New `local_reproducible_output()` helper that sets various options and env vars to help ensure consistency of output across environments.
Expand Down
8 changes: 4 additions & 4 deletions R/evaluate.R
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ evaluate <- function(input,
on_message <- check_keep(keep_message, "keep_message")
on_warning <- check_keep(keep_warning, "keep_warning", log_warning)

output_handler <- output_handler %||% default_output_handler
output_handler <- output_handler %||% evaluate_default_output_handler

if (isTRUE(include_timing)) {
warning("`evaluate(include_timing)` is deprecated")
Expand Down Expand Up @@ -144,9 +144,9 @@ evaluate <- function(input,
with_handlers(
{
for (expr in tle$exprs) {
ev <- withVisible(eval(expr, envir))
watcher$capture_plot_and_output()
watcher$print_value(ev$value, ev$visible)
ev <- withVisible(eval(expr, envir))
watcher$capture_plot_and_output()
watcher$print_value(ev$value, ev$visible, envir)
}
TRUE
},
Expand Down
3 changes: 3 additions & 0 deletions R/output-handler.R
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@
#' * If it has one argument, it called on visible values.
#' * If it has two arguments, it handles all values, with the second
#' argument indicating whether or not the value is visible.
#' * If it has three arguments, it will be called on all values, with the
#' the third argument given the evaluation environment which is needed
#' to look up print methods for S3 objects.
#' @param calling_handlers List of [calling handlers][withCallingHandlers].
#' These handlers have precedence over the exiting handler installed
#' by [evaluate()] when `stop_on_error` is set to 0.
Expand Down
20 changes: 18 additions & 2 deletions R/output.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,31 @@ show_value <- function(handler, visible) {
visible || length(formals(handler$value)) > 1
}

handle_value <- function(handler, value, visible) {
handle_value <- function(handler, value, visible, envir = parent.frame()) {
n_args <- length(formals(handler$value))
if (n_args == 1) {
handler$value(value)
} else if (n_args == 2) {
handler$value(value, visible)
} else if (n_args == 3) {
handler$value(value, visible, envir)
} else {
stop("Value output handler must have one or two arguments")
}
}

render <- function(x) if (isS4(x)) methods::show(x) else print(x)
render <- function(value, visible, envir) {
if (!visible) {
return(invisible())
}

if (isS4(value)) {
methods::show(value)
} else {
# We need to evaluate the print() generic in a child environment of the
# evaluation frame in order to find any methods registered there
print_env <- new.env(parent = envir)
print_env$value <- value
evalq(print(value), envir = print_env)
Comment on lines +49 to +51
Copy link
Member

Choose a reason for hiding this comment

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

👍

}
}
2 changes: 1 addition & 1 deletion R/replay.R
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ replay.list <- function(x) {

#' @export
replay.default <- function(x) {
render(x)
render(x, TRUE, parent.frame())
}

#' @export
Expand Down
8 changes: 2 additions & 6 deletions R/watchout.R
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,11 @@ watchout <- function(handler = new_output_handler(),
capture_output()
}

print_value <- function(value, visible) {
print_value <- function(value, visible, envir) {
if (!show_value(handler, visible))
return()

# Ideally we'd evaluate the print() generic in envir in order to find
# any methods registered in that environment. That, however, is
# challenging and only makes a few tests a little simpler so we don't
# bother.
pv <- withVisible(handle_value(handler, value, visible))
pv <- withVisible(handle_value(handler, value, visible, envir))
capture_plot_and_output()
# If the return value is visible, save the value to the output
if (pv$visible) {
Expand Down
9 changes: 8 additions & 1 deletion R/zzz.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
# used by knitr to avoid overheard of calling new_output_handler() repeatedly
# used evaluate() to avoid overhead of calling new_output_handler() repeatedly
evaluate_default_output_handler <- NULL
# used by knitr, as above, but also for value handler
default_output_handler <- NULL

.onLoad <- function(...) {
evaluate_default_output_handler <<- new_output_handler()

Check warning on line 7 in R/zzz.R

View check run for this annotation

Codecov / codecov/patch

R/zzz.R#L7

Added line #L7 was not covered by tests
default_output_handler <<- new_output_handler()
# Match knitr's expectations
default_output_handler$value <<- function(x) {
render(x, visible = TRUE, envir = parent.frame())
}

Check warning on line 12 in R/zzz.R

View check run for this annotation

Codecov / codecov/patch

R/zzz.R#L10-L12

Added lines #L10 - L12 were not covered by tests
}
3 changes: 3 additions & 0 deletions man/new_output_handler.Rd

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

15 changes: 8 additions & 7 deletions tests/testthat/test-graphics.R
Original file line number Diff line number Diff line change
Expand Up @@ -43,33 +43,34 @@ test_that("base plots in a single expression are captured", {

test_that("captures ggplots", {
skip_if_not_installed("ggplot2")
library(ggplot2)

ev <- evaluate(
"ggplot2::ggplot(mtcars, ggplot2::aes(mpg, wt)) + ggplot2::geom_point()"
"ggplot(mtcars, aes(mpg, wt)) + geom_point()"
)
expect_output_types(ev, c("source", "plot"))

ev <- evaluate(function() {
for (j in 1:2) {
print(ggplot2::ggplot(mtcars, ggplot2::aes(mpg, wt)) + ggplot2::geom_point())
print(ggplot(mtcars, aes(mpg, wt)) + geom_point())
}
})
expect_output_types(ev, c("source", "plot", "plot"))
})

test_that("erroring ggplots should not be recorded", {
skip_if_not_installed("ggplot2")

library(ggplot2)

# error in aesthetics
ev <- evaluate(function() {
ggplot2::ggplot(iris, ggplot2::aes(XXXXXXXXXX, Sepal.Length)) +
ggplot2::geom_boxplot()
ggplot(iris, aes(XXXXXXXXXX, Sepal.Length)) + geom_boxplot()
})
expect_output_types(ev, c("source", "error"))

# error in geom
ev <- evaluate(function() {
ggplot2::ggplot(iris, ggplot2::aes(Species, Sepal.Length)) +
ggplot2::geom_bar()
ggplot(iris, aes(Species, Sepal.Length)) + geom_bar()
})
expect_output_types(ev, c("source", "error"))
})
Expand Down
8 changes: 2 additions & 6 deletions tests/testthat/test-replay.R
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
test_that("replay() should work when print() returns visible NULLs", {
old <- options(prompt = "> ")
on.exit(options(old), add = TRUE)

# need to put S3 method in global namespace otherwise it isn't found
assign("print.FOO_BAR", function(x, ...) NULL, envir = globalenv())
on.exit(rm(print.FOO_BAR, envir = globalenv()), add = TRUE)
withr::local_options(prompt = "> ")

print.FOO_BAR <- function(x, ...) NULL
ret <- evaluate('structure(1, class = "FOO_BAR")')
expect_snapshot(replay(ret))
})
Expand Down
Loading