Re: [Rd] Bug (?) in vignette handling
On 29/10/2021 5:52 a.m., Martin Maechler wrote:
Duncan Murdoch
on Thu, 28 Oct 2021 13:18:54 -0400 writes:
> This StackOverflow post: https://stackoverflow.com/q/69756236/2554330
> points out that objects created in one vignette are available in a later
> vignette. I don't think this should be happening: vignettes should be
> self-contained.
I strongly agree.
> The current answer there, https://stackoverflow.com/a/69758025/2554330,
> suggests that "R CMD check" will detect this. However, sometimes one
> vignette can replace a standard function with a custom version, and then
> both will work without generating an error, but the second vignette
> won't do the same thing if run independently.
> For example, try these pure Sweave vignettes:
> -
> aaa3.Rnw:
> -
> \documentclass{article}
> %\VignetteIndexEntry{Sweave aaa3}
> \begin{document}
> <<>>=
> mean <- function(x) "I am the Sweave mean"
> @
> \end{document}
>
> aaa4.Rnw:
>
> \documentclass{article}
> %\VignetteIndexEntry{Sweave aaa4}
> \begin{document}
> <<>>=
> mean(1:5)
> @
> \end{document}
> Put these in a package, build and install the package, and you'll see
> that the mean() function in aaa4.Rnw prints the result from the
> redefined mean in aaa3.Rnw.
Is it because R is *not* run with --no-save --no-restore
accidentally?
Without looking, I would not expect that the vignettes are run
inside the same running R (even though that may speedup things)
I think for R CMD build they are run in one process, while for R CMD
check they are in separate processes. R CMD build runs
tools::buildVignettes(), which runs code that's part of the vignette
build engine.
The Sweave engine evaluates things in .GlobalEnv, so any leftover
objects will be visible there for the next vignette. I think it's up to
the writer of each vignette engine whether there's any cleanup, but it
appears that neither Sweave nor knitr does any.
One possible fix would be for buildVignettes() to make a snapshot of
what's in .GlobalEnv before processing any vignettes, and restoring it
after each one. I've tried a weaker version of this: it records the
names in .GlobalEnv at the start, and deletes anything new before
processing each vignette. So vignettes could modify or delete what's
there, but not add anything.
I think you don't want to completely clear out .GlobalEnv, because
people might choose to run buildVignettes() in an R session and expect
the vignettes to see the contents there.
"make check" in R-devel doesn't complain about this change, but I'll let
R Core decide whether it's a good idea or not. A patch is below.
Duncan Murdoch
Index: src/library/tools/R/Vignettes.R
===
--- src/library/tools/R/Vignettes.R (revision 81110)
+++ src/library/tools/R/Vignettes.R (working copy)
@@ -560,7 +560,11 @@
sourceList <- list()
startdir <- getwd()
fails <- character()
+# People may build vignettes from a session and expect
+# to see some variables, so we won't delete these
+existingVars <- ls(.GlobalEnv, all = TRUE)
for(i in seq_along(vigns$docs)) {
+ rm(list = setdiff(ls(.GlobalEnv, all = TRUE), existingVars), envir
= .GlobalEnv)
thisOK <- TRUE
file <- basename(vigns$docs[i])
enc <- vigns$encodings[i]
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] Bug (?) in vignette handling
Am 30.10.21 um 20:28 schrieb Duncan Murdoch:
On 29/10/2021 5:52 a.m., Martin Maechler wrote:
Duncan Murdoch
on Thu, 28 Oct 2021 13:18:54 -0400 writes:
> This StackOverflow post:
https://stackoverflow.com/q/69756236/2554330
> points out that objects created in one vignette are available
in a later
> vignette. I don't think this should be happening: vignettes
should be
> self-contained.
I strongly agree.
> The current answer there,
https://stackoverflow.com/a/69758025/2554330,
> suggests that "R CMD check" will detect this. However,
sometimes one
> vignette can replace a standard function with a custom version,
and then
> both will work without generating an error, but the second
vignette
> won't do the same thing if run independently.
> For example, try these pure Sweave vignettes:
> -
> aaa3.Rnw:
> -
> \documentclass{article}
> %\VignetteIndexEntry{Sweave aaa3}
> \begin{document}
> <<>>=
> mean <- function(x) "I am the Sweave mean"
> @
> \end{document}
>
> aaa4.Rnw:
>
> \documentclass{article}
> %\VignetteIndexEntry{Sweave aaa4}
> \begin{document}
> <<>>=
> mean(1:5)
> @
> \end{document}
> Put these in a package, build and install the package, and
you'll see
> that the mean() function in aaa4.Rnw prints the result from the
> redefined mean in aaa3.Rnw.
Is it because R is *not* run with --no-save --no-restore
accidentally?
Without looking, I would not expect that the vignettes are run
inside the same running R (even though that may speedup things)
I think for R CMD build they are run in one process, while for R CMD
check they are in separate processes. R CMD build runs
tools::buildVignettes(), which runs code that's part of the vignette
build engine.
Thankfully R CMD check has been building the vignettes in separate R
processes already since R 3.6.0, so has hopefully identified most
problems until now. The corresponding env var is
_R_CHECK_BUILD_VIGNETTES_SEPARATELY_.
The standard (and exported!) buildVignettes() has been weaving all
vignettes in the same session ever since it was added back in 2002. This
approach is probably more efficient (avoiding repetitive package
loading), but carry-over effects seem both likely and undesirable
(thinking of vignettes as separate and independently reproducible
manuscripts about different aspects of a package). AFAICS, it is not
explicitly documented that buildVignettes() runs all vignettes in the
same R session, so at least this is no advertised feature.
The Sweave engine evaluates things in .GlobalEnv, so any leftover
objects will be visible there for the next vignette. I think it's up to
the writer of each vignette engine whether there's any cleanup, but it
appears that neither Sweave nor knitr does any.
I think this is by design and also useful in interactive sessions to
investigate the environment after weaving.
One possible fix would be for buildVignettes() to make a snapshot of
what's in .GlobalEnv before processing any vignettes, and restoring it
after each one. I've tried a weaker version of this: it records the
names in .GlobalEnv at the start, and deletes anything new before
processing each vignette. So vignettes could modify or delete what's
there, but not add anything.
I think you don't want to completely clear out .GlobalEnv, because
people might choose to run buildVignettes() in an R session and expect
the vignettes to see the contents there.
"make check" in R-devel doesn't complain about this change, but I'll let
R Core decide whether it's a good idea or not. A patch is below.
Clearing the workspace would be an improvement, but I think it would be
even better for R CMD build to produce each vignette in a clean R
session, especially with regard to loaded packages. Changing
buildVignettes() to use clean R processes by default (I'd say even if
there is only one vignette) should be considered. I'd appreciate seeing
this report in Bugzilla to investigate further (and not forget).
Best regards,
Sebastian Meyer
Duncan Murdoch
Index: src/library/tools/R/Vignettes.R
===
--- src/library/tools/R/Vignettes.R (revision 81110)
+++ src/library/tools/R/Vignettes.R (working copy)
@@ -560,7 +560,11 @@
sourceList <- list()
startdir <- getwd()
fails <- character()
+ # People may build vignettes from a session and expect
+ # to see some variables, so we won't delete these
+ existingVars <- ls(.GlobalEnv, all = TRUE)
for(i in seq_along(vigns$docs)) {
+ rm(list = setdiff(ls(.GlobalEnv, all = TRUE), existingVars),
envir = .GlobalEnv)
thisOK <- TRUE
file <- basename(vigns$docs[i])
Re: [Rd] Fwd: Using existing envars in Renviron on friendly Windows
> ... If one still needed backslashes,
> they could then be entered in single quotes, e.g. VAR='c:\users'.
I don't think it matters whether you use single or double quotes -
both will work. Here's a proof of concept on Linux with R 4.1.1:
$ cat ./.Renviron
A=C:\users
B='C:\users'
C="C:\users"
$ Rscript -e "Sys.getenv(c('A', 'B', 'C'))"
A B C
"C:users" "C:\\users" "C:\\users"
/Henrik
On Wed, Oct 27, 2021 at 11:45 AM Tomas Kalibera
wrote:
>
>
> On 10/21/21 5:18 PM, Martin Maechler wrote:
> >> Michał Bojanowski
> >> on Wed, 20 Oct 2021 16:31:08 +0200 writes:
> > > Hello Tomas,
> > > Yes, that's accurate although rather terse, which is perhaps the
> > > reason why I did not realize it applies to my case.
> >
> > > How about adding something in the direction of:
> >
> > > 1. Continuing the cited paragraph with:
> > > In particular, on Windows it may be necessary to quote references to
> > > existing environment variables, especially those containing file
> > paths
> > > (which include backslashes). For example: `"${WINVAR}"`.
> >
> > > 2. Add an example (not run):
> >
> > > # On Windows do quote references to variables containing paths, e.g.:
> > > # If APPDATA=C:\Users\foobar\AppData\Roaming
> > > # to point to a library tree inside APPDATA in .Renviron use
> > > R_LIBS_USER="${APPDATA}"/R-library
> >
> > > Incidentally the last example is on backslashes too.
> >
> >
> > > What do you think?
> >
> > I agree that adding an example really helps a lot in such cases,
> > in my experience, notably if it's precise enough to be used +/- directly.
>
> Yes, I agree as well. I think the Renviron files should be written in a
> way so that they would work the same in a POSIX shell, so e.g.
> VAR="${VAR0}" or VAR="${VAR0}/subdir" are the recommended ways to
> preserve backslashes in VAR0. It is better to use forward slashes in
> string literals, e.g. VAR="c:/users". If one still needed backslashes,
> they could then be entered in single quotes, e.g. VAR='c:\users'.
>
> The currently implemented parsing of Renviron files differs in a number
> of details from POSIX shells, some are documented and some are not.
> Relying only on the documented behavior that is the same as in POSIX
> shells is the best choice for future compatibility.
>
> Tomas
>
> >
> >
> > > On Mon, Oct 18, 2021 at 5:02 PM Tomas Kalibera
> > wrote:
> > >>
> > >>
> > >> On 10/15/21 6:44 PM, Michał Bojanowski wrote:
> > >> > Perhaps a small update to ?.Renviron would be in order to mention
> > that...
> > >>
> > >> Would you have a more specific suggestion how to update the
> > >> documentation? Please note that it already says
> > >>
> > >> "‘value’ is then processed in a similar way to a Unix shell: in
> > >> particular the outermost level of (single or double) quotes is
> > stripped,
> > >> and backslashes are removed except inside quotes."
> > >>
> > >> Thanks,
> > >> Tomas
> > >>
> > >> > On Fri, Oct 15, 2021 at 6:43 PM Michał Bojanowski
> > wrote:
> > >> >> Indeed quoting works! Kevin suggested the same, but he didnt
> > reply to the list.
> > >> >> Thank you all!
> > >> >> Michal
> > >> >>
> > >> >> On Fri, Oct 15, 2021 at 6:40 PM Ivan Krylov
> > wrote:
> > >> >>> Sorry for the noise! I wasn't supposed to send my previous
> > message.
> > >> >>>
> > >> >>> On Fri, 15 Oct 2021 16:44:28 +0200
> > >> >>> Michał Bojanowski wrote:
> > >> >>>
> > >> AVAR=${APPDATA}/foo/bar
> > >>
> > >> Which is a documented way of referring to existing environment
> > >> variables. Now, with that in R I'm getting:
> > >>
> > >> Sys.getenv("APPDATA")# That works OK
> > >> [1] "C:\\Users\\mbojanowski\\AppData\\Roaming"
> > >>
> > >> so OK, but:
> > >>
> > >> Sys.getenv("AVAR")
> > >> [1] "C:UsersmbojanowskiAppDataRoaming/foo/bar"
> > >> >>> Hmm, a function called by readRenviron does seem to remove
> > backslashes,
> > >> >>> but not if they are encountered inside quotes:
> > >> >>>
> > >> >>>
> > https://github.com/r-devel/r-svn/blob/3f8b75857fb1397f9f3ceab6c75554e1a5386adc/src/main/Renviron.c#L149
> > >> >>>
> > >> >>> Would AVAR="${APPDATA}"/foo/bar work?
> > >> >>>
> > >> >>> --
> > >> >>> Best regards,
> > >> >>> Ivan
> > >> > __
> > >> > R-devel@r-project.org mailing list
> > >> > https://stat.ethz.ch/mailman/listinfo/r-devel
> >
> > > __
> > > R-devel@r-project.org mailing list
> > > https://stat.ethz.ch/mailman/listinfo/r-devel
>
> __
> R-devel@r-project.org mailing li
