>>>>> Michael Chirico >>>>> on Thu, 21 Aug 2025 12:56:41 -0700 writes:
> I can understand caution about functions with side effects, but rejecting a > package on those grounds alone seems strange to me. > I can easily find thousands of calls to par() on CRAN without any > associated on.exit() cleanup ([1] [2] [3] [4] with a full regex search [5]) > [1] > https://github.com/cran/raster/blob/3f64977b51a3b7c565f1309a462cb35f66daeafe/R/hist.R#L44 > [2] > https://github.com/cran/quantreg/blob/43abb16645d5e78d1377896ebd19d760efd2ace9/R/khmal.R#L107-L115 > [3] > https://github.com/cran/SensoMineR/blob/c9a159cb445e538c953d27de54288897882b3c91/R/barrow.R#L1 > [4] > https://github.com/cran/GD/blob/1ce7c3c8b63dafb1d9d0a9e2ffcd6af4917fa077/R/sesu.R#L25 > [5] > https://github.com/search?q=org%3Acran+%2F%5Cn%5B%5E%23%5D*%5B%5E%5Cw.%5Dpar%5C%28%5B+%5Cn%5D*%5B%5E.%29+%5Cn%22%27%5D%2F+path%3A%2F%5C.R%24%2F+NOT+%22on.exit%22+NOT+path%3A%2Fdemo%2F+NOT+path%3A%2Ftests%2F&type=code > I see no reference to 'par' inside {tools} (in particular QC.R), so R CMD > check is not looking for this. > IMO, clearly documenting the side effects of any such functions is more > than enough. > Mike C I agree. I've also been authoring a package with one function whose *purpose* it is to modify par() conveniently. The function is very old (older than R; I wrote it for S / S-plus in my first post-doc year in 1990: https://search.r-project.org/CRAN/refmans/sfsmisc/html/mult.fig.html It *is* however careful to return the previous par() {in <result>$old.par } such that when you use mult.fig() in a function or an R script, you can easily add an on.exit() in your function or use par(<....>) directly to revert the global par() to its original values. Martin > On Thu, Aug 21, 2025 at 1:45 AM Jan van der Laan <rh...@eoos.dds.nl> wrote: >> >> >> Someone asked for the response I got from CRAN. I included it below: >> >> --- >> [... another issue where I forgot a \value{} in the documentation of one >> of the functions] >> >> Please make sure that you do not change the user's options, par or >> working directory. If you really have to do so within functions, please >> ensure with an *immediate* call of on.exit() that the settings are reset >> when the function is exited. >> e.g.: >> ... >> oldpar <- par(no.readonly = TRUE) # code line i >> on.exit(par(oldpar)) # code line i + 1 >> ... >> par(mfrow=c(2,2)) # somewhere after >> ... >> e.g.: -> R/tgp.R >> If you're not familiar with the function, please check ?on.exit. This >> function makes it possible to restore options before exiting a function >> even if the function breaks. Therefore it needs to be called immediately >> after the option change within a function. >> For more details: >> < >> https://contributor.r-project.org/cran-cookbook/code_issues.html#change-of-options-graphical-parameters-and-working-directory >> > >> >> Please fix and resubmit. >> --- >> >> I understand why this is asked and I also know how to handle this in >> usual cases. I am not sure how to handle this in the specific case here. >> Perhaps I missed some alternative way of handling this. >> >> Best, >> Jan >> >> >> On 8/20/25 21:31, Jan van der Laan wrote: >> > I recently submitted a package to CRAN that creates a device that >> > outputs into the terminal (for terminals that support the Terminal >> > Graphics Protocol (and hopefully in the future also terminals that >> > support sixel)) [1]. >> > >> > It introduces a 'tgp' function that does the following: >> > >> > 1. It opens a ragg::agg_capture device. >> > >> > 2. When term_col = TRUE: sets the background and foreground colors of >> > the device to those of the terminal (when these can be detected). >> > >> > 3. Every time the ragg:agg_capture device is updated: updates the plot >> > in the terminal. >> > >> > Point 2 raises an issue from CRAN as par() changes some properties and >> > these are not set back at the and of the function call. This is >> > intentional, as otherwise setting them would be useless; it is the >> > intention that the colors of subsequent plot calls are changed. I am >> > not sure how I can set the colors of the device to those in the terminal >> > (as requested by the user) without violating this rule. >> > >> > Are there alternative ways of helping the user to set the colors of the >> > device without violating this issue? >> > >> > Could this be an exception to the rule as the argument intentionally >> > changes par()? This could be made more explicit by changing the default >> > value to FALSE. I guess this question is better asked at the CRAN >> > maintainers, but perhaps there are other package that do something >> > similar that I might look at. >> > >> > Instead of having this functionality in the tgp() function, I could >> > create a 'set_figure_colours_to_terminal()` function (perhaps with a >> > corresponding reset function). However, this function would run into the >> > same issue. >> > >> > I appreciate any suggestions/help that yoy could give. >> > >> > Best, >> > Jan >> > >> > >> > >> > >> > [1] The current version of the code can be found here: https:// >> > codeberg.org/djvanderlaan/terminalgraphics/src/branch/dev >> > >> > ______________________________________________ >> > R-package-devel@r-project.org mailing list >> > https://stat.ethz.ch/mailman/listinfo/r-package-devel >> >> ______________________________________________ >> R-package-devel@r-project.org mailing list >> https://stat.ethz.ch/mailman/listinfo/r-package-devel >> > [[alternative HTML version deleted]] > ______________________________________________ > R-package-devel@r-project.org mailing list > https://stat.ethz.ch/mailman/listinfo/r-package-devel ______________________________________________ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel