Re: [Bioc-devel] support the stable version of R

2019-01-15 Thread Ludwig Geistlinger
> Having said that, I'll note that specifying R as a dependency, and a version 
> of R as a criterion for your package, is really a mis-nomer for a 
> Bioconductor package -- of course it uses R, and the version of R in use 
> determines the Bioconductor version(s) that can be used! So a rational change 
> is to remove R and its version requirement from the DESCRIPTION file 
> entirely, a strategy taken by I think about 400 of our 1650+ packages.

Maybe worth including under 

https://bioconductor.org/developers/package-guidelines/#description

-> 8. “Depends/Imports/Suggests/Enhances:” fields: --> Depends


From: Bioc-devel  on behalf of Martin Morgan 

Sent: Monday, January 14, 2019 6:52 PM
To: Lulu Chen; bioc-devel@r-project.org
Subject: Re: [Bioc-devel] support the stable version of R

Remember that Bioconductor packages are tested nightly on our build system, and 
this nightly testing is an important component of offering your users a stable 
environment, not just for your package but the other packages they use. 
Recreating this standard environment is facilitated by the standard 
Bioconductor package installation instruction

  BiocManager::install("YourPackage")

The nightly builds are done on a platform where all packages are from the same 
Bioconductor release, built on the same version of R. Your newly accepted 
package is in the '3.9' version of Bioconductor, which uses the 3.6 version of 
R. It follows that the best user experience is provided to those using R 3.6. 
By 'allowing' R 3.5 and non-standard installation (e.g., from github) you are 
ultimately compromising the quality standards of Bioconductor and the 
experience of the users of your package. The support site has many questions 
from people who install packages from different Bioconductor versions, so it is 
clearly not in your interest, or the Bioconductor project interest, or your 
user's interest, to enable this kind of usage.

It is hard to see into the future, so saying something like R >= 3.5 is really 
quite bold! Conversely, packages with out-of-date promises like R >= 2.1 are 
not tested on the systems they claim compatibility with, and can be easily 
broken on old versions of R where their dependencies are no longer available.

From some scraping of the file summarizing the current devel repository 
https://bioconductor.org/packages/devel/bioc/VIEWS I see

> as_tibble(tbl) %>% arrange(desc(n))
# A tibble: 84 x 2
   Var1  n

 1 R (>= 3.5)  130
 2 R (>= 2.10) 125
 3 R (>= 3.4)  117
 4 R (>= 3.5.0) 83
 5 R (>= 3.4.0) 64
 6 R (>= 3.3)   58
 7 R (>= 2.10.0)41
 8 R (>= 3.0.0) 41
 9 R (>= 3.2.0) 41
10 R (>= 3.3.0) 41
# ... with 74 more rows
> as_tibble(tbl) %>% filter(grepl("3.6", Var1)) %>% arrange(desc(n))
# A tibble: 2 x 2
  Var1 n
  
1 R (>= 3.6)  17
2 R (>= 3.6.0) 6
> as_tibble(tbl) %>% filter(grepl("<", Var1)) %>% arrange(desc(n))
# A tibble: 1 x 2
  Var1n
 
1 R (< 3.7.0) 1

so there are many very optimistic assertions about suitability, only a few 
current versions, and a single package that is not clairvoyant!

Having said that, I'll note that specifying R as a dependency, and a version of 
R as a criterion for your package, is really a mis-nomer for a Bioconductor 
package -- of course it uses R, and the version of R in use determines the 
Bioconductor version(s) that can be used! So a rational change is to remove R 
and its version requirement from the DESCRIPTION file entirely, a strategy 
taken by I think about 400 of our 1650+ packages.

Martin

On 1/14/19, 5:40 PM, "Bioc-devel on behalf of Lulu Chen" 
 wrote:

Dear all,

When submitting package to bioconductor, it is required to change R version
in "Depends" to be >= the develop version (3.6) . As my package is also
available in GitHub, someone asks if it be possible to make it available
with the stable version of R (R3.5). In fact, my package can work well with
R3.5 if I change "Depends" back to R(>=3.5) .

So I hope to support R3.5 for the moment before next release. Should I
create another repository? Can I use a branch to support R3.5?

Thanks,
Lulu

[[alternative HTML version deleted]]

___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel

___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel
___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel


Re: [Bioc-devel] What is good convention for package-local BiocParallel param?

2019-01-15 Thread Aaron Lun
> From the above example, if I had the BPPARAM argument, it’d also clearly add 
> a lot of code noise:
> 
> data %>%
> apply_methods(method_list1, BPPARAM = MulticoreParam(stop.on.error=FALSE)) %>%
> apply_methods(method_list2, BPPARAM = MulticoreParam(stop.on.error=FALSE)) %>%
> apply_methods(method_list3, BPPARAM = MulticoreParam(stop.on.error=FALSE))
> 
> The compromise is to have
> 
> my_bpparam = MulticoreParam(stop.on.error=FALSE)
> 
> data %>%
> apply_methods(method_list1, BPPARAM = my_bpparam) %>%
> apply_methods(method_list2, BPPARAM = my_bpparam) %>%
> apply_methods(method_list3, BPPARAM = my_bpparam)

This actually looks like the best option to me. Nice and explicit, amenable to 
static code analysis (manual or automated).

> But really at that point why not just have
> 
> set_cellbench_bpparam(MulticoreParam(stop.on.error=FALSE))
> 
> data %>%
> apply_methods(method_list1) %>%
> apply_methods(method_list2) %>%
> apply_methods(method_list3)
> 
> Which I guess is the point of this mail chain. Is there a reason why I 
> shouldn’t?

Can’t hurt to have fewer globals, I guess?

-A

>> =On 12 Jan 2019, at 5:43 pm, Aaron Lun 
>>  wrote:
>> 
>> My current set-up in a variety of packages is that every parallelizable 
>> function has a BPPARAM= argument. This makes it explicit about which steps 
>> are being parallelized. Requiring users to respecify BPPARAM= in each 
>> function isn’t as annoying as you’d think, because not many steps are 
>> actually heavy enough to warrant parallelization.
>> 
>> Ideally, I'd have BPPARAM=bpparam() by default, allowing me to both respond 
>> to the register()'d bpparam() as well as any custom argument that might be 
>> supplied by the user, e.g., if they don't want to change bpparam(). However, 
>> for various reasons (discussed in the other SerialParam() thread), the 
>> current default is BPPARAM=SerialParam().
>> 
>> To be honest, I've never thought it necessary to have a global 
>> package-specific parameter for parallelization as you've done (for scPipe, 
>> presumably). The current options - global across all packages with 
>> register(), or local to individual functions with BPPARAM= - seem to be 
>> satisfactory in the vast majority of cases. At least to me.
>> 
>> And at least for RNGs, if a function from another package is giving greatly 
>> different results upon parallelization (excepting some numerical error with 
>> changed order of summation), I'd say that's a bug of some sort. That should 
>> be fixed on their end, rather than requiring other packages and users to 
>> tiptoe around them.
>> 
>> -A
>> 
>>> On 10 Jan 2019, at 23:59, Shian Su  wrote:
>>> 
>>> Hello Developers,
>>> 
>>> I’m using BiocParallel for parallelism, and I understand that register() is 
>>> the recommended method for setting threads. But I am reluctant to ask 
>>> people to run code for my package which changes how other packages operate, 
>>> so I figured I’d use local bp params. Recent discussions of RNG has made me 
>>> worried there may be hidden state gotcha’s I’ve not considered. The current 
>>> implementation is
>>> 
>>> set_mypkg_threads <- function(n) {
>>> if (n == 1) {
>>> options(“mypkg.bpparam” = SerialParam())
>>> } else if (n > 1) {
>>> if (.Platform$OS.type == "windows") {
>>> options(“mypkg.bpparam" = SnowParam(nthreads))
>>> } else {
>>> options(“mypkg.bpparam" = MulticoreParam(nthreads))
>>> }
>>> }
>>> }
>>> 
>>> Then elsewhere in my package I make use of parallelism as follows
>>> 
>>> bplapply(
>>> BPPARAM = getOption(“mypkg.bpparam”, bpparam()),
>>> …
>>> )
>>> 
>>> Where getOption() either retrieves my set option or the default value given 
>>> by bpparam(). So the behaviour is that if users have not registered params 
>>> for my package specifically then it will take the BiocParallel default, but 
>>> otherwise it will use my package’s local bpparam.
>>> 
>>> Also I know that as currently implemented, I preclude cluster parallelism 
>>> on non-Windows machines. But it’s easy to fix. Just looking for feedback on 
>>> the approach.
>>> 
>>> Kind regards,
>>> Shian Su
>>> 
>>> ___
>>> 
>>> The information in this email is confidential and intended solely for the 
>>> addressee.
>>> You must not disclose, forward, print or use it without the permission of 
>>> the sender.
>>> 
>>> The Walter and Eliza Hall Institute acknowledges the Wurundjeri people of 
>>> the Kulin
>>> Nation as the traditional owners of the land where our campuses are located 
>>> and
>>> the continuing connection to country and community.
>>> ___
>>> 
>>> [[alternative HTML version deleted]]
>>> 
>>> ___
>>> Bioc-devel@r-project.org mailing list
>>> https://stat.ethz.ch/mailman/listinfo/bioc-devel
>> 
>> ___
>> Bioc-devel@r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/bioc-devel
> 
> 

Re: [Bioc-devel] support the stable version of R

2019-01-15 Thread Martin Morgan
Probably my rant yesterday was a bit confused.

BiocCheck tries to encourage better practice. If you've chosen to specify 
version (maybe there are reasons for this, e.g., because in the past you have 
distributed your package through non-Bioconductor channels), then specify it 
correctly.

On the other hand, the Bioconductor release model doesn't require a version 
specification and in practice maintainers do a poor job updating this 
information. If the maintainer hasn't specified version, then no need to 
suggest that they do so.

I guess it's analogous to many other tests where guidance is provided on 
mis-use, rather than on non-use -- 'we notice that you didn't test for class, 
but if you had a better approach is to use `is()` rather than `class() == `'. 
Or perhaps suggesting straight-forward solutions rather than nuanced solutions 
of uncertain merit  'instead of testing for class incorrectly with `class() == 
`, use method dispatch to avoid conditional code'.

Again, a best practice for github and other repositories outside Bioconductor 
is likely to maintain distinct release branches. If one is choosing to specify 
version, then each branch should closely specify relevant version.

Martin

On 1/15/19, 2:45 AM, "Alexey Sergushichev"  wrote:

Martin,


> Having said that, I'll note that specifying R as a dependency, and a 
version of R as a criterion for your package, is really a mis-nomer for a 
Bioconductor package -- of course it uses R, and the version of R in use 
determines the Bioconductor
 version(s) that can be used! So a rational change is to remove R and its 
version requirement from the DESCRIPTION file entirely, a strategy taken by I 
think about 400 of our 1650+ packages.


Oh, it's actually an option not to include an R dependency at all. I find 
it non-obvious, given the warning message that suggest to change the version 
dependency to R-devel, not mentioning that omitting dependency will fix the 
warning too. Should it be
 put somewhere in BiocCheck message?


Also, given that, I find it even more strange to have such R requirement in 
BiocCheck. You are either allowed very strict dependency: R >= R-devel or very 
lenient (not specifying any), but nothing in the middle. 


Best,
Alexey






On Tue, Jan 15, 2019 at 3:03 AM Martin Morgan  
wrote:


Maybe a little more helpfully, I think you should create a branch for 
'before' Bioconductor, where you can specify R version dependency as you see 
fit.

Indeed, at each release the version of your package at 
git.bioconductor.org  will have a branch 
created for that release, e.g., the RELEASE_3_8 branch, and you will want to 
sync that branch with your github repository as outlined in step 10 of

https://bioconductor.org/developers/how-to/git/sync-existing-repositories/ 
 . 

A conscientious developer would take the opportunity to increment the 
version dependency of R in the master branch to the version of R in use in the 
devel builds of Bioconductor.

Martin

On 1/14/19, 5:40 PM, "Bioc-devel on behalf of Lulu Chen" 
 wrote:

Dear all,

When submitting package to bioconductor, it is required to change R 
version
in "Depends" to be >= the develop version (3.6) . As my package is also
available in GitHub, someone asks if it be possible to make it available
with the stable version of R (R3.5). In fact, my package can work well 
with
R3.5 if I change "Depends" back to R(>=3.5) .

So I hope to support R3.5 for the moment before next release. Should I
create another repository? Can I use a branch to support R3.5?

Thanks,
Lulu

[[alternative HTML version deleted]]

___
Bioc-devel@r-project.org mailing list

https://stat.ethz.ch/mailman/listinfo/bioc-devel 


___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel




___
Bioc-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/bioc-devel