Bioconductor / Contributions

Contribute Packages to Bioconductor
131 stars 33 forks source link

anota2seq #472

Closed ChrOertlin closed 6 years ago

ChrOertlin commented 6 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 6 years ago

Hi @ChrOertlin

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: anota2seq
Version: 0.99.0
Title: Generally applicable transcriptome-wide analysis of
        translational efficiency using anota2seq
Author: Christian Oertlin <christian.oertlin@ki.se>, Julie Lorent <julie.lorent@ki.se>, Ola Larsson <ola.larsson@ki.se>
Maintainer: Ola Larsson <ola.larsson@ki.se>
Description: anota2seq provides analysis of translational efficiency and differential expression analysis for  polysome-profiling and ribosome-profiling studies (two or more sample classes) quantified by RNA sequencing or DNA-microarray. Polysome-profiling and ribosome-profiling typically generate data for two RNA sources; translated mRNA and total mRNA. Analysis of differential expression is used to estimate changes within each RNA source (i.e. translated mRNA or total mRNA). Analysis of translational efficiency aims to identify changes in translation efficiency leading to altered protein levels that are independent of total mRNA levels (i.e. changes in translated mRNA that are independent of levels of total mRNA) or buffering, a mechanism regulating translational efficiency so that protein levels remain constant despite fluctuating total mRNA levels (i.e. changes in total mRNA that are independent of levels of translated mRNA). anota2seq applies analysis of partial variance and the random variance model to fulfill these tasks.
Depends: R (>= 3.4.0), methods
License: GPL-3
LazyData: yes
LazyLoad: yes
Imports: multtest,qvalue,limma,DESeq2,edgeR,RColorBrewer, grDevices,
        graphics, stats, utils, SummarizedExperiment
biocViews: GeneExpression, DifferentialExpression,
        Microarray,GenomeWideAssociation, BatchEffect, Normalization,
        RNASeq, Sequencing, GeneRegulation, Regression
NeedsCompilation: no
Packaged: 2017-09-04 16:42:15 UTC; christian.oertlin
Suggests: BiocStyle,knitr
VignetteBuilder: knitr
bioc-issue-bot commented 6 years ago

Your package has been approved for building. Your package is now submitted to our queue.

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: "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 following build report for more details:

http://bioconductor.org/spb_reports/anota2seq_buildreport_20170905081714.html

hpages commented 6 years ago

Hi Christian,

Thanks for submitting the anota2seq package!

Please use BiocStyle::latex() instead of BiocStyle::latex2() in your vignette. The latter is deprecated. Using the former will also make the current R CMD build error (happening at the end of vignette creation) go away. After you make that change please don't forget to bump the version of the package in order to trigger a new build/check.

Thanks, H.

bioc-issue-bot commented 6 years ago

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

bc87589 removed build and inst/doc 3e261c3 fixed an issue where the package would not build w... ca696c3 remove build from cached files 2a8aa3e reduced R CMD check time, removed LICENSE file - l... 0f03b36 removed README.md file bbf7a5e formatting anota2seq show method 79f4503 fixed message formatting and an issue with sortBy ... 2ed5158 update NEWS file according to changes 7360d13 made static xlim values of pvalue plots 936ccb5 text edits in object description in show method + ...

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 following build report for more details:

http://bioconductor.org/spb_reports/anota2seq_buildreport_20171003043910.html

hpages commented 6 years ago

Hi Christian,

anota2seq is in good shape. It's clear that a lot of work has been put in the package and in particular in its documentation. The vignette is excellent. I only have a few minor issues to report, all of them cosmetic. See below.

Regards, H.

  1. All the arguments in your function have default values, even mandatory arguments for which the default value is set to NULL. For example:

    > args(anota2seq.get.covariates)
    function (object = NULL) 
    
    > args(anota2seqPlotGenes)
    function (object = NULL, selContrast = NULL, analysis = NULL, 
        geneNames = NULL, plotToFile = TRUE, fileName = NULL) 
    
    > args(anota2seqDataSetFromSE)
    function (se = NULL, assayNum = 1, dataType = NULL, normalize = FALSE, 
      transformation = "TMM-log2", filterZeroGenes = FALSE, varCutOff = NULL)

    This is not a good design. The 1st argument for all these functions is of course mandatory and it wouldn't make sense to set it to NULL. Best practice is to use default argument values only for optional arguments. Mandatory arguments don't need and shouldn't have a default value.

  2. Please adhere to Bioconductor naming style for your classes and functions:

    • Names of S4 classes should start with a capital letter e.g. Anota2seqDataSet instead of anota2seqDataSet.
    • Don't use dot-separated names like anota2seq.get.output() for functions. Bioconductor recommended naming style for functions is camelCase like you did for other functions (e.g. anota2seqPlotGenes()). But more importantly than the particular style you choose, pick a style and stick to it. Don't mix styles.
    • It's a good idea to use a different naming style for your datasets e.g. underscore-separated names (anota2seq_data_T, anota2seq_data_P, and anota2seq_pheno_vec). This avoids potential confusion.
  3. According to their man page, the anota2seqDataT and anota2seqDataP matrices have 10736 rows. However, it seems that they have in fact 10748 rows.

ChrOertlin commented 6 years ago

Hello,

Thank you for the comments, we will update them as soon as possible!

Cheers, Christian

bioc-issue-bot commented 6 years ago

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

b7b47d4 Adhere to bioconductor naming style + fix data doc 21516e7 bug fix + modify default values of function argume...

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: "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 following build report for more details:

http://bioconductor.org/spb_reports/anota2seq_buildreport_20171012060808.html

bioc-issue-bot commented 6 years ago

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

b7b47d4 Adhere to bioconductor naming style + fix data doc 21516e7 bug fix + modify default values of function argume...

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 following build report for more details:

http://bioconductor.org/spb_reports/anota2seq_buildreport_20171012142512.html

ChrOertlin commented 6 years ago

Hi,

Thank you very much for these constructive comments. We have now submitted a new version of anota2seq (version 0.99.2) which address these remarks (see below).

Regards,

Christian Oertlin, Julie Lorent and Ola Larsson.

Reply to comments:

  1. All the arguments in your function have default values, even mandatory arguments for which the default value is set to NULL. For example:
> args(anota2seq.get.covariates)
    function (object = NULL) 
> args(anota2seqPlotGenes)
    function (object = NULL, selContrast = NULL, analysis = NULL, 
        geneNames = NULL, plotToFile = TRUE, fileName = NULL) 
> args(anota2seqDataSetFromSE)
    function (se = NULL, assayNum = 1, dataType = NULL, normalize = FALSE, 
      transformation = "TMM-log2", filterZeroGenes = FALSE, varCutOff = NULL)

This is not a good design. The 1st argument for all these functions is of course mandatory and it wouldn't make sense to set it to NULL. Best practice is to use default argument values only for optional arguments. Mandatory arguments don't need and shouldn't have a default value.

_Reply: We have modified all functions to remove default values of mandatory arguments. Moreover, we modified the default values of two optional arguments: filterZeroGenes in anota2seqDataSetFromMatrix and anota2seqDataSetFromSE as well as fileName in the plot functions (anota2seqPlotFC, anota2seqPlotPvalues and anota2seqPlotGenes). filterZeroGenes is now TRUE by default if the user provides raw RNAseq counts and FALSE by default otherwise. The fileName parameter was renamed fileStem because the contrast number will be added at the end of the provided stem. Its default values are now "ANOTA2SEQ_FoldchangePlot", "ANOTA2SEQ_pvalue_density" and "ANOTA2SEQ_significantGenes_plot" respectively for functions anota2seqPlotFC, anota2seqPlotPvalues and anota2seqPlotGenes._

  1. Please adhere to Bioconductor naming style for your classes and functions: Names of S4 classes should start with a capital letter e.g. Anota2seqDataSet instead of anota2seqDataSet. Don't use dot-separated names like anota2seq.get.output() for functions. Bioconductor recommended naming style for functions is camelCase like you did for other functions (e.g. anota2seqPlotGenes()). But more importantly than the particular style you choose, pick a style and stick to it. Don't mix styles. It's a good idea to use a different naming style for your datasets e.g. underscore-separated names (anota2seq_data_T, anota2seq_data_P, and anota2seq_pheno_vec). This avoids potential confusion.

Reply: we have renamed all classes, functions (using camelCase naming style) and datasets as suggested.

  1. According to their man page, the anota2seqDataT and anota2seqDataP matrices have 10736 rows. However, it seems that they have in fact 10748 rows.

Reply: Thank you for notifying this mistake, we corrected it.

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 sent to the maintainer email address in the next several days.

Thank you for contributing to Bioconductor!

hpages commented 6 years ago

Thanks for making these changes. I'm labeling the package as accepted.

Cheers, H.

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/ChrOertlin.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 biocLite(\"anota2seq\"). The package 'landing page' will be created at

https://bioconductor.org/packages/anota2seq

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.