Thanks Micheal and Martin,
I have contacted the people from CRAN, and they have let me know that in
this case it is indeed reasonable to set par and not set it back.
I will also think about Martins suggestion. Currently I return an other
object, but I might be able to add the old par-values to that.
Thanks.
Best,
Jan
On 8/22/25 09:14, Martin Maechler wrote:
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
