Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

flowTime #185

Closed wrightrc closed 7 years ago

wrightrc commented 7 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 7 years ago

Hi @wrightrc

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: flowTime
Title: Annotation and analysis of biological dynamic systems by flow cytometry
    data
Version: 0.99.0
Authors@R: c(person("R. Clay", "Wright", email = "wright.clay@gmail.com", role = c("aut", "cre")), person("Nick", "Bolten", email = "nbolten@gmail.com", role = c("aut")),person("Edith", "Pierre-Jerome", email = "", role = c("aut")))
Description: This package was developed for analysis of both dynamic and steady state experiments
    examining the function of gene regulatory networks in yeast (strain W303)
    expressing fluorescent reporter proteins using a BD Accuri C6 and SORP cytometers. However, the
    functions are for the most part general and may be
    adapted for analysis of other organisms using other flow cytometers. Functions in this
    package facilitate the annotation of flow cytometry data with experimental metadata, as
    is requisite for dissemination and general ease-of-use. Functions for creating, saving
    and loading gate sets are also included.
    In the past, we have typically generated summary statistics for
    each flowset for each timepoint and then annotated and analyzed these
    summary statistics. This method loses a great deal of the power that comes
    from the large amounts of individual cell data generated in flow cytometry,
    by essentially collapsing this data into a bulk measurement after subsetting.
    In addition to these summary functions, this package also contains
    functions to facilitate annotation and analysis of steady-state or time-
    lapse data utilizing all of the data collected from the thousands of individual
    cells in each sample.
Depends:
    R (>= 2.14.0),
    flowCore,
    plyr,
    dplyr
License: Artistic-2.0 + file LICENSE
LazyData: true
biocViews:
    FlowCytometry,
    TimeCourse,
    Visualization,
    DataImport,
    CellBasedAssays
Suggests:
    knitr,
    rmarkdown,
    flowViz,
    ggplot2,
    BiocGenerics,
    moments
VignetteBuilder: knitr
RoxygenNote: 5.0.1
bioc-issue-bot commented 7 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 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.

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/flowTime_buildreport_20161105103704.html

bioc-issue-bot commented 7 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:

4dd23bb build increment version to build

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.

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

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

bioc-issue-bot commented 7 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:

4e5cbce update version

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.

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

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

vobencha commented 7 years ago

Hi, Please address these issues raised by the Single Package Builder:

Once these are taken care of I'll do a full review. Valerie

bioc-issue-bot commented 7 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

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:

772392e build

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.

On one or more platforms, the build results were: "WARNINGS, 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/flowTime_buildreport_20161228182728.html

bioc-issue-bot commented 7 years ago

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

fb963de fixing linux issue removed data.R

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.

On one or more platforms, the build results were: "skipped, 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 following build report for more details:

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

bioc-issue-bot commented 7 years ago

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

8a2d104 fixing linux issue capitalization possibly

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/flowTime_buildreport_20161228191609.html

wrightrc commented 7 years ago

Hi Valerie,

I have addressed the issues mentioned above and made a few other updates. The package is ready for review when you get a chance.

Thanks, Clay

vobencha commented 7 years ago

Hi,

Nice job with flowTime. A few comments / suggestions below.

1) move .RData to data/ directory .RData files are usually stored in the data/ directory; they need man pages and loading them is done with data("C6Gates.RData"). I like that you have a function that lists the gates but loading should be through the standard mechanism. Having the files in data/ makes them visible via a call to data() when the package is loaded.

I would suggest keeping listGates but have a man page for each data set:
> listGates()
[1] "C6Gates.RData"      "defaultGates.RData" "SORPGates.RData"  
> ?C6Gates
No documentation for ‘C6Gates’ in specified packages and libraries:
you could try ‘??C6Gates’

Each man page should describe these objects.

> load("C6Gates.RData")
> objects()
[1] "dipdoubletGate" "dipsingletGate" "hapdoubletGate" "hapsingletGate"
[5] "yeastGate"    

Aren't there any descriptive statistics for the polygonGate? If yes, please include them on the man page.

> class(yeastGate)
[1] "polygonGate"
attr(,"package")
[1] "flowCore"
> dim(yeastGate)
Loading required package: flowCore
NULL
> length(yeastGate)
[1] 1
> names(yeastGate)
NULL

2) function names get_time() and qa.gating() are not named consistent with the other functions. It looks like you've gone with camelCase so please change those to match.

~/flowTime/man >ls
addbs.Rd             dipdoubletGate.Rd  get_time.Rd        loadGates.Rd  saveGates.Rd      yeastGate.Rd
addnorm.Rd           dipsingletGate.Rd  hapdoubletGate.Rd  ploidy.Rd     setGates.Rd
annotateFlowSet.Rd   flsummary.Rd       hapsingletGate.Rd  polygate.Rd   steadyState.Rd
createAnnotation.Rd  gateEnv.Rd         listGates.Rd       qa.gating.Rd  summarizeFlow.Rd

Valerie

wrightrc commented 7 years ago

Thank you!

Can the user make changes to the data/ directory? I included these .RData gate sets in extdata/ such that users can also add their own gate sets to this directory using saveGates. Also the user should be able to change the defaultGates to C6Gates, SORPGates, or their own gate set. Perhaps, it is best to deal with setting defaults in extdata/ (as is done currently) but pull the included gate sets from data/ as you recommend.

Now, I am also thinking it is best to separate "User" gates from "Included" gates such that package updates will not overwrite "User" gates. Upon saving a gate set (in a user-defined directory), saveGates could then prompt the user to add them to a branch of the flowTime github repository for inclusion in the next release, for ease of use and reproducibility. This would also fit the conventions and documentation requirements you mentioned.

What are your thoughts on this approach?

Thanks, Clay

vobencha commented 7 years ago

I would recommend you have non-changeable data in data/. Users should be responsible for managing their own gates and they should not be able to add these gates to the flowTime package (no access to github repo). The functions in flowTime should have a 'gatePath' input argument that by default uses the gates in data/. If the user wants to use their own gate they can provide their own path.

Valerie

vobencha commented 7 years ago

Are you planning to submit another version or should I close this issue?

Valerie

wrightrc commented 7 years ago

Yes, I will submit another version. Sorry for the delay. I will have an updated version including your suggestions by March 17.

bioc-issue-bot commented 7 years ago

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

a6f324a version update for build

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.

On one or more platforms, the build results were: "skipped, 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 following build report for more details:

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

bioc-issue-bot commented 7 years ago

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

cb503b0 fix capitalization/linux issue 0a364e6 Merge branch 'master' of https://github.com/wright...

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/flowTime_buildreport_20170315164955.html

vobencha commented 7 years ago

Thanks for making the changes. The man pages for datasets in data/ should show how to load the data with

data(hapdoubletGate)

etc.

The built tarball must be <= 4MB on disk, see http://www.bioconductor.org/developers/package-guidelines/#correctness. You'll need to reduce the size of the datasets and/or use a more efficient compression method. The current size is 24 MB:

~/proj/git >du -h flowTime_0.99.6.tar.gz 24M flowTime_0.99.6.tar.gz

Valerie

bioc-issue-bot commented 7 years ago

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

6205f62 version bump

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.

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

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

wrightrc commented 7 years ago

Ok, I've trimmed down the example data and updated the data/ man pages. The error now is from oaxaca, which I believe is soon to be decommissioned, correct?

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/flowTime_buildreport_20170321080156.html

vobencha commented 7 years ago

OK, looks good. Thanks for making the changes. I'm marking the package as accepted - you should get an email in the next week with svn credentials and instructions for next steps. Valerie