Closed ghost closed 5 years ago
Hi @abrionne
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: ViSEAGO
Version: 0.99.0
Title: Visualisation, Semantic similarity, Enrichment Analysis of Gene Ontology
Authors@R: c(
person("Aurelien","Brionne", role=c("aut","cre"),
email="aurelien.brionne@inra.fr"),
person("Amelie","Juanchich", role="aut",
email="amelie.juanchich@inra.fr"),
person("Christelle","Hennequet-Antier", role="aut",
email="christelle.hennequet-antier@inra.fr"))
Depends: R (>= 3.5)
Imports: data.table,
AnnotationDbi,
AnnotationForge,
biomaRt,
dendextend,
DiagrammeR,
GOSemSim,
DT,
dynamicTreeCut,
ggplot2,
GO.db,
heatmaply,
htmltools,
htmlwidgets,
igraph,
org.Mm.eg.db,
methods,
plotly,
topGO,
Rcpp,
RColorBrewer,
Rgraphviz,
R.utils,
scales,
UpSetR,
webshot,
BiocStyle,
knitr,
rmarkdown,
corrplot,
remotes,
BiocManager
Description: The main objective of ViSEAGO workflow is to carry out a data mining of biological functions and establish links between genes involved in the study. We developed ViSEAGO in R to facilitate functional Gene Ontology (GO) analysis of complex experimental design with multiple comparisons of interest. It allows to study large-scale datasets together and visualize GO profiles to capture biological knowledge. It stands for three major concepts: Visualization, Semantic similarity and Enrichment Analysis of Gene Ontology. It provides access to the last current GO annotations, which are retrieved from one of NCBI EntrezGene, Ensembl or Uniprot databases for available species. ViSEAGO extends classical functional GO analysis to focus on functional coherence by aggregating closely related biological themes while studying multiple datasets at once. It provides both a synthetic and detailed view using interactive functionalities respecting the GO graph structure and ensuring functional coherence supplied by semantic similarity. ViSEAGO has been successfully applied on several datasets from different species with a variety of biological questions. Results can be easily shared between bioinformaticians and biologists, enhancing reporting capabilities while maintaining reproducibility.
VignetteBuilder: knitr
License: GPL-3 + file LICENSE
Author: Aurelien Brionne [aut, cre],
Amelie Juanchich[aut],
christelle Hennequet-Antier [aut]
Maintainer: Aurelien Brionne <aurelien.brionne@inra.fr>
Repository: forgemia.inra.fr,
bioconductor.org
URL: https://forgemia.inra.fr/UMR-BOA/ViSEAGO,
https://www.bioconductor.org/packages/release/bioc/html/ViSEAGO.html
BugReports: https://forgemia.inra.fr/UMR-BOA/ViSEAGO/issues
biocViews: Software, Annotation, GO, GeneSetEnrichment, MultipleComparison, Clustering, Visualization
RoxygenNote: 6.1.1
Collate:
'genomic_ressource.R'
'Bioconductor2GO.R'
'Ensembl2GO.R'
'EntrezGene2GO.R'
'EntrezGene_orthologs.R'
'enrich_GO_terms.R'
'GO_SS.R'
'GO_clusters.R'
'GOclusters_heatmap.R'
'GOcount.R'
'GOterms_heatmap.R'
'MDSplot.R'
'Uniprot2GO.R'
'Upset.R'
'annotate.R'
'available_organisms.R'
'build_GO_SS.R'
'clusters_cor.R'
'compare_clusters.R'
'compute_SS_distances.R'
'create_topGOdata.R'
'datasets.R'
'gene2GO.R'
'merge_enrich_terms.R'
'overlapper.R'
'pkgdiagram.R'
'show_heatmap.R'
'show_table.R'
'taxonomy.R'
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: "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:
93be0df Version: 0.99.1
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:
ad94113 Version: 0.99.2
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:
bfbd8ab Version: 0.99.3
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: "TIMEOUT, 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:
1255a56 Version: 0.99.4
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: "TIMEOUT, 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:
b1ec4fc Version: 0.99.5
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:
0abcdb0 Version: 0.99.6
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:
f0c6d8b Version 0.99.7
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:
3450fad Version: 0.99.8
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:
0213739 Version: 0.99.9
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: "TIMEOUT, 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:
68229bc Version: 0.99.10
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:
5b755a9 Version 0.99.11
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:
5e92d25 Version: 0.99.12
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:
b99a3e9 Version: 0.99.13 (Rversion 3.6)
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.
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:
5a9ab90 Version 0.99.15 (vignette speedup)
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.
@nturaga was away this week but here are a few comments to get you started:
Build Report
annotate.R
. stop()
and recommend the user installs themselves.README
NEWS
?news
LICENSE
inst/extadata
man
vignettes
[ ] Optional but I recommend renaming overview.Rmd to ViSEAGO.Rmd. It is much more intuitive for a user to type vignette("ViSEAGO")
than vignette("overview")`. It is also not advisable to have something so generic as there could be potential naming conflict depending on other package dependencies loaded. When I tried to load your vignette, the wrong vignette appeared because GenomicDataCommons also has a vignette named overview that is first in the load path
> library(ViSEAGO)
> vignette("overview")
starting httpd help server ... done
Warning message:
vignette 'overview' found more than once,
using the one found in '/home/lori/R/x86_64-pc-linux-gnu-library/3.6PreRel/GenomicDataCommons/doc'
[ ] The vignette global option eval=FALSE
should not be used. Vignettes should be evaluated and run interactively. Please update. Also the fig.path should not write to the home directory. Please update to use tempdir/tempfile
to write output.
[ ] The vignette should operate from a tempdir()/tempfile()
and not create/write to a users home directory to avoid potentially overwritting a users existing directories or files. Please update throughout. So in your call to base::dir.create
this should not be to the home directory.
[ ] eval=FALSE
should be used sparingly. Please justify any use of eval=FALSE in the vignette code functions. I see some of your commits to improve build time where to add eval=FALSE, we would prefer the longer build time and code being fully executed/tested
@nturaga can do a more thorough review of the underlying R code but please start working on these.
Received a valid push; starting a build. Commits are:
f65c82d package upgrade using @lshep comments
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:
3e09b5b ext/data/ script T/F corrections
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.
Build Report
- [x] Fix the build report for global variables and function definitions. NB: no visible binding for global variable comes from data.table syntax. I modified style syntaxe for prevent theses notes except for lines 204 and 212 in compute_SS_distances. build report: " compute_SS_distances,ANY-character: no visible binding for global variable ‘N’ compute_SS_distances,ANY-character: no visible binding for global variable ‘IC’ Undefined global functions or variables: IC N"
- [x] Do not try to install packages for a user in
annotate.R
.stop()
and recommend the user installs themselves.README
- [x] Also include Bioconductor Instructions
NEWS
- [x] Do not have an empty NEWS file. Either remove or add a relevant informative message. See formatting with
?news
LICENSE
- [x] Can be removed. The GPL-3 licenses is already distributed with R. inst/extadata
- [x] Please provide a corresponding inst/script that describes how the data in inst/extadata was generated along with any source information.
man
- [x] I recommend having a package level man page ViSEAGO.Rd that gives an overview of the package and directs to the main functions of the package.
vignettes
- [x] Optional but I recommend renaming overview.Rmd to ViSEAGO.Rmd. It is much more intuitive for a user to type
vignette("ViSEAGO")
than vignette("overview")`.- [x] The vignette global option
eval=FALSE
should not be used. Vignettes should be evaluated and run interactively. Please update. Also the fig.path should not write to the home directory. Please update to usetempdir/tempfile
to write output.- [x] The vignette should operate from a
tempdir()/tempfile()
and not create/write to a users home directory to avoid potentially overwritting a users existing directories or files. Please update throughout. So in your call tobase::dir.create
this should not be to the home directory.- [x]
eval=FALSE
should be used sparingly. Please justify any use of eval=FALSE in the vignette code functions. I see some of your commits to improve build time where to add eval=FALSE, we would prefer the longer build time and code being fully executed/tested
I have checked package for mouse_bionconductor vignette:
I kept eval=FALSE to prevent warnings build report
Thanks to you @lshep for the first code pass. I wait now @nturaga report for the second pass.
@abrionne
Please look below for the review.
You seem to using the function for base R, in the following way,
base::paste
. This is not required. You can call the base R
functions by their name, unless you are redefining the function
paste
in your code.
Even this call to base::
is not done uniformly. This is cluttering your codebase for no reason, making it extremely hard to read.
Please remove, base::
to all functions, like cat, paste, vapply and all the functions/methods defined in base R.
Similarly with methods
, this is distributed with base R. For a
list of packages distributed by base R, try:
i <- installed.packages()
rownames(i[ i[,"Priority"] %in% c("base","recommended"), c("Package", "Priority")])
Please check this,http://bioconductor.org/developers/how-to/coding-style/
"""
Namespaces
Import all symbols used from packages other than “base”. Except for default packages (base, graphics, stats, etc.) or when overly tedious, fully enumerate imports.
Export all symbols useful to end users. Fully enumerate exports.
"""
Please format your code also accordingly after you have removed the calls to all the functions available to you with base R.
To check the class type of an object, please use the function is
.
is(object, "enrich_GO_terms")
as opposed to
class(object) %in% c("enrich_GO_terms") ...
Make this change everywhere this paradigm is used.
When issuing end-user-messages please make sure you are using the appropriate ones, http://bioconductor.org/developers/how-to/coding-style/.
End-User messages
message() communicates diagnostic messages (e.g., progress during lengthy computations) during code evaluation.
warning() communicates unusual situations handled by your code.
stop() indicates an error condition.
cat() or print() are used only when displaying an object to the user, e.g., in a show method.
eg:
You have to use message
here and not cat
base::cat("GOcount is required for multiples comparisons")
You seem to be using the function rm
to remove some object. This
is not needed, if you are "saving" memory. R does this
automatically. Please remove all calls to rm
.
This is in the file GOterms_heatmap.R. The way it's written right now, makes it extremely difficult to follow the logic as a reviewer.
I highly recommend you reformat your code by hand, using 4 space
indent and removing all the calls to <package>::
. If you are not
getting access to that function, pleaes import in your NAMESPACE.
There are unoptimized chunks of code which use apply
and other
loop like statements all over your code. In GOterms_heatmap.R,
mat<-base::apply(mat,base::c(1,2),function(x){
if(base::is.na(x)){
NA
}else{
if(x<2){0}else{1}
}
})
You could just write this as,
mat <- ifelse(mat < 2, 0, 1)
There is no need to use apply
here. Pleae check the use of
apply/loops all across your package. Some of the vapply
's are
also not needed. Please vectorize your code.
http://bioconductor.org/developers/how-to/efficient-code/
Look at the "Vectorize" section.
Instead of using seq_len(length(x))
you can use seq_along(x)
.
You don't need to use which
for subsetting. In overLapper.R
mycol1 <-base::which(base::colnames(setmatrix) %in% allcombl[[index]])
mycol2 <-base::which(!base::colnames(setmatrix) %in% allcombl[[index]])
cond1 <-base::rowSums(setmatrix[, base::rep(mycol1, 2)]) == 2 * base::length(mycol1)
cond2 <-base::rowSums(setmatrix[, base::rep(mycol2, 2)]) == 0
vs
mycol <-colnames(setmatrix) %in% allcombl[[index]]
cond1 <-matrixStats::rowSums2(setmatrix[, rep(mycol, 2)]) == 2 * length(mycol1))
cond2 <-matrixStats::rowSums2(setmatrix[, rep(!mycol, 2)]) == 0
Simplify your code.
The package needs some improvement before it can be accepted. Please make the changes given above, and report back point by point so I can go through the code again.
Thank you,
Nitesh
Received a valid push; starting a build. Commits are:
66f37b8 package upgrade according @nturaga comments
Hi Nitesh,
Thanks you for our package code evaluation. I think I made all required changes.
Sincerely,
Aurélien,
Review
R
You seem to using the function for base R, in the following way,
base::paste
. This is not required. You can call the base R functions by their name, unless you are redefining the functionpaste
in your code. Even this call tobase::
is not done uniformly. This is cluttering your codebase for no reason, making it extremely hard to read.
- [x] Please remove,
base::
to all functions, like cat, paste, vapply and all the functions/methods defined in base R.- [x] Similarly with
methods
, this is distributed with base R. For a list of packages distributed by base R, try:i <- installed.packages()
rownames(i[ i[,"Priority"] %in% c("base","recommended"), c("Package", "Priority")])
Please check this,http://bioconductor.org/developers/how-to/coding-style/
--> I do it, but now I have a NOTE during check process for global variable environment for methods package functions.
"""
Namespaces
[x] Import all symbols used from packages other than “base”. Except for default packages (base, graphics, stats, etc.) or when overly tedious, fully enumerate imports.
[x] Export all symbols useful to end users. Fully enumerate exports.
"""
- [x] Please format your code also accordingly after you have removed the calls to all the functions available to you with base R.
- [x] To check the class type of an object, please use the function
is
.is(object, "enrich_GO_terms")
as opposed to
class(object) %in% c("enrich_GO_terms") ...
Make this change everywhere this paradigm is used.
- When issuing end-user-messages please make sure you are using the appropriate ones, http://bioconductor.org/developers/how-to/coding-style/.
* * [x] End-User messages message() communicates diagnostic messages (e.g., progress during lengthy computations) during code evaluation. warning() communicates unusual situations handled by your code. stop() indicates an error condition. cat() or print() are used only when displaying an object to the user, e.g., in a show method.
eg:
You have to use
message
here and notcat
base::cat("GOcount is required for multiples comparisons")
- [x] You seem to be using the function
rm
to remove some object. This is not needed, if you are "saving" memory. R does this automatically. Please remove all calls torm
. This is in the file GOterms_heatmap.R. The way it's written right now, makes it extremely difficult to follow the logic as a reviewer.
- [x] I highly recommend you reformat your code by hand, using 4 space indent and removing all the calls to
<package>::
. If you are not getting access to that function, pleaes import in your NAMESPACE.- [x] There are unoptimized chunks of code which use
apply
and other loop like statements all over your code. In GOterms_heatmap.R,mat<-base::apply(mat,base::c(1,2),function(x){ if(base::is.na(x)){ NA }else{ if(x<2){0}else{1} } })
You could just write this as,
mat <- ifelse(mat < 2, 0, 1)
There is no need to use
apply
here. Pleae check the use of apply/loops all across your package. Some of thevapply
's are also not needed. Please vectorize your code. http://bioconductor.org/developers/how-to/efficient-code/ Look at the "Vectorize" section.
- [x] Instead of using
seq_len(length(x))
you can useseq_along(x)
.
- [ ] You don't need to use
which
for subsetting. In overLapper.Rmycol1 <-base::which(base::colnames(setmatrix) %in% allcombl[[index]]) mycol2 <-base::which(!base::colnames(setmatrix) %in% allcombl[[index]]) cond1 <-base::rowSums(setmatrix[, base::rep(mycol1, 2)]) == 2 * base::length(mycol1) cond2 <-base::rowSums(setmatrix[, base::rep(mycol2, 2)]) == 0
vs
mycol <-colnames(setmatrix) %in% allcombl[[index]] cond1 <-matrixStats::rowSums2(setmatrix[, rep(mycol, 2)]) == 2 * length(mycol1)) cond2 <-matrixStats::rowSums2(setmatrix[, rep(!mycol, 2)]) == 0 -->I am sorry but this code don't work (even mycol1 to mycol correction), I keep the previous. Any idea?
Simplify your code.
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:
9222191 Version 0.99.20
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:
946503a switch in description R version from 3.5 to 3.6
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:
501f8c1 add R.utils package description and namespace
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.
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.