-
-
Notifications
You must be signed in to change notification settings - Fork 978
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
error=TRUE
not honored with certain ggplot2 errors
#2363
Comments
@sjspielman That's really a great bug report! Should be fixed in the dev version of evaluate now: remotes::install_github('r-lib/evaluate') Thanks! @cderv If you have time, please help with adding tests to https://github.com/r-lib/evaluate/blob/main/tests/testthat/test-graphics.r At least we should expect evaluate::evaluate('library(ggplot2); ggplot(iris) +
aes(x = Speciess, y = Sepal.Length) +
geom_boxplot()')
evaluate::evaluate('library(ggplot2); ggplot(iris) +
aes(x = Species, y = Sepal.Length) +
geom_bar()') not to contain recorded plots (because the plots are empty). Thanks! BTW, its Github actions may need to be updated, too. |
I can add those tests yet. It is interesting to see though that I can't reproduce on windows the issues in the Rmd file shared. Using dev version of evaluate, I get the no error as expected, and this time empty graphics are not included. Just wanted to share this. I'll add the test for the later case |
@yihui I added test in evaluate for this change you made. |
Thanks!
I didn't get an error (on macOS), either. The HTML output page was generated without an error (no R or Pandoc errors), but the empty images were displayed as the icons for missing images, so I thought we should not generate such plots in the first place. |
Ok. On my side, I had the images and the files, but blank images. Anyway, this is fixed. |
… 0.16 Christophe Dervieux (2): Update GHA workflows (#109) Test that empty ggplot are not recorded (#110) Yihui Xie (3): start the next version fix rstudio/rmarkdown#2363: ignore empty plots that only contain calls to requireNamespace() CRAN release v0.16
This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary. |
Checklist
When filing a bug report, please check the boxes below to confirm that you have provided us with the information we need. Have you:
formatted your issue so it is easier for us to read?
included a minimal, self-contained, and reproducible example?
pasted the output from
xfun::session_info('rmarkdown')
in your issue?upgraded all your packages to their latest versions (including your versions of R, the RStudio IDE, and relevant R packages)?
installed and tested your bug with the development version of the rmarkdown package using
remotes::install_github("rstudio/rmarkdown")
?Bug Report
I am filing this issue because I am observing circumstances where
error=TRUE
is not honored in code chunks. When the chunk containsggplot2
code and the error is related to an incorrectaes()
specification, it seems that rmarkdown is expecting to include an output image associated with that chunk. There should not be any image, becauseerror=TRUE
. Other errors inggplot2
code that are not related to incorrectly specifiedaes()
arguments do not have this problem -error=TRUE
is honored and the document knits. Similarly, base Rgraphics
plots with errors do not prevent knitting.I have included here an Rmarkdown file and its rendered HTML with several chunks that demonstrate what I am seeing. I have commented out the chunks that prevent knitting in spite of
error=TRUE
. When I uncomment these chunks, the document does not render with errors that I include in the comments within.potential_bug.zip
Note that I originally observed this with
rmarkdown 2.14
, so I installed2.14.1
and the problem remained (hence it's in thesessionInfo()
in the rendered HTML.)The Rmarkdown itself contains a
sessionInfo()
chunk relevant to what is in the environment during knitting, but either way I am also pasting mysessionInfo()
from Console below:The text was updated successfully, but these errors were encountered: