hierinf - Githubissues

crbasel commented 6 years ago

Repository: https://github.com/crbasel/hierinf

Confirm the following by editing each check box to '[x]'

[x] I understand that by submitting my package to Bioconductor, the package source and all review commentary are visible to the general public.
[x] I have read the Bioconductor Package Submission instructions. My package is consistent with the Bioconductor Package Guidelines.
[x] I understand that a minimum requirement for package acceptance is to pass R CMD check and R CMD BiocCheck with no ERROR or WARNINGS. Passing these checks does not result in automatic acceptance. The package will then undergo a formal review and recommendations for acceptance regarding other Bioconductor standards will be addressed.
[x] My package addresses statistical or bioinformatic issues related to the analysis and comprehension of high throughput genomic data.
[x] I am committed to the long-term maintenance of my package. This includes monitoring the support site for issues that users may have, subscribing to the bioc-devel mailing list to stay aware of developments in the Bioconductor community, responding promptly to requests for updates from the Core team in response to changes in R or underlying software.

I am familiar with the essential aspects of Bioconductor software management, including:

[x] The 'devel' branch for new packages and features.
[x] The stable 'release' branch, made available every six months, for bug fixes.
[x] Bioconductor version control using Git (optionally via GitHub).

For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.

bioc-issue-bot commented 6 years ago

Hi @crbasel

Thanks for submitting your package. We are taking a quick look at it and you will hear back from us soon.

The DESCRIPTION file for this package is:

Package: hierinf
Type: Package
Title: Hierarchical Inference
Version: 0.5.0
Authors@R: c(person("Claude", "Renaux", role = c("aut", "cre"), 
          email = "renaux@stat.math.ethz.ch"), 
   person("Laura", "Buzdugan", role = c("aut")), 
   person("Markus", "Kalisch", role = c("aut")), 
   person("Peter", "Bühlmann", role = c("aut")))
Description: Tools to perform hierarchical inference for one or multiple 
    studies / data sets based on high-dimensional multivariate (generalised) 
    linear models. A possible application is to perform hierarchical inference 
    for GWA studies to find significant groups or single SNPs (if the signal 
    is strong) in a data-driven and automated procedure. The method is based 
    on an efficient hierarchical multiple testing correction and controls the 
    FWER. The functions can easily be run in parallel.
License: GPL-3 | file LICENSE
Encoding: UTF-8
LazyData: yes
Depends: 
    R (>= 3.5.0)
Imports: 
    fmsb,
    glmnet, 
    parallel
Suggests: 
    knitr,
    MASS,
    testthat
biocViews:
    Clustering, 
    GenomeWideAssociation,
    LinkageDisequilibrium, 
    Regression,
    SNP
VignetteBuilder: 
    knitr
RoxygenNote: 6.0.1

bioc-issue-bot commented 6 years ago

A reviewer has been assigned to your package Learn what to expect during the review process.

IMPORTANT: Please read the instructions for setting up a push hook on your repository, or further changes to your repository will NOT trigger a new build.

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

1033e35 Submission to bioconductor

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

crbasel commented 6 years ago

Hi @hpages,

First of all, thanks a lot for reviewing our R package.

Can you please give me a hint why we receive a warning under windows? I’m puzzled because running R CMD check using win-builder.r-project.org does not return any warning.

Thanks a lot and best wishes, Claude

lshep commented 6 years ago

@crbasel In the summary section it indicates that the check time is above 5 min. Since the other two platform pass, It will be okay to ignore for now. @hpages will have a technical review of your package soon.

crbasel commented 6 years ago

Hi @lshep,

Thanks for your reply. I'll add testthat::skip_on_bioc() or testthat::skip_on_os("windows") in the next version such that the check time will be shorter.

Best wishes, Claude

dvantwisk commented 6 years ago

Hello, I've reviewed your package due to Herve being on vacation. The package looks very well written and I only have a few things to add. The [REQUIRED] tags indicate that an issue must either be fixed or responded to. The [CONSIDER] tags indicate issues where changes are advisable but not necessary. Please message me back when you have finished making you changes.

GENERAL

[REQUIRED] remove bioc-comments.md, it doesn't seem to be useful once the package is accepted.

R/GENERAL

[CONSIDER] Use BiocParallel instead of parallel. BiocParallel encapsulates a lot more functionality that just what the parallel package has, and it is also easier to use. For example, BiocParallel figures out the number of cores etc automatically behind the scenes.

R/comp-cluster-pval.R

[REQUIRED] L131-135: Improve efficiency by declaring gamma.step with vapply:

gamma.step <- vapply(gamma.seq, function(g) {
min(1, stats::quantile(pvals / g, g, na.rm = TRUE))
}, numeric(1))

R/test-only-hierarchy.R

[REQUIRED] L185-189: Improve efficiency by declaring colnames.multisplit using vapply as shown previously.

dvantwisk commented 6 years ago

Hello, It's been a few week since we've last heard from you. Have you made any progress on this package? I do need to say that If you don't have any activity within the next two weeks, we'll have to close the issue for the time being.

crbasel commented 6 years ago

Hi @dvantwisk,

Thanks a lot for your very helpful review. I'll make the changes in the coming days and push the newest version. Sorry for my delay in replying. I was out-of-office.

Best wishes, Claude

bioc-issue-bot commented 6 years ago

Received a valid push; starting a build. Commits are:

7475a07 Small changes after review.

bioc-issue-bot commented 6 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

crbasel commented 6 years ago

Hi @dvantwisk,

Thanks again for your helpful review. I've made the small changes to the R code.

Best wishes, Claude

dvantwisk commented 6 years ago

The changes look good! I don't have anything else to add so I'm going to move to accept the package.

bioc-issue-bot commented 6 years ago

Your package has been accepted. It will be added to the Bioconductor Git repository and nightly builds. Additional information will be posed to this issue in the next several days.

Thank you for contributing to Bioconductor!

crbasel commented 6 years ago

Thanks a lot. That's great.

mtmorgan commented 6 years ago

The master branch of your GitHub repository has been added to Bioconductor's git repository.

To use the git.bioconductor.org repository, we need an 'ssh' key to associate with your github user name. If your GitHub account already has ssh public keys (https://github.com/crbasel.keys is not empty), then no further steps are required. Otherwise, do the following:

See further instructions at

https://bioconductor.org/developers/how-to/git/

for working with this repository. See especially

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

to keep your GitHub and Bioconductor repositories in sync.

Your package will be included in the next nigthly 'devel' build (check-out from git at about 6 pm Eastern; build completion around 2pm Eastern the next day) at

https://bioconductor.org/checkResults/

(Builds sometimes fail, so ensure that the date stamps on the main landing page are consistent with the addition of your package). Once the package builds successfully, you package will be available for download in the 'Devel' version of Bioconductor using biocLite(\"hierinf\"). The package 'landing page' will be created at

https://bioconductor.org/packages/hierinf

If you have any questions, please contact the bioc-devel mailing list (https://stat.ethz.ch/mailman/listinfo/bioc-devel); this issue will not be monitored further.

Bioconductor / Contributions

hierinf #773