Minor clean-up translation vignette setup #531
Conversation
| @@ -21,16 +21,22 @@ knitr::opts_chunk$set( | |||
|
|
|||
| pkgs <- c( | |||
| "dplyr", | |||
| "datawizard", | |||
There was a problem hiding this comment.
No need to check if {datawizard} is available.
| @@ -39,7 +47,7 @@ This vignette can be referred to by citing the following: | |||
|
|
|||
| Patil et al., (2022). datawizard: An R Package for Easy Data Preparation and Statistical Transformations. *Journal of Open Source Software*, *7*(78), 4684, https://doi.org/10.21105/joss.04684 | |||
|
|
|||
| ```{css, echo=FALSE, eval = evaluate_chunk} | |||
| ```{css, echo=FALSE, eval = TRUE} | |||
There was a problem hiding this comment.
No need to eval CSS code conditionally. Doesn't hurt us if it is evaluated.
vignettes/tidyverse_translation.Rmd
Outdated
| @@ -84,10 +92,6 @@ This vignette is largely inspired from `{dplyr}`'s [Getting started vignette](ht | |||
| </div> | |||
|
|
|||
| ```{r, eval = evaluate_chunk} | |||
| library(dplyr) | |||
| library(tidyr) | |||
| library(datawizard) | |||
There was a problem hiding this comment.
We always load all needed libraries in the setup chunk. Happy to revert this if you think this reduces readability of the vignette.
There was a problem hiding this comment.
I think it's better to show explicitly what packages are used so I'd like to keep this chunk (with eval=FALSE since they are loaded in setup)
vignettes/tidyverse_translation.Rmd
Outdated
| @@ -84,10 +92,6 @@ This vignette is largely inspired from `{dplyr}`'s [Getting started vignette](ht | |||
| </div> | |||
|
|
|||
| ```{r, eval = evaluate_chunk} | |||
| library(dplyr) | |||
| library(tidyr) | |||
| library(datawizard) | |||
There was a problem hiding this comment.
I think it's better to show explicitly what packages are used so I'd like to keep this chunk (with eval=FALSE since they are loaded in setup)
vignettes/tidyverse_translation.Rmd
Outdated
| if (can_vignette_be_evaluated) { | ||
| evaluate_chunk <- TRUE | ||
| } else { |
There was a problem hiding this comment.
I don't understand this. Why not put evaluate_chunk <- all_deps_available && getRversion() >= "4.1.0" and remove this if condition?
There was a problem hiding this comment.
Good point. No need for an extra level of indirection.
Hopefully, this slightly improves the setup chunk code, irrespective of whether it solves #527 or not.