Merge pull request #1394 from matthew-brett/jupyter-outputs

NEWS.md

@@ -1,5 +1,26 @@
 # reticulate (development version)
 
+- New optional feature: Reticulate now accepts a new option `jupyter_compat`
+  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 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). 
 

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,36 +184,40 @@ 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)
 
     # 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
-
-    # use trailing semicolon to suppress output of return value
-    suppress <- grepl(";\\s*$", snippet)
-    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 output
+    # handle matplotlib and other plot output
     captured <- eng_python_autoprint(
       captured = captured,
-      options  = options,
-      autoshow = i == length(ranges)
+      options  = options
     )
 
+    # 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 <-
       !.engine_context$pending_plots$empty() ||
@@ -214,7 +226,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 +239,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 +275,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)
   }
@@ -455,7 +455,7 @@ 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
     options <- knitr::opts_current$get()
@@ -585,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()
@@ -604,17 +604,11 @@ 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()
-    }
+    # 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("")
 
   } else if (eng_python_is_seaborn_output(value)) {