Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

RESOLVE #2841

Closed danro9685 closed 2 years ago

danro9685 commented 2 years ago

Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor

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

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

For questions/help about the submission process, including questions about the output of the automatic reports generated by the SPB (Single Package Builder), please use the #package-submission channel of our Community Slack. Follow the link on the home page of the Bioconductor website to sign up.

bioc-issue-bot commented 2 years ago

Hi @danro9685

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: RESOLVE
Version: 0.99.4
Date: 2022-10-16
Title: RESOLVE: An R package for the efficient analysis of mutational signatures from cancer genomes
Authors@R: c(
    person("Daniele", "Ramazzotti", role=c("aut"),email="daniele.ramazzotti@unimib.it",
      comment = c(ORCID = "0000-0002-6087-2666")),
    person("Luca", "De Sano", role=c("cre","aut"), email="luca.desano@gmail.com",
      comment = c(ORCID = "0000-0002-9618-3774")))
Depends:
    NMF
Imports:
    Biostrings,
    BSgenome,
    BSgenome.Hsapiens.1000genomes.hs37d5,
    cluster,
    data.table,
    GenomeInfoDb,
    GenomicRanges,
    glmnet,
    ggplot2,
    gridExtra,
    IRanges,
    lsa,
    nnls,
    parallel,
    reshape2
Suggests:
    BiocGenerics,
    BiocStyle,
    testthat,
    knitr
Description:
    Cancer is a genetic disease caused by somatic mutations in genes controlling key biological functions such as cellular growth and division. Such mutations may arise both through cell-intrinsic and exogenous processes, generating characteristic mutational patterns over the genome named mutational signatures. The study of mutational signatures have become a standard component of modern genomics studies, since it can reveal which (environmental and endogenous) mutagenic processes are active in a tumor, and may highlight markers for therapeutic response. Mutational signatures computational analysis presents many pitfalls. First, the task of determining the number of signatures is very complex and depends on heuristics. Second, several signatures have no clear etiology, casting doubt on them being computational artifacts rather than due to mutagenic processes. Last, approaches for signatures assignment are greatly influenced by the set of signatures used for the analysis. To overcome these limitations, we developed RESOLVE (Robust EStimation Of mutationaL signatures Via rEgularization), a framework that allows the efficient extraction and assignment of mutational signatures. RESOLVE implements a novel algorithm that enables (i) the efficient extraction, (ii) exposure estimation, and (iii) confidence assessment during the computational inference of mutational signatures.
Encoding: UTF-8
License: file LICENSE
URL: https://github.com/danro9685/RESOLVE
BugReports: https://github.com/danro9685/RESOLVE/issues
biocViews: BiomedicalInformatics, 
 SomaticMutation
RoxygenNote: 7.2.1
VignetteBuilder: knitr
nilseling commented 2 years ago

Hi @lshep, this issue addresses the package name change from #2798. Could you please add @luca-dex as maintainer? Thanks for your help with this!

bioc-issue-bot commented 2 years ago

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

IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. It is required to push a version bump to git.bioconductor.org to trigger a new build.

Bioconductor utilized your github ssh-keys for git.bioconductor.org access. To manage keys and future access you may want to active your Bioconductor Git Credentials Account

bioc-issue-bot commented 2 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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/RESOLVE to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

danro9685 commented 2 years ago

Dear @nilseling,

We should have addressed all the issues you have listed in your previous post. Moreover, @luca-dex also added package name to Watched Tags, so the ERROR should also have been dealt with.

Please let us know if you have additional comments.

Thanks, Daniele & @luca-dex

nilseling commented 2 years ago

Hi @danro9685 and @luca-dex thanks for addressing most of the comments. There are still some that have not been addressed and I'd like to ask you to comment on the points below once you address them or give an explanation if you are not planing on adapting them.

RESOLVE Coverage: 41.91%
R/visualization.R: 0.00%
R/mutational.signatures.discovery.R: 47.64%
R/mutational.signatures.significance.R: 70.99%
R/data.import.R: 97.14%
* NOTE: Avoid 1:...; use seq_len() or seq_along()
* NOTE: Avoid 'cat' and 'print' outside of 'show' methods
* NOTE: Avoid using '=' for assignment and use '<-' instead
* WARNING: Avoid the use of 'library' or 'require' in R code
danro9685 commented 2 years ago

Dear @nilseling,

1) In the latest update on GitHub we should have addressed all your points regarding the vignette. 2) Sounds good about the unit tests. 3) Regarding BiocParallel: we are planning to move to BiocParallel for parallel computation, however the current implementation is quite complex and we don't think we enough bandwidth (among this and the work for the other packages we have submitted/support) to perform these changes (without including bugs) by the upcoming release. Would it be acceptable if we plan to upgrade the package in the future, after acceptance? 4) R code: the WARNING: Avoid the use of 'library' or 'require' in R code is related to Q3, as the function library is used for the parallel computation. We will remove this when moving to BiocParallel. Furthermore: Avoid 1:...; use seq_len() or seq_along() should be fixed now.

Please let us know if we missed anything or if you have any additional feedback while we address the remaining things.

Thanks, Daniele and @luca-dex

nilseling commented 2 years ago

Hi @danro9685 and @luca-dex

  1. In the latest update on GitHub we should have addressed all your points regarding the vignette.

Thanks for adding the title and more general info on how to detect mutational signatures. I think what would still help the user is to understand why RESOLVE is part of Bioconductor instead of e.g. CRAN. You have already developed the SpareSignatures Bioconductor package and it would be straight forward for users that are familiar with this package to now also use RESOLVE. You could add a sentence or two in the vignette stating that RESOLVE is an addition to other Bioconductor packages such as SparseSignatures, MutationalPatterns, musicatk, among others that now implements a new approach for detecting mutational signatures.

  1. Sounds good about the unit tests.

Done

  1. Regarding BiocParallel: we are planning to move to BiocParallel for parallel computation, however the current implementation is quite complex and we don't think we enough bandwidth (among this and the work for the other packages we have submitted/support) to perform these changes (without including bugs) by the upcoming release. Would it be acceptable if we plan to upgrade the package in the future, after acceptance?

I think it's fine to adapt the package after release.

  1. R code: the WARNING: Avoid the use of 'library' or 'require' in R code is related to Q3, as the function library is used for the parallel computation. We will remove this when moving to BiocParallel. Furthermore: Avoid 1:...; use seq_len() or seq_along() should be fixed now.

Thanks for addressing these warnings and notes and the warning regarding the library call will be ignored for now. However, there are new NOTEs in your R code :

NOTE: Avoid the use of 'paste' in condition signals

This one relates to lines like:

message(paste0("Performing repetition ", cv_repetitions, " out of ",
                    cross_validation_repetitions, "..."), "\n")

which you can simply fix by:

message("Performing repetition ", cv_repetitions, " out of ",
                    cross_validation_repetitions, "...", "\n")

The following NOTE

* NOTE: Avoid redundant 'stop' and 'warn*' in signal conditions

relates to

results <- tryCatch({}, error = function(e) {
                    message(paste0("An error has occurred: ",
                      e$message), "\n")})

which you can simply resolve by:

results <- tryCatch({}, error = function(e) {
                    paste0("An error has occurred: ", e$message, "\n")})

Please let us know if we missed anything or if you have any additional feedback while we address the remaining things.

Could you please push these changes to Bioconductor to trigger another built?

danro9685 commented 2 years ago

Dear @nilseling,

Thanks for the feedback, we have just updated our GitHub repository with all the necessary changes. @luca-dex please perform the push to Bioconductor.

Thanks, Daniele

nilseling commented 2 years ago

Hi @luca-dex,

could you please push the changes to Bioconductor so that the checks pass before Wednesday? Let me know if there are issues with the access rights.

bioc-issue-bot commented 2 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: f16ec16556f9f8d4a25e9e9c17f211cc9e7d4170

luca-dex commented 2 years ago

Hi @nilseling ,

last week the system was not accepting the push, but I just tried again and it worked.

Thanks

bioc-issue-bot commented 2 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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/RESOLVE to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

nilseling commented 2 years ago

Thanks for pushing the changes! I will now accept the package. For the next release I would recommend to switch to BiocParallel to address the last WARNING.

bioc-issue-bot commented 2 years ago

Your package has been accepted. It will be added to the Bioconductor nightly builds.

Thank you for contributing to Bioconductor!

Reviewers for Bioconductor packages are volunteers from the Bioconductor community. If you are interested in becoming a Bioconductor package reviewer, please see Reviewers Expectations.

lshep commented 2 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/danro9685.keys is not empty), then no further steps are required. Otherwise, do the following:

  1. Add an SSH key to your github account
  2. Submit your SSH key to Bioconductor

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 BiocManager::install("RESOLVE"). The package 'landing page' will be created at

https://bioconductor.org/packages/RESOLVE

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.