Do not add outputs when results = 'hide' is set

[cderv]

closes #1599

As mentioned, include is handled by knitr directly since a long time
https://github.com/yihui/knitr/blob/44a7bee566afff3eb4c8b13fd6fa64487e12981f/R/block.R#L350

so I replaced by results = 'hide' information.

It also seems that there are two places where output is handled (because of specific handling for matplotlib, so I added the if check there too

I think this is all that is missing for results to be supported.

If you prefer to keep is_include for safety, then we can add it back in each if

[cderv]

Let's make it a draft for now, because I want to check something regarding results = 'hide' and plot output in knitr with R

[cderv]

Ok so for R, there is a difference between plots and text outputs
https://github.com/yihui/knitr/blob/44a7bee566afff3eb4c8b13fd6fa64487e12981f/R/block.R#L292

results = 'hide' only really applies to evaluated output in R where is.character() is TRUE. So it does not applies to plot.

```{r, results="hide"}
library(ggplot2)

ggplot(mtcars, aes(cyl, mpg)) + geom_point()
```

So it may requires some similar logic for filtering output in the reticulate engine 🤔

but honestly, I think this could be ok to ignore this (at least for now). I don't see how results = "hide" set on chunk would have an expectation of having plots still show up.

[t-kalinowski]

Thanks for the PR!

I don't see how results = "hide" set on chunk would have an expectation of having plots still show up.

Looking at the docs here: https://yihui.org/knitr/options/

results: ('markup'; character) Controls how to display the text results. Note that this option only applies to normal text output (not warnings, messages, or errors).
...
hide (or FALSE): Hide text output.

It is pretty clear that results = 'hide' is meant to only suppresses text output.

I can imagine that some users use that chunk option to suppress unwanted text output, while still displaying plots.

I find it interesting that evaluating a ggplot() object draws to the graphics device still---I think this means that print() is called, since merely calling ggplot() will not draw to the graphics device. Does this mean that the chunk is evaluated "normally", and then the text output is discarded later, after the chunk is fully evaluated?

Should we take a similar approach here?

(Should this options be handled in knitr, like include = FALSE is?)

[cderv]

I can imagine that some users use that chunk option to suppress unwanted text output, while still displaying plots.

That would be the scenario - I agree.

Does this mean that the chunk is evaluated "normally", and then the text output is discarded later, after the chunk is fully evaluated?

knit_print() is called as part of the evaluation, and the filtering for text output happens after the chunk evaluation.
For ggplot, knit_print will be a print.

(Should these options be handled in knitr, like include = FALSE is?)

We do it globally through sew.character() method. This means that if an engine like reticulate returns an object of class character, we do apply results = "hide". The initial problem here is that in some case, reticulate is wrapping some output in output_asis() function, making it a knit_asis object for which sew.knit_asis will apply.

Writing this, I am now seeing this is different behavior than in R engine where we filter output any output for which is.character is TRUE (which include a string of class knit_asis).

Maybe this difference is ok. My understanding is that anything that comes from an external non-R engine will be considered by knitr as text output. There is no non-text object handled in this case. So results = 'hide' would hide everything if we do the same. That is why currently it is handled by the class with sew() method like the rest.
If this option is handled at the engine level, what is considered as text output and will be hidden is the choice of the engine.

Should we take a similar approach here?

What approach do you have in mind related to graphic device ?

[t-kalinowski]

What approach do you have in mind related to graphic device ?

I'm not sure. Python plots from reticulate::eng_python() don't go through the R graphics device, but it would be nice if we could match the behavior of R chunks that do. We currently output Python plots using knitr::include_graphics().

The question I suppose we should resolve: is html or image chunk output considered "text", when determining if the results = 'hide' chunk option applies.

Consider: what if a user changes a global option in their document, which changes the output emitted by a chunk. Output can be either raw html, an svg, an image, or a plot on the R graphics device. But regardless of how it is emitted, the representation in the final rendered document is roughly the same. Does the results = 'hide' option cause the output to be hidden for some of the cases but not others?

Based on the knitr chunk option documentation, my understanding would be that results = 'hide' suppresses "text", in the sense that it only suppresses output that would be otherwise presented in the final rendered document as plain, monospaced, chunk output. Any chunk output that would ultimately be seen by the user's eyeballs in the final report as a "richer" representation is not suppressed. E.g, plots and fancy tables are not suppressed, regardless of if they are included via the R graphics device, or an included image file, or an svg, or as raw html. But then, I would expect such a chunk option to be called output ... 🤔.

I think the best we can aim for is consistency with the behavior of the R engine. If I'm understanding correctly, the knitr R engine with chunk option results = 'hide' does not hide plots or non-ascii tables. We should do the same here, no?

I am open to what you think is best.