From 52deb17fa38bed67a0c5732a0e1f37e87cb8bf55 Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Tue, 20 Jun 2023 14:17:55 +0100 Subject: [PATCH 1/5] First draft of jupyter_compat mode --- R/knitr-engine.R | 78 +++++++++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 34 deletions(-) diff --git a/R/knitr-engine.R b/R/knitr-engine.R index 2d689a77c..ba381f940 100644 --- a/R/knitr-engine.R +++ b/R/knitr-engine.R @@ -144,6 +144,11 @@ eng_python <- function(options) { ranges <- mapply(c, starts, ends, SIMPLIFY = FALSE) } + # Stash some options. + is_hold <- identical(options$results, "hold") + is_include <- isTRUE(options$include) + jupyter_compat <- isTRUE(options$jupyter_compat) + # line index from which source should be emitted pending_source_index <- 1 @@ -156,6 +161,9 @@ eng_python <- function(options) { # 'held' outputs, to be appended at the end (for results = "hold") held_outputs <- stack() + # Outputs to be appended to; these depend on the "hold" option. + outputs_target <- if (is_hold) held_outputs else outputs + # synchronize state R -> Python eng_python_synchronize_before() @@ -176,11 +184,14 @@ eng_python <- function(options) { }, add = TRUE) } + # Flag to signal plt command called, but not yet shown. + .engine_context$matplotlib_pending_show <- FALSE for (i in seq_along(ranges)) { # extract range range <- ranges[[i]] + last_range <- i == length(ranges) # extract code to be run snippet <- extract(code, range) @@ -189,8 +200,12 @@ eng_python <- function(options) { py_compile_eval("'__reticulate_placeholder__'") .engine_context$matplotlib_show_was_called <- FALSE - # use trailing semicolon to suppress output of return value - suppress <- grepl(";\\s*$", snippet) + # In standard mode, calculate output by default, but use trailing semicolon + # to suppress output and check of return value, including plot. In + # jupyter_compat mode, always check output, including plot, but drop + # output further down, if required. + end_semicolon <- grepl(";\\s*$", snippet) + suppress <- if (jupyter_compat) FALSE else end_semicolon compile_mode <- if (suppress) "exec" else "single" # run code and capture output @@ -199,13 +214,17 @@ eng_python <- function(options) { else py_compile_eval(snippet, compile_mode) - # handle matplotlib output + # handle matplotlib and other plot output captured <- eng_python_autoprint( captured = captured, options = options, - autoshow = i == length(ranges) + autoshow = (last_range & !jupyter_compat) ) + # For Jupyter-compat mode, do not show output by default, unless this is + # the last code line, in which case, show if no semicolon. + captured <- if (jupyter_compat & (!last_range | end_semicolon)) "" else captured + # emit outputs if we have any has_outputs <- !.engine_context$pending_plots$empty() || @@ -214,7 +233,7 @@ eng_python <- function(options) { if (has_outputs) { # append pending source to outputs (respecting 'echo' option) - if (!identical(options$echo, FALSE) && !identical(options$results, "hold")) { + if (!identical(options$echo, FALSE) && !is_hold) { extracted <- extract(code, c(pending_source_index, range[2])) if(!identical(options$collapse, TRUE) && identical(options$strip.white, TRUE)) { @@ -227,34 +246,15 @@ eng_python <- function(options) { } # append captured outputs (respecting 'include' option) - if (isTRUE(options$include)) { - - if (identical(options$results, "hold")) { - - # append captured output - if (!identical(captured, "")) - held_outputs$push(captured) - - # append captured images / figures - plots <- .engine_context$pending_plots$data() - for (plot in plots) - held_outputs$push(plot) - .engine_context$pending_plots$clear() - - } else { - - # append captured output - if (!identical(captured, "")) - outputs$push(captured) - - # append captured images / figures - plots <- .engine_context$pending_plots$data() - for (plot in plots) - outputs$push(plot) - .engine_context$pending_plots$clear() - - } - + if (is_include) { + # append captured output + if (!identical(captured, "")) + outputs_target$push(captured) + + # append captured images / figures + for (plot in .engine_context$pending_plots$data()) + outputs_target$push(plot) + .engine_context$pending_plots$clear() } # update pending source range @@ -282,8 +282,15 @@ eng_python <- function(options) { outputs$push(output) } + if (.engine_context$matplotlib_pending_show & is_include) { + plt <- import("matplotlib.pyplot", convert = TRUE) + plt$show() + for (plot in .engine_context$pending_plots$data()) + outputs_target$push(plot) + } + # if we were using held outputs, we just inject the source in now - if (identical(options$results, "hold")) { + if (is_hold) { output <- structure(list(src = code), class = "source") outputs$push(output) } @@ -456,6 +463,7 @@ eng_python_initialize_matplotlib <- function(options, envir) { plt$show <- function(...) { .engine_context$matplotlib_show_was_called <- TRUE + .engine_context$matplotlib_pending_show = FALSE # get current chunk options options <- knitr::opts_current$get() @@ -613,6 +621,8 @@ eng_python_autoprint <- function(captured, options, autoshow) { if (autoshow && !.engine_context$matplotlib_show_was_called) { plt <- import("matplotlib.pyplot", convert = TRUE) plt$show() + } else if (isTRUE(options$jupyter_compat)) { + .engine_context$matplotlib_pending_show <- TRUE } return("") From a85d0bc6f4359d1c1b6c06f8858463c14764870f Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Tue, 20 Jun 2023 17:56:59 +0100 Subject: [PATCH 2/5] Add NEWS entry --- NEWS.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/NEWS.md b/NEWS.md index c3d9158e4..5e97fc792 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,20 @@ # reticulate (development version) +- New optional feature: Reticulate now accepts a new option `jupyter_compat` + set to `FALSE` by default, that changes the default display behavior of + Reticulate chunks, to better match the behavior of Jupyter. In the + Reticulate default, each standalone code expression in the code chunk that + does not end in a semi-colon, generates display of the expression output. For + Matplotlib plots, an explicit `plt.show()` generates the figure, but + otherwise, the chunk (by default) only generates a figure if the last line in + the chunk is an expression returning a recognized Matplotlib object, and + without a semi-colon. With `jupyter_compat=TRUE`, a standalone expression + returning a Matplotlib object, anywhere in the chunk, will cause the plot to + be displayed automatically after the chunk. No expression in the chunk will + generate output, except if there is a standalone expression as the last code + statement in the chunk, and that expression does not have a semicolon + — a semicolon suppresses the output, as it does for the default behavior. + - Fix: the knitr engine now automatically calls `plt.show()` for matplotlib bar plots, like it does for other matplotlib plot types (#1391). From f2fa4079f4ca4ed7252e0f565152f23738fbc4d2 Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Tue, 20 Jun 2023 20:00:12 +0100 Subject: [PATCH 3/5] Add link to NEWS entry. --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 5e97fc792..bfa37a08e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -14,6 +14,8 @@ generate output, except if there is a standalone expression as the last code statement in the chunk, and that expression does not have a semicolon — a semicolon suppresses the output, as it does for the default behavior. + See [PR](https://github.com/rstudio/reticulate/pull/1394) and [original + issue](https://github.com/rstudio/reticulate/issues/1391). - Fix: the knitr engine now automatically calls `plt.show()` for matplotlib bar plots, like it does for other matplotlib plot types (#1391). From 14d8467e7fba6ab3683d42128fa64831163fbb68 Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Tue, 20 Jun 2023 21:59:10 +0100 Subject: [PATCH 4/5] Simplify logic for output suppression. --- R/knitr-engine.R | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/R/knitr-engine.R b/R/knitr-engine.R index ba381f940..58d981c4e 100644 --- a/R/knitr-engine.R +++ b/R/knitr-engine.R @@ -200,19 +200,11 @@ eng_python <- function(options) { py_compile_eval("'__reticulate_placeholder__'") .engine_context$matplotlib_show_was_called <- FALSE - # In standard mode, calculate output by default, but use trailing semicolon - # to suppress output and check of return value, including plot. In - # jupyter_compat mode, always check output, including plot, but drop - # output further down, if required. - end_semicolon <- grepl(";\\s*$", snippet) - suppress <- if (jupyter_compat) FALSE else end_semicolon - compile_mode <- if (suppress) "exec" else "single" - # run code and capture output captured <- if (capture_errors) - tryCatch(py_compile_eval(snippet, compile_mode), error = identity) + tryCatch(py_compile_eval(snippet, 'single'), error = identity) else - py_compile_eval(snippet, compile_mode) + py_compile_eval(snippet, 'single') # handle matplotlib and other plot output captured <- eng_python_autoprint( @@ -221,9 +213,12 @@ eng_python <- function(options) { autoshow = (last_range & !jupyter_compat) ) - # For Jupyter-compat mode, do not show output by default, unless this is - # the last code line, in which case, show if no semicolon. - captured <- if (jupyter_compat & (!last_range | end_semicolon)) "" else captured + # In all modes, code statements ending in semicolons always suppress repr + # output. In jupyter_compat mode, also suppress repr output for all + # but the final expression. + if ((grepl(";\\s*$", snippet)) | (jupyter_compat & !last_range)) { + captured = "" + } # emit outputs if we have any has_outputs <- @@ -625,6 +620,7 @@ eng_python_autoprint <- function(captured, options, autoshow) { .engine_context$matplotlib_pending_show <- TRUE } + # Always suppress Matplotlib reprs return("") } else if (eng_python_is_seaborn_output(value)) { From 72327454d2e99246485d068ee267e797c003f8e0 Mon Sep 17 00:00:00 2001 From: Matthew Brett Date: Wed, 21 Jun 2023 15:07:02 +0100 Subject: [PATCH 5/5] Only retain pending plots method for plot display Drop the older method of autoshow, and document the loss of the ability to suppress plots with semicolons. --- NEWS.md | 32 ++++++++++++++++++-------------- R/knitr-engine.R | 22 +++++----------------- 2 files changed, 23 insertions(+), 31 deletions(-) diff --git a/NEWS.md b/NEWS.md index bfa37a08e..2958cab1b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,21 +1,25 @@ # reticulate (development version) - New optional feature: Reticulate now accepts a new option `jupyter_compat` - set to `FALSE` by default, that changes the default display behavior of - Reticulate chunks, to better match the behavior of Jupyter. In the - Reticulate default, each standalone code expression in the code chunk that - does not end in a semi-colon, generates display of the expression output. For - Matplotlib plots, an explicit `plt.show()` generates the figure, but - otherwise, the chunk (by default) only generates a figure if the last line in - the chunk is an expression returning a recognized Matplotlib object, and - without a semi-colon. With `jupyter_compat=TRUE`, a standalone expression - returning a Matplotlib object, anywhere in the chunk, will cause the plot to - be displayed automatically after the chunk. No expression in the chunk will + set to `FALSE` by default, that changes the default expression output display + behavior of Reticulate chunks, to better match the behavior of Jupyter. In + the Reticulate default, each standalone code expression in the code chunk + that does not end in a semi-colon, generates display of the expression + output. With the `jupyter_compat` option set, no expression in the chunk will generate output, except if there is a standalone expression as the last code - statement in the chunk, and that expression does not have a semicolon - — a semicolon suppresses the output, as it does for the default behavior. - See [PR](https://github.com/rstudio/reticulate/pull/1394) and [original - issue](https://github.com/rstudio/reticulate/issues/1391). + statement in the chunk, and that expression does not have a semicolon. + A semicolon always suppresses the expression output, for the default and + `jupyter_compat` case. See + [PR](https://github.com/rstudio/reticulate/pull/1394) and [original + issue](https://github.com/rstudio/reticulate/issues/1391) for discussion for + this and the next item. + +- Behavior change: Previously, a Matplotlib plot would only be automatically + displayed (without `plt.show()`) if there was a final standalone expression + returning a Matplotlib object, and that expression did not have a final + semicolon. With this update, any standalone expression returning + a Matplotlib object, with or without a semicolon, will cause chunk to display + the plot automatically. See above for discussion. - Fix: the knitr engine now automatically calls `plt.show()` for matplotlib bar plots, like it does for other matplotlib plot types (#1391). diff --git a/R/knitr-engine.R b/R/knitr-engine.R index 58d981c4e..4d80dffa6 100644 --- a/R/knitr-engine.R +++ b/R/knitr-engine.R @@ -198,7 +198,6 @@ eng_python <- function(options) { # clear the last value object (so we can tell if it was updated) py_compile_eval("'__reticulate_placeholder__'") - .engine_context$matplotlib_show_was_called <- FALSE # run code and capture output captured <- if (capture_errors) @@ -209,8 +208,7 @@ eng_python <- function(options) { # handle matplotlib and other plot output captured <- eng_python_autoprint( captured = captured, - options = options, - autoshow = (last_range & !jupyter_compat) + options = options ) # In all modes, code statements ending in semicolons always suppress repr @@ -457,7 +455,6 @@ eng_python_initialize_matplotlib <- function(options, envir) { # override show implementation plt$show <- function(...) { - .engine_context$matplotlib_show_was_called <- TRUE .engine_context$matplotlib_pending_show = FALSE # get current chunk options @@ -588,7 +585,7 @@ eng_python_altair_chart_id <- function(options, ids) { } -eng_python_autoprint <- function(captured, options, autoshow) { +eng_python_autoprint <- function(captured, options) { # bail if no new value was produced by interpreter value <- py_last_value() @@ -607,18 +604,9 @@ eng_python_autoprint <- function(captured, options, autoshow) { if (eng_python_is_matplotlib_output(value)) { - # by default, we suppress "side-effect" outputs from matplotlib - # objects; only when 'autoshow' is set will we try to render the - # associated matplotlib plot - # - # handle matplotlib output. note that the default hook installed by - # reticulate will update the 'pending_plots' item - if (autoshow && !.engine_context$matplotlib_show_was_called) { - plt <- import("matplotlib.pyplot", convert = TRUE) - plt$show() - } else if (isTRUE(options$jupyter_compat)) { - .engine_context$matplotlib_pending_show <- TRUE - } + # Handle matplotlib output. Note that the default hook for plt.show + # installed by reticulate will update the 'pending_plots' item. + .engine_context$matplotlib_pending_show <- TRUE # Always suppress Matplotlib reprs return("")