Address CRAN issues

Author: dipterix

.Rbuildignore

@@ -21,8 +21,8 @@
 ^inst/threeBrainJS/old
 ^inst/threeBrainJS/src
 ^inst/threeBrainJS/dist/[0-9]+.threebrain-[a-zA-Z0-9]+.js
-^inst/threeBrainJS/dist/threebrain-main\.js\.mapp$
-^inst/threeBrainJS/dist/threebrain-worker\.js\.mapp$
+^inst/threeBrainJS/dist/threebrain-main\.js\.map$
+^inst/threeBrainJS/dist/threebrain-worker\.js\.map$
 ^inst/threeBrainJS/build\.sh
 ^inst/threeBrainJS/package
 ^inst/threeBrainJS/webpack

R/class_electrode_proto.R

@@ -511,7 +511,8 @@ ElectrodePrototype <- R6::R6Class(
       if(with_texture && !is.null(self$.last_texture)) {
         texture_file <- file.path(tempdir(check = TRUE), sprintf("%s.png", private$id))
         grDevices::png(filename = texture_file, units = "px", width = 256, height = 256)
-        graphics::par(mar = c(0,0,0,0))
+        oldpar <- graphics::par(mar = c(0, 0, 0, 0))
+        on.exit({ graphics::par(oldpar) })
         self$preview_texture(...)
         grDevices::dev.off()
         texture_file <- normalizePath(texture_file)

R/ext_media.R

@@ -33,11 +33,11 @@ video_content <- function(path, duration = Inf, time_start = 0, asp_ratio = 16 /
     if( local ){
       # try to download video because path is probably an URL
       url <- path
-      timeout <- getOption("timeout", 60)
-      on.exit({
-        options("timeout" = timeout)
-      })
-      options("timeout" = 6000)
+
+      # 60s is too short to download a video
+      oldopt <- options("timeout" = 6000)
+      on.exit({ options(oldopt) })
+
       path <- tempfile(fileext = '.mp4')
       utils::download.file(url, destfile = path)
       temp <- TRUE

R/plot_volume-slices.R

@@ -54,6 +54,10 @@ plot_slices <- function(
   # overlays <- list.files(root_path, pattern = "\\.nii.gz$", full.names = TRUE)
   # overlay_alpha <- 0.3
 
+  # Make sure `par` is reset on exit
+  oldpar <- graphics::par(no.readonly = TRUE)
+  on.exit({ graphics::par(oldpar) })
+
   title_position <- match.arg(title_position)
 
   if( is.character(volume) ) {
@@ -182,8 +186,6 @@ plot_slices <- function(
 
   pos <- rbind(t(as.matrix(expand.grid(x, x, KEEP.OUT.ATTRS = FALSE))), 0, 1)
 
-  oldpar <- graphics::par(no.readonly = TRUE)
-
   if(!length(nc) || is.na(nc[[1]])) {
     nc <- grDevices::n2mfrow(npts, asp = 1/n_plots)[[2]]
   } else {
@@ -223,14 +225,14 @@ plot_slices <- function(
       padding_top <- 0.8
     }
 
+    # The function calls on.exit({ graphics::par(oldpar) }) so no need to reset here
     graphics::par(
       bg = pal[[1]],
       fg = pal[[length(pal)]],
       col.main = pal[[length(pal)]],
       col.axis = pal[[1]],
       mar = c(0,0,0,0)
     )
-    on.exit({ do.call(graphics::par, oldpar) })
   }
 
   # Calculate plt
@@ -244,6 +246,8 @@ plot_slices <- function(
     ratio <- pin[[1]] / pin[[2]]
     plt <- c( 0, 1, 0.5 - ratio / 2, 0.5 + ratio / 2 )
   }
+
+  # The function calls on.exit({ graphics::par(oldpar) }) so no need to reset here
   adjust_plt <- function(reset = FALSE) {
     if( reset ) {
       graphics::par("plt" = c(0, 1, 0, 1))

cran-comments.md

@@ -3,7 +3,49 @@
 ## 2024-11-01
 **Version 1.2.0 (current)**
 
-To address the `CRAN` issues:
+To address the `CRAN` issues (2024-11-05)
+
+```
+\dontrun{} should only be used if the example really cannot be executed (e.g. because of missing additional software, missing API keys, ...) by the user. That's why wrapping examples in \dontrun{} adds the comment ("# Not run:") as a warning for the user. Does not seem necessary. Please replace \dontrun with \donttest.
+Please unwrap the examples if they are executable in < 5 sec, or replace dontrun{} with \donttest{}.
+-> create_group.Rd; freesurfer_brain.Rd; geom_freemesh.Rd
+```
+
+Thanks, currently there are three `\dontrun{}` because these examples do require users to 
+
+* download additional data and software that are not licensed under, nor built into this package.
+* run the code in interactive sessions
+
+I have also tried my best to provide toy-examples if possible. This package originally has lots of `dontrun`s and I have converted most of them to `donttest` back in version `0.1.2` (see comments down below). I believe these three `dontrun` cases have been left since then.
+
+
+```
+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/class_electrode_proto.R; R/ext_media.R; R/plot_volume-slices.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.
+```
+
+Thanks, I have added `on.exit` to all functions that change `options` and `par`. 
+
+Just in case you miss it, there are multiple lines changing `par` in the function `plot_slices` (`R/plot_volume-slices.R`), and I make sure the `par` remain unchanged via the following two lines at the very beginning.
+
+```r
+oldpar <- graphics::par(no.readonly = TRUE)
+on.exit({ graphics::par(oldpar) })
+```
+
+
+
+
+
+To address the previous `CRAN` issues (2024-11-01)
 
 ```
 URL: https://cran.r-project.org/web/packages/threeBrain/index.html
@@ -22,7 +64,7 @@ Package `ravetools` has been updated and on `CRAN` now. The dependency is cleare
 
 This package (`threeBrain`) is developed out of fun and used in my thesis and later projects. I am the solo developer in this project (wrote 99.99% code). Other contributors are explicitly claimed in the `DESCRIPTION`.
 
-There is one external `JavaScript` library `three-brain-js`. The code is located at `inst/threeBrainJS`. I am also the main maintainer and contributor of that project. The distribution included is a compiled bundle that is released under `MPL-2.0` as a whole. As required, the license file has been included when `mpn` compiles the bundles, see `inst/threeBrainJS/dist`. There might some other external programs, but they can't claim the authorship of the release bundle. Their corresponding license files are included too.
+There is one external `JavaScript` library `three-brain-js`. The code is located at `inst/threeBrainJS`. I am also the main maintainer and contributor of that project. The distribution included is a compiled bundle that is released under `MPL-2.0` as a whole. As required, the license file has been included when `npm` compiles the bundles, see `inst/threeBrainJS/dist`. There might some other external programs, but they can't claim the authorship of the release bundle. Their corresponding license files are included too.
 
 
 Self check: 0 errors | 0 warnings | 1 note
@@ -130,7 +172,7 @@ Please fix and resubmit.
 
 #### Solution:
 
-* exported internal functions needed by exaples
+* exported internal functions needed by examples
 * changed dontrun to donttest