Skip to content

Commit

Permalink
Address CRAN issues
Browse files Browse the repository at this point in the history
  • Loading branch information
dipterix committed Nov 5, 2024
1 parent 5a28c7d commit 8f9edee
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 14 deletions.
4 changes: 2 additions & 2 deletions .Rbuildignore
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion R/class_electrode_proto.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions R/ext_media.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 7 additions & 3 deletions R/plot_volume-slices.R
Original file line number Diff line number Diff line change
Expand Up @@ -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) ) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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))
Expand Down
48 changes: 45 additions & 3 deletions cran-comments.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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


Expand Down

0 comments on commit 8f9edee

Please sign in to comment.