Re: [Bioc-devel] Possible problems with BiocParallel and R cmd check on Github
Yes, I've added the line of code you suggested to the workflows and it
solves the problems for Windows, thank you very much! Since it appears
to be a common problem for macOS we will proceed with submission then,
thank you again (Sorry for the reply all)
Il 30/07/2020 11:47, Henrik Bengtsson ha scritto:
(I assume you just forgot to 'Reply All' so I've added bioc-devel back
to the cc:)
Unfortunately what I'm mentioning only happens in Github actions, which
are the standard ones (we used usethis::use_github_action_check_standard),
I'd say that that 'usethis' setup did not anticipate package tests
that run parallel code. So, don't assume it's perfect and that it
covers all use cases.
we haven't modified the action ...
That's actually my suggestion - did you try my one-line addition? I'm
95% certain it will solve your check errors on Windows as it did for
me.
... It also appears same actions performed on MacOS have
some kind of problems since they stop even before checking the package.
Yes, there seems to be some hiccups. I also get those since a few
days back. These are out of our control and we just have to wait for
them to resolved upstream/elsewhere.
/Henrik
On Thu, Jul 30, 2020 at 2:05 AM Giulia Pais wrote:
Thanks for the reply,
Unfortunately what I'm mentioning only happens in Github actions, which
are the standard ones (we used
usethis::use_github_action_check_standard), we haven't modified the
action, errors I'm mentioning do not happen in local R environments on
windows machines. It also appears same actions performed on MacOS have
some kind of problems since they stop even before checking the package.
Is it relevant for a bioconductor reviewer the result of these Github
action reports? We would like to submit the package for moderation soon
but we're unsure if we can due to this problem. Thank you.
Il 29/07/2020 19:25, Henrik Bengtsson ha scritto:
From a very quick look at this, I think you also need to explicitly
install the package itself for it to be available in external R
session (contrary to when using forked processing as on Linux and
macOS). Something like this:
- name: Install dependencies
run: |
remotes::install_deps(dependencies = TRUE)
remotes::install_cran("rcmdcheck")
install.packages(".", repos=NULL, type="source") ## needed by
parallel workers
shell: Rscript {0}
That's what I had to do when testing 'future'
(https://github.com/HenrikBengtsson/future/blob/1835a122764bbc0196c78be6da6973c8063922b3/.github/workflows/R-CMD-check.yaml#L69).
/Henrik
On Wed, Jul 29, 2020 at 10:18 AM Giulia Pais wrote:
Hi bioconductor team,
we are currently developing a package for future submission on
bioconductor which you can find here
https://github.com/calabrialab/ISAnalytics.
We use Github actions to keep track of R cmd checks at every commit, and
this time, surprisingly for us, we had a failure on R cmd checks for
windows (which is odd, since we develop on windows and performing check
on 2 different windows machines didn't raise any error or warning) and
we suspect this could be tied to the use of BiocParallel. For Windows,
in fact, we use SnowParam instead of MulticoreParam and as the vignette
and manual of BiocParallel specifies we must ensure that every worker is
loaded with the proper dependencies, but apparently this is not
necessary if the function to execute in bplapply belongs to the package
in question. Here is the code of the function that might raise some
problems:
.import_type <- function(q_type, files, workers) {
files <- files %>% dplyr::filter(.data$Quantification_type == q_type)
# Register backend according to platform
if (.Platform$OS.type == "windows") {
p <- BiocParallel::SnowParam(workers = workers, stop.on.error = FALSE)
} else {
p <- BiocParallel::MulticoreParam(workers = workers, stop.on.error
= FALSE)
}
# Import every file
suppressMessages(suppressWarnings({
matrices <- BiocParallel::bptry(
BiocParallel::bplapply(files$Files_chosen, FUN = function(x) {
matrix <- ISAnalytics::import_single_Vispa2Matrix(x)
}, BPPARAM = p)
)
}))
BiocParallel::bpstop(p)
correct <- BiocParallel::bpok(matrices)
imported_files <- files %>% dplyr::mutate(Imported = correct)
matrices <- matrices[correct]
# Bind rows in single tibble for all files
if (purrr::is_empty(matrices)) {
return(NULL)
}
matrices <- purrr::reduce(matrices, function(x, y) {
x %>% dplyr::bind_rows(y) %>% dplyr::distinct()
})
list(matrices, imported_files)
}
The report of the Github action can be found here:
https://github.com/calabrialab/ISAnalytics/runs/923261561
The check apparently fails with these warnings: Warning - namespace
'ISAnalytics' is not available and has been replaced. We tried adding
'library(ISAnalytics)' and 'require(ISAnalytics)' but if we do that
BiocCheck fails with a
Re: [Bioc-devel] Possible problems with BiocParallel and R cmd check on Github
(I assume you just forgot to 'Reply All' so I've added bioc-devel back
to the cc:)
> Unfortunately what I'm mentioning only happens in Github actions, which
> are the standard ones (we used usethis::use_github_action_check_standard),
I'd say that that 'usethis' setup did not anticipate package tests
that run parallel code. So, don't assume it's perfect and that it
covers all use cases.
> we haven't modified the action ...
That's actually my suggestion - did you try my one-line addition? I'm
95% certain it will solve your check errors on Windows as it did for
me.
> ... It also appears same actions performed on MacOS have
> some kind of problems since they stop even before checking the package.
Yes, there seems to be some hiccups. I also get those since a few
days back. These are out of our control and we just have to wait for
them to resolved upstream/elsewhere.
/Henrik
On Thu, Jul 30, 2020 at 2:05 AM Giulia Pais wrote:
>
> Thanks for the reply,
>
> Unfortunately what I'm mentioning only happens in Github actions, which
> are the standard ones (we used
> usethis::use_github_action_check_standard), we haven't modified the
> action, errors I'm mentioning do not happen in local R environments on
> windows machines. It also appears same actions performed on MacOS have
> some kind of problems since they stop even before checking the package.
>
> Is it relevant for a bioconductor reviewer the result of these Github
> action reports? We would like to submit the package for moderation soon
> but we're unsure if we can due to this problem. Thank you.
>
> Il 29/07/2020 19:25, Henrik Bengtsson ha scritto:
> > From a very quick look at this, I think you also need to explicitly
> > install the package itself for it to be available in external R
> > session (contrary to when using forked processing as on Linux and
> > macOS). Something like this:
> >
> > - name: Install dependencies
> > run: |
> >remotes::install_deps(dependencies = TRUE)
> >remotes::install_cran("rcmdcheck")
> >install.packages(".", repos=NULL, type="source") ## needed by
> > parallel workers
> >shell: Rscript {0}
> >
> > That's what I had to do when testing 'future'
> > (https://github.com/HenrikBengtsson/future/blob/1835a122764bbc0196c78be6da6973c8063922b3/.github/workflows/R-CMD-check.yaml#L69).
> >
> > /Henrik
> >
> > On Wed, Jul 29, 2020 at 10:18 AM Giulia Pais wrote:
> >> Hi bioconductor team,
> >>
> >> we are currently developing a package for future submission on
> >> bioconductor which you can find here
> >> https://github.com/calabrialab/ISAnalytics.
> >>
> >> We use Github actions to keep track of R cmd checks at every commit, and
> >> this time, surprisingly for us, we had a failure on R cmd checks for
> >> windows (which is odd, since we develop on windows and performing check
> >> on 2 different windows machines didn't raise any error or warning) and
> >> we suspect this could be tied to the use of BiocParallel. For Windows,
> >> in fact, we use SnowParam instead of MulticoreParam and as the vignette
> >> and manual of BiocParallel specifies we must ensure that every worker is
> >> loaded with the proper dependencies, but apparently this is not
> >> necessary if the function to execute in bplapply belongs to the package
> >> in question. Here is the code of the function that might raise some
> >> problems:
> >>
> >> .import_type <- function(q_type, files, workers) {
> >> files <- files %>% dplyr::filter(.data$Quantification_type == q_type)
> >> # Register backend according to platform
> >> if (.Platform$OS.type == "windows") {
> >> p <- BiocParallel::SnowParam(workers = workers, stop.on.error =
> >> FALSE)
> >> } else {
> >> p <- BiocParallel::MulticoreParam(workers = workers, stop.on.error
> >> = FALSE)
> >> }
> >> # Import every file
> >> suppressMessages(suppressWarnings({
> >> matrices <- BiocParallel::bptry(
> >> BiocParallel::bplapply(files$Files_chosen, FUN = function(x) {
> >> matrix <- ISAnalytics::import_single_Vispa2Matrix(x)
> >> }, BPPARAM = p)
> >> )
> >> }))
> >> BiocParallel::bpstop(p)
> >> correct <- BiocParallel::bpok(matrices)
> >> imported_files <- files %>% dplyr::mutate(Imported = correct)
> >> matrices <- matrices[correct]
> >> # Bind rows in single tibble for all files
> >> if (purrr::is_empty(matrices)) {
> >> return(NULL)
> >> }
> >> matrices <- purrr::reduce(matrices, function(x, y) {
> >> x %>% dplyr::bind_rows(y) %>% dplyr::distinct()
> >> })
> >> list(matrices, imported_files)
> >> }
> >>
> >> The report of the Github action can be found here:
> >> https://github.com/calabrialab/ISAnalytics/runs/923261561
> >>
> >> The check apparently fails with these warnings: Warning - namespace
> >> 'ISAnalytics' is not available and has been replaced. We tried adding
> >> 'library(ISAnalytics)' and 'require(ISAnalytics)' but if we do
