Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

GOpro #120

Closed lidiaad closed 7 years ago

lidiaad commented 8 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 8 years ago

Hi @lidiaad

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: GOpro
Type: Package
Title: Finds the most characteristic gene
    ontology terms for groups of human genes
Version: 0.99.0
Date: 2016-08-31
Author: Lidia Chrabaszcz
Maintainer: Lidia Chrabaszcz <chrabaszcz.lidia@gmail.com>
Description: Finds the most characteristic gene ontology terms for
    groups of human genes.
    Based on the gene expressions find the structure somewhat
    comparable to gene signature. From all given genes,
    determine which are significantly different between sets.
    These sets may relate to different health conditions of patients,
    i.e. different types of cancer.
    Then divide interesting genes into subsets.
    Genes belong to a particular subset if they share the same feature.
    There are two implemented methods that can be used to create
    genes' subsets.
    The first method is so-called all pairwise comparisons by
    Tukey's procedure.
    Genes that have the same profile (a result of all comparisons)
    are assigned to one subset.
    The second way of determining subsets is a method of
    hierarchical clustering.
    When all genes are divided into subsets, then
    for each subset all relevant GO terms are searched for
    in org.Hs.eg.db database.
    Each found GO terms is tested using Fisher's test to find out
    which of them are the most characteristic for the given subset of genes.
    This package was created as a part of
    the thesis which was developed under the auspices of MI^2 Group
    (http://mi2.mini.pw.edu.pl/, https://github.com/geneticsMiNIng).
License: GPL-3
Depends:
    R (>= 3.3)
Imports:
    AnnotationDbi,
    dendextend,
    doParallel,
    foreach,
    parallel,
    org.Hs.eg.db,
    GO.db,
    Rcpp,
    stats,
    graphics
LinkingTo: Rcpp, BH
RoxygenNote: 5.0.1
Suggests: knitr,
    rmarkdown,
    RTCGA.PANCAN12
VignetteBuilder: knitr
LazyData: true
biocViews: Annotation, Clustering, GO,
    GeneExpression, GeneSetEnrichment, MultipleComparison
URL: https://github.com/mi2-warsaw/GOpro
BugReports: https://github.com/mi2-warsaw/GOpro/issues
bioc-issue-bot commented 8 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 8 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/GOpro_buildreport_20160919164159.html

lshep commented 8 years ago

Thank you for your submission to Bioconductor. Please see the following that need to be addressed:

GENERAL:

It seems like your function findGO is really the main function of your package. It is the simplest for a user to use and condenses down all the intermediate steps you go through in the current vignette; Yet it is not mentioned anywhere. I would focus on this function in the vignette and also reference it in the GOpro.Rd (ie. in GOpro.Rd add a \seealso{\link{findGO}} )

The steps of using aovTopTest, groupByTukey/clustering, findAllGOs, findTopGOs, etc... are all run from within findGO. I would recommend, not exposing the user to these intermediate functions (aovTopTest, groupByTukey/clustering, findAllGOs, findTopGOs, etc..), making them internal, and only focusing on findGO. It also seems like tukeyHSDTest, isConflict, makeNames, TukeyCore, and ontologyOver are all used internally and never by a user; please also make these internal.

We like compatibility with other Bioconductor objects. I would change or at least additionally add the input of findGO to be compatible with a MultiAssayExperiment, which is comparable with your list of data.frames of varying dimension. See: MultiAssayExperiment

Your output could be more structured and utilize the exisiting S4Vector infastructure for list of character vectors, etc. It is fine if you would like to define your own class("GO" and "geneclusterplot"), but please base it off of a more structured S4Vectors class like CharacterList, NumericList, etc. See: IRanges

library(IRanges)
?CharacterList

An example of use of CharacterList including how to subset:

libary(IRanges) 
cl = CharacterList(list(letters[1:5], LETTERS, state.name))
cl
il = IntegerList(list(1:5, 1:26, 1:52))
df = DataFrame(char = cl, int = il)
cl[il > 11]

(Note: if you really want to keep all the intermediate functions (aovTopTest, groupByTukey/clustering, findAllGOs, findTopGOs, etc) visible and accessable to the user, we would require the output of these structures to be more in line with a CharacterList, NumericList, etc. )

OTHER COMMENTS:

Remove all "::" for R code. You should not need these if the packages are properly specified in the DESCRIPTION and NAMESPACE files.

I feel like the DESCRIPTION Description should be the consice description in your man page GOpro.Rd and this more detailed description should be in GOpro.Rd.

Please look over your vignette. There are some typos and phrasing issues.
Please also change the output to BiocStyle::html_document and add suggest BiocStyle to the DESCRIPTION. Bioconductor likes to have a consistant vignette syle. vignette sytle

Please also include a section for installation and loading by adding sections like the following:

<installation,eval=FALSE>>=
source("http://www.bioconductor.org/biocLite.R")
biocLite("GOpro",dependencies=TRUE)
@

After the package is installed, it can be loaded into R workspace typing

<<loading,eval=TRUE,results='hide',warning=FALSE,message=FALSE>>=
library(GOpro)
@

I think your packages has a lot of potential and with some restructuring and focus will be a nice addition to Bioconductor. I look forward to reviewing your updates.

bioc-issue-bot commented 8 years ago

We only start builds when the Version field in the DESCRIPTION file is incremented. For example, by changing

Version: 0.99.0

to

Version 0.99.1

If you did not intend to start a build, you don't need to do anything. If you did want to start a build, increment the Version: field and try again.

bioc-issue-bot commented 8 years ago

We only start builds when the Version field in the DESCRIPTION file is incremented. For example, by changing

Version: 0.99.0

to

Version 0.99.1

If you did not intend to start a build, you don't need to do anything. If you did want to start a build, increment the Version: field and try again.

bioc-issue-bot commented 8 years ago

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

40ab860 S4Vectors, IRanges, MultiAssayExperiment, new vig...

bioc-issue-bot commented 8 years ago

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

0ddffbe incremented version again to start a build

bioc-issue-bot commented 8 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/GOpro_buildreport_20160929130906.html

lshep commented 8 years ago

Thank you for your updates. A few more minor changes that should be addressed:

VIGNETTE It would be nice if the plots are displayed

UNIT TESTS It is a NOTE in the build report but we do strongly encourage adding unit tests to the package. Some simple suggestions: test your input data object you provide that it has your restrictions of being >=3 cohorts and that all the rownames are equivalent.

R/CODE remove "::" they shouldn't be needed if declared correctly in DESCRIPTION/NAMESPACE findAllGOs.R there is an AnnotationDbi:: , you import(annotationDbi) in NAMESPACE so "::" not necessary RcppExports.R there is Rcpp::compileArrtributes , please add this as an importFrom in your NAMESPACE and remove "::"

This is very close to acceptance with these minor changes. Thank you Lori

bioc-issue-bot commented 8 years ago

We only start builds when the Version field in the DESCRIPTION file is incremented. For example, by changing

Version: 0.99.0

to

Version 0.99.1

If you did not intend to start a build, you don't need to do anything. If you did want to start a build, increment the Version: field and try again.

bioc-issue-bot commented 8 years ago

We only start builds when the Version field in the DESCRIPTION file is incremented. For example, by changing

Version: 0.99.0

to

Version 0.99.1

If you did not intend to start a build, you don't need to do anything. If you did want to start a build, increment the Version: field and try again.

bioc-issue-bot commented 8 years ago

We only start builds when the Version field in the DESCRIPTION file is incremented. For example, by changing

Version: 0.99.0

to

Version 0.99.1

If you did not intend to start a build, you don't need to do anything. If you did want to start a build, increment the Version: field and try again.

bioc-issue-bot commented 8 years ago

We only start builds when the Version field in the DESCRIPTION file is incremented. For example, by changing

Version: 0.99.0

to

Version 0.99.1

If you did not intend to start a build, you don't need to do anything. If you did want to start a build, increment the Version: field and try again.

bioc-issue-bot commented 7 years ago

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

1f8c8ac unit tests

bioc-issue-bot commented 7 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/GOpro_buildreport_20161003105539.html

lshep commented 7 years ago

Thank you for making these changes. The package looks okay now and I will recommend be accepted. You should get svn instructions via email in the next couple of days