Bioconductor / Contributions

Contribute Packages to Bioconductor
135 stars 33 forks source link

metaseqR2 #1435

Closed pmoulos closed 4 years ago

pmoulos commented 4 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 help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.

bioc-issue-bot commented 4 years ago

Hi @pmoulos

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: metaseqR2
Type: Package
Title: An R package for the analysis and result reporting
    of RNA-Seq data by combining multiple statistical
    algorithms
Author: Panagiotis Moulos <moulos@fleming.gr>
Maintainer: Panagiotis Moulos <moulos@fleming.gr>
Depends:
    R (>= 4.0.0),
    DESeq,
    limma,
    qvalue
Imports:
    ABSSeq,
    baySeq,
    Biobase,
    BiocParallel,
    biomaRt,
    Biostrings,
    corrplot,
    DESeq2,
    DSS,
    DT,
    EDASeq,
    edgeR,
    GenomeInfoDb,
    GenomicAlignments,
    GenomicFeatures,
    GenomicRanges,
    gplots,
    graphics,
    grDevices,
    heatmaply,
    htmltools,
    httr,
    IRanges,
    jsonlite,
    log4r,
    NOISeq,
    magrittr,
    methods,
    NBPSeq,
    pander,
    parallel,
    rmarkdown,
    rmdformats,
    Rsamtools,
    RSQLite,
    rtracklayer,
    S4Vectors,
    splines,
    stats,
    stringr,
    SummarizedExperiment,
    survcomp,
    utils,
    VennDiagram,
    vsn,
    zoo
Suggests:
    BiocGenerics,
    BiocManager,
    BSgenome,
    knitr,
    RMySQL,
    RUnit
Enhances:
    TCC
Description: Provides an interface to several normalization and
    statistical testing packages for RNA-Seq gene expression data.
    Additionally, it creates several diagnostic plots, performs
    meta-analysis by combinining the results of several statistical
    tests and reports the results in an interactive way.
License: GPL (>= 3)
Encoding: UTF-8
LazyLoad: yes
LazyData: yes
URL: http://www.fleming.gr
biocViews: Software, GeneExpression, DifferentialExpression, WorkflowStep,
    Preprocessing, QualityControl, Normalization, ReportWriting, RNASeq,
    Transcription, Sequencing, Transcriptomics, Bayesian, Clustering,
    CellBiology, BiomedicalInformatics, FunctionalGenomics, SystemsBiology,
    ImmunoOncology, AlternativeSplicing, DifferentialSplicing, 
    MultipleComparison, TimeCourse, DataImport
VignetteBuilder: knitr
Authors@R: c(person(given="Panagiotis", family="Moulos",
    email="moulos@fleming.gr", role=c("aut", "cre")))
Version: 0.0.21
Date: 2020-03-25
Collate:
    'annotation.R'
    'argcheck.R'
    'count.R'
    'metaseqr2-data.R'
    'export.R'
    'filter.R'
    'json.R'
    'main.R'
    'meta.R'
    'norm.R'
    'metaseqR2-package.R'
    'plot.R'
    'query.R'
    'reportdb.R'
    'sim.R'
    'stat.R'
    'tracks.R'
    'util.R'
    'zzz.R'
bioc-issue-bot commented 4 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.

mtmorgan commented 4 years ago

@pmoulos can you explain the relationship between this package and metaseqR ? Usually it is counter-productive to have two versions of a package available. Why not just increment your existing package to version 1.99.0, 1.99.1, ... until the release, when it will become version 2.0.0 ?

bioc-issue-bot commented 4 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 4 years ago

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

4486b85 Fixed 1st submission check errors

pmoulos commented 4 years ago

@mtmorgan, there is a relationship between the two packages similarly with the case of DESeq and DESeq2. metaseqR2 has some similar logic to metaseqR but it has major and breaking additions, different calculation models and severe refactoring, including changes in argument naming, exported functions and many other aspects. Therefore, it would be easier for users to educate themselves on a new, simpler in terms of usage, but at the same time richer in functionalities package, than breaking many functionalities of the previous one.

In addition to the above, here is a summary of severe changes which to my opinion justify a new package:

Therefore, my logic is to maintain both packages:

mtmorgan commented 4 years ago

I suggest that you deprecate metaseqR in the devel branch immediately after the next release. See for instance the problems with DESeq in the thread here: https://support.bioconductor.org/p/129152

bioc-issue-bot commented 4 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.

pmoulos commented 4 years ago

@mtmorgan, yes this can be done. In the meanwhile, would it be a problem for the two packages to co-exist in a release? Would it pose a problem for the current submission? Thanks.

mtmorgan commented 4 years ago

we can continue with the current submission.

bioc-issue-bot commented 4 years ago

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

28425f3 Removed Author/Maintainer fields and left Authors@...

bioc-issue-bot commented 4 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.

bioc-issue-bot commented 4 years ago

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

cd787e2 Fixes one of the NOTEs in check

bioc-issue-bot commented 4 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.

bioc-issue-bot commented 4 years ago

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

9d7a965 Fixed typo causing DSS no replicates warning

bioc-issue-bot commented 4 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.

bioc-issue-bot commented 4 years ago

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

8d2eab1 Fixed issue with DE heatmaps in z-score mode and t...

bioc-issue-bot commented 4 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, 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.

bioc-issue-bot commented 4 years ago

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

da6d247 Removed R version dependency and fully imported sp...

bioc-issue-bot commented 4 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.

bioc-issue-bot commented 4 years ago

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

c02fabb Fixed a but in the report and improved DESCRIPTION...

bioc-issue-bot commented 4 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.

bioc-issue-bot commented 4 years ago

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

a20bdd9 Bump version for rebuild

bioc-issue-bot commented 4 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.

bioc-issue-bot commented 4 years ago

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

dbe29cf Try to reduce check timings

bioc-issue-bot commented 4 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.

pmoulos commented 4 years ago

Hi @Kayla-Morrell thanks for looking at the package. Is there anything else required from my side given that the upcoming release package inclusion deadline is approaching?

Kayla-Morrell commented 4 years ago

@pmoulos - Thanks for checking in, I'm working on getting the initial review posted by the end of today. Hopefully that will be enough time for you to work on the changes and we can get it in by the deadline.

Kayla-Morrell commented 4 years ago

Hello @pmoulos,

Thank you for submitting to Bioconductor. Please see the initial review of the package below. The required changes must be made while the suggested changes do not have to be but we strongly encourage them. Comment back here with updates that have been made and when the package is ready for a re-review. Keep in mind that the deadline for accepting new packages into the Bioconductor 3.11 release is 4/22.

DESCRIPTION

NAMESPACE

Well organized.

CITATION

Vignettes

metaseqr2-annotation

if(!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
BiocManager::install("metaseqR2")

metaseqr2-statistics

if(!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
BiocManager::install("metaseqR2")

Man pages

R code

Best, Kayla

pmoulos commented 4 years ago

Hello @Kayla-Morrell, thank you for the review. Below, points addressed, answers and some comments:

DESCRIPTION

  • [X] REQUIRED: 'LazyLoad' isn't a needed field and should be removed.
  • [X] REQUIRED: 'LazyData' should be set to false. It's out experience that when this is set to true it can slow down the loading of packages, especially those with data.
  • [X] SUGGESTION: Consider adding these automatically suggested biocViews:
    • ATACSeq, Microarray, Metabolomics, Proteomics, Epigenetics, Cheminformatics, Regression, ExonArray, ChIPSeq, OneChannel, TwoChannel, MicroRNAArray, mRNAMicroarray, ProprietaryPlatforms, GeneSetEnrichment, BatchEffect
  • [X] SUGGESTION: It is encouraged to include the 'BugReports:' field with a relevant link to GitHub for reporting Issues.

Comments on DESCRIPTION

CITATION

  • [X] REQUIRED: The 'mheader' for the CITATION file explains how to cite metaseqR. This should be changed to metaseqR2.

Comments on CITATION

Vignettes

  • [X] REQUIRED: Inline code should have single backticks , yours currently have three which indicates blocks of code to be run. Please change all inline code to single backticks.

metaseqr2-annotation

  • [X] REQUIRED: There should be an installation section that demonstrates to the user how to install the package from Bioconductor. The code should look like the following and should include eval=FALSE:
if(!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
BiocManager::install("metaseqR2")
  • [ ] SUGGESTION: We strongly encourage the use of table of contents.
  • [X] REQUIRED: The last section of the vignette should be 'Session Info' and should include sessionInfo().
  • [ ] REQUIRED: I'm not able to get this vignette building locally. I hope that once you change the inline coding the builders will pick up the issues I'm experiencing. But until then I'll work more on my side to see if I can debug some more. (It seems to error out from line 180-238).

metaseqr2-statistics

  • [X] REQUIRED: There should be an installation section that demonstrates to the user how to install the package from Bioconductor. The code should look like the following and should include eval = FALSE:
if(!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
BiocManager::install("metaseqR2")
  • [ ] SUGGESTION: We strongly encourage the use of table of contents.
  • [X] REQUIRED: Line 350, data should not be downloaded from Google Drive. The data should be part of the package, be coming from one of our Hubs, or from reliable sources.
  • [X] REQUIRED: There should not be eval=FALSE sections in the vignette, this defeats the purpose of having executable code.
  • [X] REQUIRED: The last section of the vignette should be 'Session Info' and should include sessionInfo().

Comments on Vignettes

Man pages

  • [X] REQUIRED: There needs to be runnable examples in the following man pages which document exported object. If the example is not feasible the function needs to be tested somehow, where that be in a code chunk in a vignette or by using tests.
    • createSignalTracks.Rd
    • metaTest.Rd
    • read2count.Rd
    • readTargets.Rd
    • statBayseq.Rd

Comments on Man pages

R code

  • [X] REQUIRED: While trying to run the vignette code locally I ran into a few warnings that weren't shown in the vignettes themselves because you set warnings=FALSE. Please investigate the use of these functions GenomeInfoDb::fetchExtendedChromInfoFromUCSC is deprecated and Specifying width/height in layout() in now deprecated.
  • [X] REQUIRED: Unexported object imported by ':::' calls. You are not allowed to access non-exported functions from other packages. Either ask the maintainer to export the function or write your own version of it using exported functions.
    • 'GenomicFeatures:::GFF_FEATURE_TYPES'
    • 'biomaRt:::generateFilterXML'
    • 'biomaRt:::.setResultColNames'
    • 'biomaRt:::martCheck'
    • 'biomaRt:::martDataset'
    • 'biomaRt:::martHost'
    • 'biomaRt:::martVSchema'
  • [X] REQUIRED: There are ':::' calls to the package's namespace in its code. A package almost never needs to use ':::' for its own objects:
    • '.chromInfoFromBAM'
    • '.chromInfroToSeqInfoDf'
    • 'cmclapply'
    • 'getUcscOrganism'
    • 'getValidChrs'
  • [ ] REQUIRED: install, biocLite, install.packages, or update.packages found in R files. Generally you shouldn't be making a call to these functions in your code.
    • annotation.R, line 1907
  • [X] REQUIRED: Avoid sapply(); use vapply() if possible. Found in files:
    • annotation.R, lines 1240, 1298, and 1357
    • json.R, line 2189
    • main.R, lines 946, 950, 1075, and 1079
    • meta.R, lines 350 and 358
    • plot.R, lines 2057 and 2766
    • sim.R, lines 361, 388, 389, 390, 416, 416, and 418
    • stat.R, lines 14, 307, and 653
    • tracks.R, lines 99 and 222
    • util.R, lines 2968 and 2969
  • [X] REQUIRED: Avoid 1:...; use seq_len() or seq_along() instead, if possible. Found in files:
    • annotation.R, lines 154, 215, 268, 341, 716, 738, 1162, 1222, 1279, 1339, 1538, 1587, 1595, 1612, 1616, 1656, 1922, 2773, 2805, 2837, 2852, 2902, 2953, 2968, 2998, 3030, 3045, 3102, 3154, 3172, and 3203
    • argcheck.R, line 6
    • count.R, lines 218, 515, 551, and 583
    • export.R, line 25
    • json.R, lines 149, 232, 407, 441, 629, 835, 867, 868, 989, 1097, 1117, 1123, 1245, 1489, 1502, 1526, 2036, 2152, 2178, 2629, 2888, 3242, 3245, and 3257
    • main.R, lines 788, 879, 1011, 1124, 1753, 1756, and 2095
    • meta.R, lines 94, 126, 160, 271, 312, 341, 348, and 356
    • norm.R, lines 12, 139, 166, 200, and 277
    • plot.R, lines 9, 47, 57, 192, 266, 285, 365, 366, 461, 483, 484, 522 593, 594, 647, 649, 673, 675, 855, 904, 1257, 1275, 1317, 1318, 1457, 1582, 1805, 1810, 1860, 1865, 1875, 2093, 2158, 2206, 2231, 2639, 2658, 2677, 2791, 2806, 2814, 2815, 2828, 2829, 2837, 2838, 2851, 2852, 2952, 3116, 3118, 3126, 3143, 3159, and 3160
    • reportdb.R, lines 422, 521, 538, 548, 563, 573, 601, 610, 720, 721, 731, 732, 810, 839, 864, 873, 912, 1001, and 1017
    • sim.R, lines 12, 19, 28, 29, 56, 57, 102, 103, 125, 162, 164, 168, 170, 183, 188, 194, 201, 207, 210, 212, 238, 249, 250, 295, 323, 329, 346, 349, 354, 357, 359, 362, 378, and 406
    • stat.R, lines 5, 113, 265, 437, 515, 524, 556, 566, 579, 592, 603, 613, 623, 642, 699, 781, 968, and 1284
    • tracks.R, lines 245, 262, and 274
    • util.R, lines 1588, 1613, 1621, 1638, 1647, 1670, 1738, 1832, 2805, 2810, 2819, 2829, 2839, and 2969
  • [X] REQUIRED: We do not allow for set.seed() to be used in code. This includes using setting it as a global variable. Please remove this.
  • [X] SUGGESTION: For formating reasons, consider shorter lines. There are 23 lines that are > 80 characters long.
  • [ ] SUGGESTION: For formating reasons, consider multiples of 4 spaces for line indents. There are 417 lines that are not.

Comments on R code

bioc-issue-bot commented 4 years ago

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

0462f78 Version bump?

bioc-issue-bot commented 4 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.

bioc-issue-bot commented 4 years ago

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

91de3d6 Try to reduce check times

bioc-issue-bot commented 4 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.

pmoulos commented 4 years ago

Hi @Kayla-Morrell, any hope to look at this :-)

Kayla-Morrell commented 4 years ago

@pmoulos - Thank you for making the necessary changes. I have just a couple follow up comments and then I should be ready to accept the package. Let me know when you are ready for me to look over these points.

Comments on DESCRIPTION

  • Not all biocViews were added as the package clearly does not address them and may be misleading, specifically, Microarray, Metabolomics, Proteomics, Cheminformatics, ExonArray, OneChannel, TwoChannel, MicroRNAArray, mRNAMicroarray

That's fine that you've left some out from the list. These are automatically generated from BiocCheck based on the other biocViews that are listed.

Comments on CITATION

  • An article describing metaseqR2 will be submitted this week. Should I change the reference now or wait until it gets published and then update the citation in dev?

As shown in our package guidelines, the citation is more so for how the package should be cited. I'll let you be the judge of if you want to leave it as is or not.

Comments on Vignettes

  • Regarding the TOC, for each vignette, a TOC is generated by the BiocStyle. Do you mean a TOC linking to all vignettes? How can I do that?
  • I removed the example requiring to download data from Google Drive and replaced with additional external resources (GitHub wiki) for additional help on using the package.
  • Regarding the vignette that you cannot build locally, the code in that particular chunk is platform-dependent. It will not run on Windows machines (but the code checks for that). Other than that, I can build it locally and it seems that the Bioconductor build system does not face an issue.

By adding the 'toc' in the top of the vignette code then TOC's will be put at the top of the individual vignettes (see the code below).

BiocStyle::html_document:
toc: true
toc_float: true

Comments on Man pages

  • A small subset of 2 real BAM files was added in inst/extdata. These files were the basis for creating runnable examples for createSignalTracks.Rd, read2count.Rd, readTargets.Rd.
  • Runnable examples were added in metaTest.Rd and statBayseq.Rd

Thank you for adding the BAM files, there needs to be some documentation about these files. There needs to be documentation for the 'inst/extdata/' files in an 'inst/script/'. Please see our package guidelines here for what to include for the documentation.

Best, Kayla

pmoulos commented 4 years ago

Hi @Kayla-Morrell, I added a README.txt file in inst/script with a description on the source of these files and addressed the TOC issue. Hope this is OK. Thank you for all the help.

bioc-issue-bot commented 4 years ago

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

90f9967 Address 2nd review

Kayla-Morrell commented 4 years ago

@pmoulos - Those look fine. Just one last last change that I can see. In the vignettes, line 11 (should be the same in the both files) should be:

%\VignetteEngine{knitr::rmarkdown}

this will help render the vignettes properly.

bioc-issue-bot commented 4 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: "skipped, 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 4 years ago

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

3259ee1 Version bump

pmoulos commented 4 years ago

Hi @Kayla-Morrell, I changed the line in the vignettes. There was an error in the last built in Linux, however I suspect that this is related to Ensembl and its reduced functionality over the past few weeks... Let's see how this built goes. Thanks.

bioc-issue-bot commented 4 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.

bioc-issue-bot commented 4 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!

mtmorgan commented 4 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/pmoulos.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("metaseqR2"). The package 'landing page' will be created at

https://bioconductor.org/packages/metaseqR2

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.