Skip to content

Commit

Permalink
Deprecate include_timing (#159)
Browse files Browse the repository at this point in the history
I can't find any evidence of use of GitHub, it was never tested, and it adds a substantial amount of complexity to the code.
  • Loading branch information
hadley authored Jun 20, 2024
1 parent ab6b58c commit 7a68cb9
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 60 deletions.
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)

* `evalute(include_timing)` has been deprecated. I can't find any use of it on GitHub, and it adds substantial code complexity for little gain.
* `watchout()` is no longer exported; it's really an implementation detail that should never have been leaked to the public interface.
* `evaluate()` gains an output class (`evaluate_evaluation`/`list`) and a basic print method.
* `evaluate()` now correctly captures plots created before messages/warnings/errors (#28).
Expand Down
33 changes: 11 additions & 22 deletions R/eval.R
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@
#' processes the output from the evaluation. The default simply prints the
#' visible return values.
#' @param filename string overrriding the [base::srcfile()] filename.
#' @param include_timing if `TRUE`, evaluate will wrap each input
#' expression in `system.time()`, which will be accessed by following
#' `replay()` call to produce timing information for each evaluated
#' command.
#' @param include_timing Deprecated.
#' @import graphics grDevices utils
evaluate <- function(input,
envir = parent.frame(),
Expand All @@ -56,6 +53,10 @@ evaluate <- function(input,
stop_on_error <- as.integer(stop_on_error)
stopifnot(length(stop_on_error) == 1)

if (isTRUE(include_timing)) {
warning("`evaluate(include_timing)` is deprecated")
}

parsed <- parse_all(input, filename, stop_on_error != 2L)
if (inherits(err <- attr(parsed, 'PARSE_ERROR'), 'error')) {
source <- new_source(parsed$src, expression(), output_handler$source)
Expand Down Expand Up @@ -91,8 +92,7 @@ evaluate <- function(input,
keep_warning = keep_warning,
keep_message = keep_message,
log_warning = log_warning,
output_handler = output_handler,
include_timing = include_timing
output_handler = output_handler
)
watcher$check_devices()

Expand Down Expand Up @@ -120,8 +120,7 @@ evaluate_top_level_expression <- function(exprs,
keep_warning = TRUE,
keep_message = TRUE,
log_warning = FALSE,
output_handler = new_output_handler(),
include_timing = FALSE) {
output_handler = new_output_handler()) {
stopifnot(is.expression(exprs))

source <- new_source(src, exprs[[1]], output_handler$source)
Expand Down Expand Up @@ -190,30 +189,20 @@ evaluate_top_level_expression <- function(exprs,
} else {
handle <- force
}
if (include_timing) {
timing_fn <- function(x) system.time(x)[1:3]
} else {
timing_fn <- function(x) {x; NULL}
}

user_handlers <- output_handler$calling_handlers
evaluate_handlers <- list(error = eHandler, warning = wHandler, message = mHandler)
# The user's condition handlers have priority over ours
handlers <- c(user_handlers, evaluate_handlers)

for (expr in exprs) {
srcindex <- length(output)
time <- timing_fn(
ev <- handle(
with_handlers(
withVisible(eval(expr, envir)),
handlers
)
ev <- handle(
with_handlers(
withVisible(eval(expr, envir)),
handlers
)
)
handle_output(TRUE)
if (!is.null(time))
attr(output[[srcindex]]$src, 'timing') <- time

if (show_value(output_handler, ev$visible)) {
# Ideally we'd evaluate the print() generic in envir in order to find
Expand Down
35 changes: 1 addition & 34 deletions R/replay.R
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ replay.character <- function(x) {

#' @export
replay.source <- function(x) {
s <- if (is.null(attr(x$src,'timing'))) '' else render_timing(attr(x$src, 'timing'))
cat(paste0(s, line_prompt(x$src)))
cat(line_prompt(x$src))
}

#' @export
Expand Down Expand Up @@ -70,38 +69,6 @@ replay.recordedplot <- function(x) {
print(x)
}

render_timing <- function(t) {
if (max(t) < 0.5) '' else paste0(
'[', render_sec(t[[1]] + t[[2]]), # User time + Kernel time
',', render_sec(t[[3]]), # Wall time
']'
)
}

render_sec <- function(s) {
if (s < 0.005) return('<5ms')
if (s < 1) return(paste0(round(s,2), 's'))
if (s < 10) return(paste0(round(s,1), 's'))
sec <- round(s,0)
if (sec < 120) return(paste0(sec, 's'))
min <- floor(sec/60)
sec <- sec - min*60
if (min < 10) return(paste0(
min, 'm', formatC(sec, digits = 0, width = 2, format = "f", flag = "0"), 's'
))
min <- min + round(sec/60, 0)
if (min < 120) return(paste0(min, 'm'))
h <- floor(min/60)
min <- min - h * 60
if (h < 48) return(paste0(
h, 'h', formatC(min, digits = 0, width = 2, format = "f", flag = "0"), 'm'
))
d <- floor(h/24)
h <- h - d*24
return(paste0(d, 'd', h, 'h'))
}


format_condition <- function(x) {
if (inherits(x, "rlang_warning") || inherits(x, "rlang_error")) {
format(x)
Expand Down
5 changes: 1 addition & 4 deletions man/evaluate.Rd

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

0 comments on commit 7a68cb9

Please sign in to comment.