knitr: save and process figures from the correct wd

NEWS.md

@@ -4,6 +4,10 @@
   exception type and any exception notes when chunk options
   `error = TRUE` is set (reported in #1520, fixed in #1527).
 
+- Fixed an issue where the knitr python engine would fail to include
+  figures from python chunks if a custom `root.dir` chunk option was set.
+  (reported in #1526, fixed in #1529)
+
 - R external pointers (EXTPTRSXP objects) now round-trip through
   `py_to_r(r_to_py(x))` successfully.
   (reported in #1511, fixed in #1519, contributed by @llaniewski).

R/knitr-engine.R

@@ -410,7 +410,11 @@ eng_python_initialize <- function(options, envir) {
 
 }
 
-eng_python_knit_figure_path <- function(options, suffix = NULL) {
+eng_python_knit_include_graphics <-
+  function(options, suffix = NULL, write_figure = function(path) NULL) {
+
+  # ensure that both the figure file saving code, as well as
+  # knitr::include_graphics(), are run with the correct working directory.
 
   # we need to work in either base.dir or output.dir, depending
   # on which of the two has been requested by the user. (note
@@ -433,29 +437,30 @@ eng_python_knit_figure_path <- function(options, suffix = NULL) {
     number  = number
   )
 
-  # ensure parent path exists
-  lapply(paths, function(path){dir.create(dirname(path), recursive = TRUE, showWarnings = FALSE)})
+  for (path in paths) {
+    # ensure parent path exists
+    dir.create(dirname(path), recursive = TRUE, showWarnings = FALSE)
 
-  # return path
-  paths
+    # write figures
+    write_figure(path)
+  }
+
+  # include the first requested path
+  knitr::include_graphics(paths[1])
 
 }
 
 eng_python_matplotlib_show <- function(plt, options) {
 
-  # get figure path
-  paths <- eng_python_knit_figure_path(options)
+  on.exit(plt$close())
 
-  # save the current figure to all requested devices
-  lapply(paths, function(path){
-    dir.create(dirname(path), recursive = TRUE, showWarnings = FALSE)
-    plt$savefig(path, dpi = options$dpi)
-  })
-
-  plt$close()
-
-  # include the requested path
-  knitr::include_graphics(paths[1])
+  # save, return knitr::include_graphics() wrapped figure path
+  eng_python_knit_include_graphics(
+    options, write_figure = function(path) {
+      # save the current figure to all requested devices
+      plt$savefig(path, dpi = options$dpi)
+    }
+  )
 
 }
 
@@ -688,12 +693,12 @@ eng_python_autoprint <- function(captured, options) {
   } else if (eng_python_is_seaborn_output(value)) {
 
     # get figure path
-    paths <- eng_python_knit_figure_path(options)
-    lapply(paths, function(path) {
+    included_path <- eng_python_knit_include_graphics(
+      options, write_figure = function(path) {
       value$savefig(path)
-    })
+  })
 
-    .engine_context$pending_plots$push(knitr::include_graphics(paths[1]))
+    .engine_context$pending_plots$push(included_path)
     return("")
 
   } else if (inherits(value, "pandas.core.frame.DataFrame")) {
@@ -712,15 +717,16 @@ eng_python_autoprint <- function(captured, options) {
              py_module_available("psutil") &&
              py_module_available("kaleido")) {
 
-    paths <- eng_python_knit_figure_path(options)
-    lapply(paths, function(path) {
-      value$write_image(
-      file   = path,
-      width  = options$out.width.px,
-      height = options$out.height.px
+    included_path <- eng_python_knit_include_graphics(
+      options, write_figure = function(path) {
+        value$write_image(
+          file   = path,
+          width  = options$out.width.px,
+          height = options$out.height.px
+        )
+      }
     )
-    })
-    .engine_context$pending_plots$push(knitr::include_graphics(paths[1]))
+    .engine_context$pending_plots$push(included_path)
     return("")
 
   } else if (eng_python_is_altair_chart(value)) {
@@ -747,11 +753,13 @@ eng_python_autoprint <- function(captured, options) {
       data <- as_r_value(value$to_html(output_div = id))
       .engine_context$pending_plots$push(knitr::raw_html(data))
     } else {
-      paths <- eng_python_knit_figure_path(options)
-      lapply(paths, function(path){
-        value$save(path)
-      })
-      .engine_context$pending_plots$push(knitr::include_graphics(paths[1]))
+
+      included_path <- eng_python_knit_include_graphics(
+        options, write_figure = function(path) {
+          value$save(path)
+        }
+      )
+      .engine_context$pending_plots$push(included_path)
     }
 
     return("")

tests/testthat/resources/test-custom-root-dir.Rmd

@@ -0,0 +1,57 @@
+---
+title: "reticulate"
+---
+
+Change the knit root directory to the temporary R session.
+See https://github.com/rstudio/reticulate/issues/1526 for context.
+
+```{r setup}
+knitr::opts_knit$set("root.dir" = tempdir())
+```
+
+```{r confirm-wd}
+getwd()
+```
+
+```{r configure-python}
+library("reticulate")
+matplotlib <- import("matplotlib")
+```
+
+```{r}
+reticulate::py_config()
+```
+
+The R plot is saved relative to the location of the Rmd file:
+
+```{r r-plot}
+plot(1:10)
+```
+
+The Python plot is also saved relative to the location of the Rmd file (not root.dir):
+
+```{python py-plot}
+import matplotlib.pyplot as plt
+
+x = range(1, 10)
+plt.plot(x, x)
+```
+
+```{r}
+knitr::opts_knit$get("root.dir")
+knitr::opts_knit$get("base.dir") 
+knitr::opts_knit$get("output.dir")
+
+if (length(Sys.glob(paste0(knitr::opts_knit$get("root.dir"), "/figure*/*"))))
+  stop("Figures saved in the wrong dir")
+
+if (!setequal(
+  basename(Sys.glob(paste0(knitr::opts_knit$get("output.dir"), "/figure*/*"))),
+  c("r-plot-1.png", "py-plot-1.png")))
+  stop("Figures not found in expected output.dir")
+
+if( file.exists(file.path(knitr::opts_knit$get("root.dir"), "figure", "r-plot-1.png")))    stop("figure saved in wrong place1")
+if( file.exists(file.path(knitr::opts_knit$get("root.dir"), "figure", "py-plot-1.png")))   stop("figure saved in wrong place2")
+if(!file.exists(file.path(knitr::opts_knit$get("output.dir"), "figure", "r-plot-1.png")))  stop("figure saved in wrong place3")
+if(!file.exists(file.path(knitr::opts_knit$get("output.dir"), "figure", "py-plot-1.png"))) stop("figure saved in wrong place4")
+```

tests/testthat/test-python-knitr-engine.R

@@ -27,6 +27,28 @@ test_that("An R Markdown document can be rendered using reticulate", {
 
 
 
+test_that("Figures saved in correct place with custom root.dir", {
+
+  skip_on_cran()
+  skip_if_not_installed("rmarkdown")
+  skip_if(py_version() < "3.8") # end_lineno attr added in 3.8
+
+  rmd_filepath <- normalizePath(file.path(test_path("resources"),
+                                          "test-custom-root-dir.Rmd"))
+
+  dir.create(dir <- tempfile("reticulate-knitr-test-dir"))
+  owd <- setwd(dir)
+
+  file.copy(rmd_filepath, ".")
+  output <- knitr::knit("test-custom-root-dir.Rmd", quiet = TRUE)
+
+  expect_true(file.exists(output), "test-custom-root-dir.Rmd rendered successfully")
+
+  setwd(owd)
+
+})
+
+
 test_that("In Rmd chunks, comments and output attach to code correctly", {
 
   skip_on_cran()