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 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
