Closed gamerino closed 6 years ago
Hi @gamerino
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: NBSplice
Type: Package
Version: 0.99.0
Date: 2018-06-15
Title: Negative Binomial Models to detect Differential Splicing
Author: Gabriela A. Merino and Elmer A. Fernandez
Maintainer: Gabriela Merino <merino.gabriela33@gmail.com>
Description: The package proposes a differential splicing evaluation method
based on isoform quantification. It applies generalized linear models with
negative binomial distribution to infer changes in isoform relative
expression.
URL: http://www.bdmg.com.ar
License: GPL (>=2)
Depends:
R (>= 3.5),
methods
Imports:
edgeR,
stats,
MASS,
car,
mppa,
BiocParallel,
ggplot2,
reshape2
Suggests:
knitr,
RUnit,
BiocGenerics
Collate:
'NBSplice-package.R'
'IsoDataSet.R'
'IsoDataSet-getters.R'
'IsoDataSet-show.R'
'IsoDataSet-print.R'
'IsoDataSet-buildLowExpIdx.R'
'IsoDataSet-core.R'
'NBSpliceRes.R'
'NBSpliceRes-initialize.R'
'NBSpliceRes-constructor.R'
'NBSpliceRes-getters.R'
'NBSpliceRes-show.R'
'NBSpliceRes-print.R'
'IsoDataSet-NBTest.R'
'totalGeneCounts.R'
'IsoDataSet-initialize.R'
'IsoDataSet-constructor.R'
'NBSpliceRes-getDSGenes.R'
'NBSpliceRes-getDSResults.R'
'NBSpliceRes-getGeneResults.R'
'NBSpliceRes-plotRatiosDisp.R'
'NBSpliceRes-plotVolcano.R'
'NBSpliceRes-plotGeneResults.R'
'NBSplice-isoCounts.R'
'NBSplice-geneIso.R'
'NBSplice-designMatrix.R'
'NBSplice-myIsoDataSet.R'
'NBSplice-myDSResults.R'
biocViews: Software, StatisticalMethod, AlternativeSplicing, Regression,
DifferentialExpression, DifferentialSplicing, RNASeq
VignetteBuilder: knitr
RoxygenNote: 6.0.1
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.
@lshep will review your package. Please update the vignette so that installation instructions assume the package has been accepted to Bioconductor -- biocLite("NBSplice)
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.
Received a valid push; starting a build. Commits are:
9f0c4d1 Correcting packages errors
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.
Received a valid push; starting a build. Commits are:
5dd726b Registering at the support site
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.
Received a valid push; starting a build. Commits are:
b65bf22 Updating vignette assuming NBSplice has been accep...
Hello!
I have updated the package vignette assuming that NBSplice has been accepted to Bioconductor.
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.
Received a valid push; starting a build. Commits are:
2ac3633 Correcting fitModel function
I have updated the fitModel function to solve a problem found when the contrast parameter is specified by the user.
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.
General
[ ] Please comment on some differences from existing packages: ballgown EBseq / EBseqHMM IsoformSwitchAnalyzeR
[ ] Why did you develop a new class? The data is rectangular and seems like SummarizedExperiment or MultiAssayExperiment should be used or extended. Justify why a new class is needed. There are additional comments about the classes in vignette feedback but again, if you think your new classes are justified it is ok to leave as is but please give explanation . Best practices involve reusing existing Bioconductor infastructure or at least having wrappers that may incorporate common structures for data types.
Man Pages
[ ] For data objects: More information on how these were created? What is source? what code was run to construct? Where is the example data taken from?
[ ] (optional) It is recommended to document constructor and method functions for particular classes on the same page. So the IsoDataSet and NBSplice indivudal pages could be combined. This way all the relavant documentation is on one help page rather than a user having to navigate through several to find which was needed. Achieved through @rdname, @describedIn
Vignette
[ ] Input Data - are these geneIso and designMatrix generated from some instrumentation or are they user constructed? There needs to be more information here.
[ ] If using expression counts, the functions can take in matrices but should also minimally take in SummarizedExperiment objects? could it take in both?
[ ] Can the geneIso also be represented as a geneSetCollection object from GSEABase?
[ ] It seems like relavant genoIso and designMatrix could be pData of a SummarizedExperiment object, so beside individual matrices could work with this object?
[ ] Your functions aren't actually run in the vignette? You trick it by storing the results and then loading the data objects. These should be run interactively whenever possible. Please remove the eval=FALSE.
[ ] Instead of own class object for IsoDataSet, please investigate the use of SummarizedExperiment or MultiAssayExperiment. Justify why a new class is needed
Like the getter methods! Very nice
The further exploration functions for your results are also nice!
Thank you for including the model information and implementation sections.
R code
Please make the adjustments above. When ready please do another version bump to kick off a new build and comment back here with updates and justifications.
Cheers
Received a valid push; starting a build. Commits are:
9d54629 NBSplice optimization following BioC team recommen...
Hello Lori,
Thank you for all your comments about our package. I have pushed a new version of NBSplice fixing many of the issues you identified. I detailed below the changes and suggestions you made me and my responses, in bold text.
Please, let me know about any inconvenient or another change needed.
Cheers
General
Differences with existing packages: ballgown, EBSeq/EBSeqHMM and IsoformSwitchAnalzeR: To our knowledge, the packages ballgown, EBSeq, and EBSeqHMM perform differential isoform expression. However, they don’t evaluate differential isoform usage or differential splicing which is the aim of NBSplice. The IsoformSwicthAnalyzeR package tests differential isoform usage by independent analysis of gene isoforms based on non-parametric tests. We consider that this approach could be useful when the number of replicates per condition is high, as in the IsoformSwicthAnalyzeR paper. However, when a low number of replicates are available, parametric approaches are more appropriated. Our package proposes the identification of differential spliced genes by linear hypothesis test and the Simes test performed on coefficients of a robust parametric model sharing information across gene isoforms. Our approach is also novel because it identifies low-expressed isoforms which are not modeled but their counts are considered on the model fitting avoiding overestimation of high-expressed isoforms.
Why did you develop a new class?: Although we identified that the isoCounts, geneIso and designMatrix objects could be easily represented as the assay, rowData and colData of SummarizedExperiment class, we decided to develop a new class because our assay data is composed of two expression matrices, at the isoform level and at the gene level. In addition, the identification instead of the elimination of low-expressed isoforms is a key on NBSplice. The index of low-expressed isoforms are stored as a class slot and is frequently used to discard or kept those isoforms from the analysis.
Man Pages
For data objects: More information on how these were created? What is source? what code was run to construct? Where is the example data taken from? This information was added to the corresponding documentation files.
(optional) It is recommended to document constructor and method functions for particular classes on the same page. So the IsoDataSet and NBSplice indivudal pages could be combined. This way all the relavant documentation is on one help page rather than a user having to navigate through several to find which was needed. Achieved through @rdname, @describedin We decided to use different man pages to avoid so long documentation pages. To help the navigation we have used the @seealso field to link with another relevant methods
Vignette
Input Data - are these geneIso and designMatrix generated from some instrumentation or are they user constructed? There needs to be more information here. This information was added to the corresponding sections.
Can the geneIso also be represented as a geneSetCollection object from GSEABase? We considered that it is not needed because geneIso a single matrix or data.frame with two columns containing isoform and gene IDS. Please, let me known if you think that it is necessary
Your functions aren't actually run in the vignette? You trick it by storing the results and then loading the data objects. These should be run interactively whenever possible. Please remove the eval=FALSE. This has been modified
-If using expression counts, the functions can take in matrices but should also minimally take in SummarizedExperiment objects? could it take in both? I don't completely understand the question. Is about building an IsoDataSet from a summarizedExperiment object? NBsplice methods have been created based on kallisto and RSEM expression counts which are stored in tab files. For this reason, we have based the IsoDataSet constructor on matrixes or data.frames.
R code
You define getters for the class but then internal don't use them. For example in NBTest, object@geneCounts - but you have defined a geneCount() getter method for the class. Please use the getter methods whenever possible. We have used the getters when it was possible. In the case of the geneCounts slot of an IsoDataSet object it stores an ‘expanded expression matrix’ at the gene level. This matrix is ‘expanded’ because it has as many rows as the isoCount slot has, i.e.: each row represents an isoform. It is useful to avoid time consuming each time that we need to work with the expanded matrix. Whereas, the geneCount getter returns the reduced expression matrix at the gene level, i.e each row represents a gene. When we use the object@geneCounts we are working with the expanded expression matrix.
For loops like those found at NBTest line # 101, should probably be updated to a vapply for efficiency. We have incorporated the use of vapply instead of the loop for in this line.
Similarly NBtest for loop at #127 - you could preallocate the data.frame and fill by line rather than the efficient concatentate to the vectors. Done
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.
Hi Lori,
I have updated NBSplice including the changes you recommended.
Should I push it again to avoid the warnings in the build report? I didn't see those on my laptop so I suppose they are a temporary warning.
Best
Gabriela
Yes please push it again to see if it resolves. Even if it remains I will review the package again but it would be nice to have the clean report.
Received a valid push; starting a build. Commits are:
f73f646 Fixing report warnings
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.
I'll ignore it for now and have a review for you soon.
Thank you. I'm replicating the build, install and checks steps on my Mac, but I don't get those warnings.
Thank you. While I still strongly encourage expanding or utilizing SummarizedExperiment or MultiAssayExperiment class structure it is your decision as a maintainer.
Please update the installation instructions as we are currently in the process of updating this site wide. There is a new package on CRAN BiocManager that will install Bioconductor packages and avoid sourcing a script.
if (!require("BiocManager"))
install.packages("BiocManager")
BiocManager::install("NBSplice", version="devel")
Received a valid push; starting a build. Commits are:
7e626bd Updating information about package installation
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.
Hi Lori,
I have updated the installation instructions as you suggested before.
Best
Gabriela
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!
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/gamerino.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(\"NBSplice\")
. The package 'landing page' will be created at
https://bioconductor.org/packages/NBSplice
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.
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]'
[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:
For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.