From b401fcbcb6ab7f13b05c25c4a6427ce8ecd1e62f Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 25 Jul 2023 08:38:52 -0300 Subject: [PATCH 1/4] Still include the stdout output captured before the autoprint mechanism. --- R/knitr-engine.R | 6 +-- .../_snaps/python-knitr-engine/knitr-print.md | 39 +++++++++++++++++++ .../python-knitr-engine/knitr-print2.md | 33 ++++++++++++++++ tests/testthat/resources/knitr-print.Rmd | 25 ++++++++++++ 4 files changed, 100 insertions(+), 3 deletions(-) diff --git a/R/knitr-engine.R b/R/knitr-engine.R index 5251d8c60..5928649d3 100644 --- a/R/knitr-engine.R +++ b/R/knitr-engine.R @@ -197,21 +197,21 @@ eng_python <- function(options) { py_compile_eval("'__reticulate_placeholder__'") # run code and capture output - captured <- if (capture_errors) + captured_stdout <- if (capture_errors) tryCatch(py_compile_eval(snippet, 'single'), error = identity) else py_compile_eval(snippet, 'single') # handle matplotlib plots and other special output captured <- eng_python_autoprint( - captured = captured, + captured = captured_stdout, options = options ) # A trailing ';' suppresses output. # In jupyter mode, only the last expression in a chunk has repr() output. if (grepl(";\\s*$", snippet) | (jupyter_compat & !last_range)) - captured <- "" + captured <- captured_stdout # emit outputs if we have any has_outputs <- diff --git a/tests/testthat/_snaps/python-knitr-engine/knitr-print.md b/tests/testthat/_snaps/python-knitr-engine/knitr-print.md index 330e35834..93bbbfc6a 100644 --- a/tests/testthat/_snaps/python-knitr-engine/knitr-print.md +++ b/tests/testthat/_snaps/python-knitr-engine/knitr-print.md @@ -2,3 +2,42 @@ bt$print("Hello world") ## Hello world + +Both in `jupyter_compat = TRUE` and `jupyter_compat = FALSE` we should +see the results, because a `print` was called: + + print(1) + + ## 1 + + print(2) + + ## 2 + + print(1) + + ## 1 + + print(2) + + ## 2 + +For the `jupyter_compat = FALSE` mode we should see the output of both +expressions. In `jupyter_compat`, we should only see the output for the +last expression. + + 1 + 0 + + ## 1 + + 1 + 1 + + ## 2 + + 1 + 0 + + ## 1 + + 1 + 1 + + ## 2 diff --git a/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md b/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md index 330e35834..ec0a7bf72 100644 --- a/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md +++ b/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md @@ -2,3 +2,36 @@ bt$print("Hello world") ## Hello world + +Both in `jupyter_compat = TRUE` and `jupyter_compat = FALSE` we should +see the results, because a `print` was called: + + print(1) + + ## 1 + + print(2) + + ## 2 + + print(1) + print(2) + + ## 2 + +For the `jupyter_compat = FALSE` mode we should see the output of both +expressions. In `jupyter_compat`, we should only see the output for the +last expression. + + 1 + 0 + + ## 1 + + 1 + 1 + + ## 2 + + 1 + 0 + 1 + 1 + + ## 2 diff --git a/tests/testthat/resources/knitr-print.Rmd b/tests/testthat/resources/knitr-print.Rmd index e0693a546..cb50f1ea9 100644 --- a/tests/testthat/resources/knitr-print.Rmd +++ b/tests/testthat/resources/knitr-print.Rmd @@ -8,3 +8,28 @@ bt <- reticulate::import_builtins() bt$print("Hello world") ``` +Both in `jupyter_compat = TRUE` and `jupyter_compat = FALSE` we should see the +results, because a `print` was called: + +```{python} +print(1) +print(2) +``` + +```{python, jupyter_compat = TRUE} +print(1) +print(2) +``` + +For the `jupyter_compat = FALSE` mode we should see the output of both expressions. +In `jupyter_compat`, we should only see the output for the last expression. + +```{python} +1 + 0 +1 + 1 +``` + +```{python, jupyter_compat = TRUE} +1 + 0 +1 + 1 +``` From f84a1deee87a9176814a545fe40ff155ab17d6cf Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 25 Jul 2023 09:03:21 -0300 Subject: [PATCH 2/4] Go back to using the `compile_mode` so we use the `exec` mode when running from jupyter_compat. --- R/knitr-engine.R | 10 +++++++--- .../_snaps/python-knitr-engine/knitr-print.md | 18 ++++++++++-------- .../_snaps/python-knitr-engine/knitr-print2.md | 18 +++++++++++++----- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/R/knitr-engine.R b/R/knitr-engine.R index 5928649d3..6b39333f1 100644 --- a/R/knitr-engine.R +++ b/R/knitr-engine.R @@ -196,11 +196,15 @@ eng_python <- function(options) { # clear the last value object (so we can tell if it was updated) py_compile_eval("'__reticulate_placeholder__'") + # use trailing semicolon to suppress output of return value + suppress <- grepl(";\\s*$", snippet) || (jupyter_compat & !last_range) + compile_mode <- if (suppress) "exec" else "single" + # run code and capture output captured_stdout <- if (capture_errors) - tryCatch(py_compile_eval(snippet, 'single'), error = identity) + tryCatch(py_compile_eval(snippet, compile_mode), error = identity) else - py_compile_eval(snippet, 'single') + py_compile_eval(snippet, compile_mode) # handle matplotlib plots and other special output captured <- eng_python_autoprint( @@ -210,7 +214,7 @@ eng_python <- function(options) { # A trailing ';' suppresses output. # In jupyter mode, only the last expression in a chunk has repr() output. - if (grepl(";\\s*$", snippet) | (jupyter_compat & !last_range)) + if (suppress) captured <- captured_stdout # emit outputs if we have any diff --git a/tests/testthat/_snaps/python-knitr-engine/knitr-print.md b/tests/testthat/_snaps/python-knitr-engine/knitr-print.md index 93bbbfc6a..a76edeab8 100644 --- a/tests/testthat/_snaps/python-knitr-engine/knitr-print.md +++ b/tests/testthat/_snaps/python-knitr-engine/knitr-print.md @@ -3,8 +3,8 @@ ## Hello world -Both in `jupyter_compat = TRUE` and `jupyter_compat = FALSE` we should -see the results, because a `print` was called: +In `jupyter_compat = FALSE`: we should only see the output of both +expressions. print(1) @@ -14,6 +14,9 @@ see the results, because a `print` was called: ## 2 +In `jupyter_compat = TRUE`: we should only see the output of both +expressions. + print(1) ## 1 @@ -22,9 +25,8 @@ see the results, because a `print` was called: ## 2 -For the `jupyter_compat = FALSE` mode we should see the output of both -expressions. In `jupyter_compat`, we should only see the output for the -last expression. +In `jupyter_compat = FALSE`: we should only see the output of both +expressions. 1 + 0 @@ -34,10 +36,10 @@ last expression. ## 2 - 1 + 0 - - ## 1 +`jupyter_compat = TRUE`: we should only see the output of the last +expression. + 1 + 0 1 + 1 ## 2 diff --git a/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md b/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md index ec0a7bf72..a76edeab8 100644 --- a/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md +++ b/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md @@ -3,8 +3,8 @@ ## Hello world -Both in `jupyter_compat = TRUE` and `jupyter_compat = FALSE` we should -see the results, because a `print` was called: +In `jupyter_compat = FALSE`: we should only see the output of both +expressions. print(1) @@ -14,14 +14,19 @@ see the results, because a `print` was called: ## 2 +In `jupyter_compat = TRUE`: we should only see the output of both +expressions. + print(1) + + ## 1 + print(2) ## 2 -For the `jupyter_compat = FALSE` mode we should see the output of both -expressions. In `jupyter_compat`, we should only see the output for the -last expression. +In `jupyter_compat = FALSE`: we should only see the output of both +expressions. 1 + 0 @@ -31,6 +36,9 @@ last expression. ## 2 +`jupyter_compat = TRUE`: we should only see the output of the last +expression. + 1 + 0 1 + 1 From 61e300f6bff98afa64e6458fe37d49aa3b57d47a Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 25 Jul 2023 09:07:54 -0300 Subject: [PATCH 3/4] Add more tests --- .../_snaps/python-knitr-engine/knitr-print.md | 25 ++++++++++++++ .../python-knitr-engine/knitr-print2.md | 25 ++++++++++++++ tests/testthat/resources/knitr-print.Rmd | 33 ++++++++++++++++--- 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/tests/testthat/_snaps/python-knitr-engine/knitr-print.md b/tests/testthat/_snaps/python-knitr-engine/knitr-print.md index a76edeab8..5493981f0 100644 --- a/tests/testthat/_snaps/python-knitr-engine/knitr-print.md +++ b/tests/testthat/_snaps/python-knitr-engine/knitr-print.md @@ -3,6 +3,8 @@ ## Hello world +## Print statements should always be captured + In `jupyter_compat = FALSE`: we should only see the output of both expressions. @@ -25,6 +27,8 @@ expressions. ## 2 +## Only outputs of last expressions are captured in jupyter compat + In `jupyter_compat = FALSE`: we should only see the output of both expressions. @@ -43,3 +47,24 @@ expression. 1 + 1 ## 2 + +## One can disable outputs by using `;` + +In `jupyter_compat = FALSE`: we should only see the print statement +output + + print("hello"); + + ## hello + + 1 + 0; + 1 + 1; + +`jupyter_compat = TRUE`: we should only see the print statement output + + print("hello"); + + ## hello + + 1 + 0; + 1 + 1; diff --git a/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md b/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md index a76edeab8..5493981f0 100644 --- a/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md +++ b/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md @@ -3,6 +3,8 @@ ## Hello world +## Print statements should always be captured + In `jupyter_compat = FALSE`: we should only see the output of both expressions. @@ -25,6 +27,8 @@ expressions. ## 2 +## Only outputs of last expressions are captured in jupyter compat + In `jupyter_compat = FALSE`: we should only see the output of both expressions. @@ -43,3 +47,24 @@ expression. 1 + 1 ## 2 + +## One can disable outputs by using `;` + +In `jupyter_compat = FALSE`: we should only see the print statement +output + + print("hello"); + + ## hello + + 1 + 0; + 1 + 1; + +`jupyter_compat = TRUE`: we should only see the print statement output + + print("hello"); + + ## hello + + 1 + 0; + 1 + 1; diff --git a/tests/testthat/resources/knitr-print.Rmd b/tests/testthat/resources/knitr-print.Rmd index cb50f1ea9..2073a25a9 100644 --- a/tests/testthat/resources/knitr-print.Rmd +++ b/tests/testthat/resources/knitr-print.Rmd @@ -8,28 +8,53 @@ bt <- reticulate::import_builtins() bt$print("Hello world") ``` -Both in `jupyter_compat = TRUE` and `jupyter_compat = FALSE` we should see the -results, because a `print` was called: +## Print statements should always be captured + +In `jupyter_compat = FALSE`: we should only see the output of both expressions. ```{python} print(1) print(2) ``` +In `jupyter_compat = TRUE`: we should only see the output of both expressions. + ```{python, jupyter_compat = TRUE} print(1) print(2) ``` -For the `jupyter_compat = FALSE` mode we should see the output of both expressions. -In `jupyter_compat`, we should only see the output for the last expression. +## Only outputs of last expressions are captured in jupyter compat + +In `jupyter_compat = FALSE`: we should only see the output of both expressions. ```{python} 1 + 0 1 + 1 ``` +`jupyter_compat = TRUE`: we should only see the output of the last expression. + ```{python, jupyter_compat = TRUE} 1 + 0 1 + 1 ``` + +## One can disable outputs by using `;` + +In `jupyter_compat = FALSE`: we should only see the print statement output + +```{python} +print("hello"); +1 + 0; +1 + 1; +``` + +`jupyter_compat = TRUE`: we should only see the print statement output + +```{python, jupyter_compat = TRUE} +print("hello"); +1 + 0; +1 + 1; +``` + From 70d32f960b57f976ec9f640b85d8b645fdec81ed Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 25 Jul 2023 10:38:34 -0300 Subject: [PATCH 4/4] add test case for interleaved expressions --- .../_snaps/python-knitr-engine/knitr-print.md | 15 +++++++++++++++ .../_snaps/python-knitr-engine/knitr-print2.md | 15 +++++++++++++++ tests/testthat/resources/knitr-print.Rmd | 10 ++++++++++ 3 files changed, 40 insertions(+) diff --git a/tests/testthat/_snaps/python-knitr-engine/knitr-print.md b/tests/testthat/_snaps/python-knitr-engine/knitr-print.md index 5493981f0..fbf7eca1d 100644 --- a/tests/testthat/_snaps/python-knitr-engine/knitr-print.md +++ b/tests/testthat/_snaps/python-knitr-engine/knitr-print.md @@ -68,3 +68,18 @@ output 1 + 0; 1 + 1; + +## `jupyter_compat` works with interleaved expressions + + print('first_stdout') + + ## first_stdout + + 'first_expression' + print('second_stdout') + + ## second_stdout + + 'final_expression' + + ## 'final_expression' diff --git a/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md b/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md index 5493981f0..fbf7eca1d 100644 --- a/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md +++ b/tests/testthat/_snaps/python-knitr-engine/knitr-print2.md @@ -68,3 +68,18 @@ output 1 + 0; 1 + 1; + +## `jupyter_compat` works with interleaved expressions + + print('first_stdout') + + ## first_stdout + + 'first_expression' + print('second_stdout') + + ## second_stdout + + 'final_expression' + + ## 'final_expression' diff --git a/tests/testthat/resources/knitr-print.Rmd b/tests/testthat/resources/knitr-print.Rmd index 2073a25a9..36cd871f6 100644 --- a/tests/testthat/resources/knitr-print.Rmd +++ b/tests/testthat/resources/knitr-print.Rmd @@ -58,3 +58,13 @@ print("hello"); 1 + 1; ``` +## `jupyter_compat` works with interleaved expressions + +```{python, jupyter_compat=TRUE} +print('first_stdout') +'first_expression' +print('second_stdout') +'final_expression' +``` + +