Closed markziemann closed 4 years ago
Hi @markziemann
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: mitch
Title: Multidimensional Gene Set Enrichment Analysis
Version: 0.2.5
Authors@R: c(
person(given = "Mark",
family = "Ziemann",
role = c("aut","cre","cph"),
email = "mark.ziemann@gmail.com"),
person(given="Antony",
family="Kaspi",
role = c("aut","cph"))
)
Description: Mitch applies rank-MANOVA to identify differential gene set enrichments in single and multi-profiling data.
Multi-profiling data can be multi-omics such as combinations of proteomics, transcriptomics, epigenomics, but
also high throughput profiling datasets of up to 15 contrasts. Mitch imports differential expression data from
commonly used tools such as edgeR, limma, DESeq2, ABSSeq and Sleuth as well as scRNA-seq data output by Seurat.
Mitch can output data in the form of a high resolution PDF or as a self contained HTML report.
Depends: R (>= 3.6.0)
Suggests:
Imports: pkgload, fs, remotes, processx, stats, tidyselect, grDevices, graphics, utils, MASS, plyr, reshape2, parallel, pbmcapply, GGally, grid, gridExtra, knitr, rmarkdown, ggplot2, gplots, beeswarm
License: CC BY-SA 4.0 + file LICENSE
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.1.1.9001
biocViews: GeneExpression,GeneSetEnrichment,SingleCell,Transcriptomics,Epigenetics,Proteomics, DifferentialExpression
VignetteBuilder: knitr
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.
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.
Received a valid push; starting a build. Commits are:
3fc1259 builds OK
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.
Received a valid push; starting a build. Commits are:
31b5cc0 fixing few R CMD checks
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.
Received a valid push; starting a build. Commits are:
160b140 fix Rmd
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:
4191240 bumb version
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:
9540c6a bump
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:
21e27e6 version bump
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:
cfdd95b bump
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.
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.
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:
0b4d4c7 added support for importing data from several upst...
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.
Hello @markziemann,
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.
[ ] REQUIRED: The directory 'data-raw' is not allowed. Raw data should go in the 'inst/extdata/' directory. Please move and remove things accordingly.
[ ] REQUIRED: If 'example_datagen.R' explains how the data was generated, than this belongs in the 'inst/script/' directory.
[ ] REQUIRED: What is the purpose of the 'mitch.Rmd' file. It is present in the top directory as well as in 'inst/'.
[ ] REQUIRED: There should be no R files in the top directory, what is the purpose of 'quickStart.R'.
[ ] SUGGESTION: In the 'Description:' I would keep things consistent and use 'mitch' instead of 'Mitch'.
[ ] REQUIRED: 'LazyData:' should be set to 'false'. When this is true it can slow down the loading of packages that include data.
[ ] SUGGESTION: It is encouraged to include the relevant link to Github for reporting Issues.
[ ] REQUIRED: With the 3.10 release, we are now under a new devel version so you need to update R version dependency from 3.6.0 to 4.0.
[ ] REQUIRED: There should be packages listed for 'Suggests:', these packages are ones that are used in vignettes or examples. If there are none, please remove this tag.
[ ] SUGGESTION: Consider adding the automatically suggested biocViews 'Reactome'.
importFrom()
is encouraged over importing an entire
package, however if there are many functions from a single package, import()
is okay. Please try to limit your use of import()
and use importFrom()
where
possible.inst/extdata/
.
This should be located in an inst/script/
directory. More on this below.[ ] SUGGESTION: Throughout the whole vignette, I would keep things consistent and use 'mitch' instead of 'Mitch'.
[ ] REQUIRED: The 'Background' section should serve as an abstract to introduce the objective, models, unique functions, key points, etc that distinguish the package from other packages of similar type. You direct users to the github for more information but you should be explain the package in more detail here.
[ ] REQUIRED: Before demonstrating what the package can do there should be an
'Installation' section that shows the user how to download the package from
Bioconductor. The code should look something like this and have eval = FALSE
.
if(!requireNamespace("BiocManager", quietly = TRUE))
install.packages("BiocManager")
BiocManager::install("mitch")
[ ] REQUIRED: After showing the user how to download the data, you should show
the library(mitch)
command in line 21 by changing include
from FALSE to
TRUE.
[ ] SUGGESTION: We strongly encourage a table of contents.
[ ] SUGGESTION: Use ` ` around function names so that they will be formatted
correctly, for example mitch_import
in line 38 and mitch_calc
in line 95.
[ ] REQUIRED: Why do you access data from reactome.org when you provide this
data in the data/
directory of the package?
[ ] REQUIRED: A vignette provides reproducibility, the vignette produces the
same results as copying the corresponding commands into an R session. Therefore,
lines 43-51 should be shown to the user. Or else, when the user tries to run
line 53-55 dge1
and dge2
won't be known.
[ ] REQUIRED: Lines 44, 47, and 50, data should not be imported from github.
This is the data you provide in the package so it should be accessed using
system.file()
.
[ ] REQUIRED: Lines 57-59 should be shown to the user.
[ ] REQUIRED: Lines 63-65 and 70-72, it's okay to have these code chunks as
eval = FALSE
but you should explain to the user that they are just examples
and nothing will happen if they are evaluated.
[ ] REQUIRED: Lines 101-103 should not be eval = FALSE
.
[ ] REQUIRED: The last section of the vignette should be 'Session Info' and
should include sessionInfo()
.
inst/extdata/
, a related script must be present in this
directory documenting very clearly how the data was generated. It should include
source urls and any important information regarding filtering or processing. It
can be executible code, sudo code, or a text description. A user should be able
to download the data and roughly reproduce the file or object that is present as
data.RUnit
and testthat
.
Please see http://bioconductor.org/developers/how-to/unitTesting-guidelines/ for
more details on how to use one of these two packages to test.[ ] REQUIRED: Undefined global functions for variables: mitch_report: no visible binding for global variable 'x' mitch_report: no visible binding for global variable 'y'
[ ] SUGGESTION: For formatting reasons, consider shorter lines; 209 lines are > 80 characters long.
[ ] SUGGESTION: For formatting reasons, consider multiples of 4 spaces for line indents, 8 lines are not.
Best, Kayla
Dear Kayla, thanks for the expert review! I will let you know when I have addressed all points.
Received a valid push; starting a build. Commits are:
7dabbd7 updates to remove data-raw and update DESCRIPTION
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.
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.
Received a valid push; starting a build. Commits are:
b1b26d8 just about finished the BioC requested changes
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.
Received a valid push; starting a build. Commits are:
a2fa661 bump
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:
5b50041 fixed tests namespace and long lines
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.
Hello @Kayla-Morrell it would appear that the package builder is still using R version 3.6 and is thus giving an error. What is your advice: should I roll back the mitch dependancy version or wait until the builder R version is updated? I also wanted to ask your permission to acknowledge you by name for your help in reviewing the package. Is that OK?
Hi, it seems a very interesting package. I don't know if there is a need to create a new gmt_import
function, as this is implemented already on the core package GSEABase.
Also for clarity probability it would be better to split the big mitch.R file in several smaller ones for future maintenance, for instance one file for scores functions, another for plotting functions...
Hope it helps and the package will be soon included in Bioconductor
Hi again @Kayla-Morrell
Here I've outlined my responses to the items raised.
General package development REQUIRED: The directory 'data-raw' is not allowed. Raw data should go in the 'inst/extdata/' directory. Please move and remove things accordingly.
Done: I have moved the 'data-raw' directory to inst/exdata.
REQUIRED: If 'example_datagen.R' explains how the data was generated, than this belongs in the 'inst/script/' directory.
Done; moved it to 'inst/script/' directory.
REQUIRED: What is the purpose of the 'mitch.Rmd' file. It is present in the top directory as well as in 'inst/'.
‘mitch.Rmd’ in the ‘inst/’ directory is used as a template for users to generate an analysis report. The ‘mitch.Rmd’ file in the top directory has been deleted.
REQUIRED: There should be no R files in the top directory, what is the purpose of 'quickStart.R'.
Files ‘quickStart.R’ and ‘mitch.Rmd’ have been deleted from the top directory.
DESCRIPTION SUGGESTION: In the 'Description:' I would keep things consistent and use 'mitch' instead of 'Mitch'.
Done
REQUIRED: 'LazyData:' should be set to 'false'. When this is true it can slow down the loading of packages that include data.
Done; used “data()” in examples to load necessary input datasets.
SUGGESTION: It is encouraged to include the relevant link to Github for reporting Issues.
Done
REQUIRED: With the 3.10 release, we are now under a new devel version so you need to update R version dependency from 3.6.0 to 4.0.
Done
REQUIRED: There should be packages listed for 'Suggests:', these packages are ones that are used in vignettes or examples. If there are none, please remove this tag.
Stringi, and testthat have been added to suggests. Stringi is used in the vignette and testthat for unit tests.
SUGGESTION: Consider adding the automatically suggested biocViews 'Reactome'.
Done
NAMESPACE REQUIRED: Generally importFrom() is encouraged over importing an entire package, however if there are many functions from a single package, import() is okay. Please try to limit your use of import() and use importFrom() where Possible.
NEWS REQUIRED: Thank you for including a NEWS file, please update this file with relevant information.
Data REQUIRED: There needs to be documentation for the data in inst/extdata/. This should be located in an inst/script/ directory. More on this below.
Vignettes SUGGESTION: Throughout the whole vignette, I would keep things consistent and use 'mitch' instead of 'Mitch'.
Done
REQUIRED: The 'Background' section should serve as an abstract to introduce the objective, models, unique functions, key points, etc that distinguish the package from other packages of similar type. You direct users to the github for more information but you should be explain the package in more detail here.
I have expanded the Background section to cover these points
REQUIRED: Before demonstrating what the package can do there should be an 'Installation' section that shows the user how to download the package from Bioconductor. The code should look something like this and have eval = FALSE. if(!requireNamespace("BiocManager", quietly = TRUE)) install.packages("BiocManager") BiocManager::install("mitch")
Done
REQUIRED: After showing the user how to download the data, you should show the library(mitch) command in line 21 by changing include from FALSE to TRUE.
Done
SUGGESTION: We strongly encourage a table of contents.
Done
SUGGESTION: Use
around function names so that they will be formatted
correctly, for example mitch_import in line 38 and mitch_calc in line 95.
Done
REQUIRED: Why do you access data from reactome.org when you provide this data in the data/ directory of the package?
REQUIRED: A vignette provides reproducibility, the vignette produces the same results as copying the corresponding commands into an R session. Therefore, lines 43-51 should be shown to the user. Or else, when the user tries to run line 53-55 dge1 and dge2 won't be known.
This has been rectified and each time mitch_import is executed the head of the result is shown to the user.
REQUIRED: Lines 44, 47, and 50, data should not be imported from github. This is the data you provide in the package so it should be accessed using system.file().
Based on this comment I’ve changed the vignette to only use the data in the R function examples, which has simplified things a bit.
REQUIRED: Lines 57-59 should be shown to the user.
I’ve made sure that all the code chunks are shown to the user.
REQUIRED: Lines 63-65 and 70-72, it's okay to have these code chunks as eval = FALSE but you should explain to the user that they are just examples and nothing will happen if they are evaluated.
Due to the change outlined above, all chunks are evaluated except the installation
REQUIRED: Lines 101-103 should not be eval = FALSE.
Due to the change outlined above, all chunks are evaluated except the installation
REQUIRED: The last section of the vignette should be 'Session Info' and should include sessionInfo().
Done
Man pages REQUIRED: Data man pages must include source information and data structure information. You include source information but there should be more details about the structure of the data.
inst/script/ REQUIRED: The scripts in this directory can vary. Most importantly if data was included in the inst/extdata/, a related script must be present in this directory documenting very clearly how the data was generated. It should include source urls and any important information regarding filtering or processing. It can be executable code, sudo code, or a text description. A user should be able to download the data and roughly reproduce the file or object that is present as Data.
Unit tests REQUIRED: Thank you for including tests, but they are not formatted correctly. The two main frameworks for testing are RUnit and testthat. Please see http://bioconductor.org/developers/how-to/unitTesting-guidelines/ for more details on how to use one of these two packages to test.
R code REQUIRED: Undefined global functions for variables: mitch_report: no visible binding for global variable 'x' mitch_report: no visible binding for global variable 'y'
Done.
SUGGESTION: For formatting reasons, consider shorter lines; 209 lines are > 80 characters long.
Done
SUGGESTION: For formatting reasons, consider multiples of 4 spaces for line indents, 8 lines are not.
Done I think. I’ve screened for the lines with odd intends and fixed them. The following awk command yields no results.
awk -F'[^ ]' '{print length($1),NR}' R/mitch.R | egrep -vw '(0|4|8|12|16)'
Hello @llrs , thanks for these good suggestions. I will address these after the BioC review
@markziemann Please note that I'm not an official reviewer, so you are not compelled to do it, but I think they will help you in the long run.
Received a valid push; starting a build. Commits are:
fb0f388 rollback Rversion
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.
Hello @markziemann - For some reason the build report from your last commit never posted so I kicked off a manual build so you would have a proper build report. I guess I was a bit preemptive with my review, we are still working on updating the R version on the single package builder. It should hopefully be resolved by the end of the week. I can let you know when to change it back to R 4.0, but leaving it R 3.6 for now is fine.
Thank you for your patience and I will re-review the package by the end of this week.
Best, Kayla
Received a valid push; starting a build. Commits are:
22be982 tests_working
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:
c5a5bac adjust test for html report
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.
Hello @markziemann - Thank you for making the necessary changes. I have one additional requirement for you to make to your package and then it should be nearly ready to be accepted. Please see the note below.
inst/script/example_datagen.R
file, avoid the use of system()
and instead use system2()
. system2
is a more portable and flexible interface than system
.Other than that we just need to wait for R 4.0 to be formatted correctly on our builders, make sure you get a clean build with the new R version, and then I can accept your package. I will let you know when we are ready for you to change the dependency in your package.
Thanks again for your patience, Kayla
Received a valid push; starting a build. Commits are:
bfd7ed1 got rid of the system() command
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.
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.