Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

singleCellTK #699

Closed dfjenkins3 closed 6 years ago

dfjenkins3 commented 6 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 6 years ago

Hi @dfjenkins3

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: singleCellTK
Type: Package
Title: Interactive Analysis of Single Cell RNA-Seq Data
Version: 0.99.0
Author: David Jenkins
Authors@R: c(person(given="David", family="Jenkins", email="dfj@bu.edu", role=c("aut","cre")),
   person(given=c("Tyler"), family="Faits", email="tfaits@bu.edu", role=c("aut")),
   person(given=c("Emma"), family="Briars", email="tfaits@bu.edu", role=c("aut")),
   person(given=c("Sebastian"), family="Carrasco Pro", email="abild@coh.org", role=c("aut")),
   person(given=c("W.", "Evan"), family="Johnson", email="wej@bu.edu", role=c("aut")))
Maintainer: David Jenkins <dfj@bu.edu>
Depends:
    R (>= 3.5),
    SummarizedExperiment,
    SingleCellExperiment,
    DelayedArray,
    Biobase
Description: Run common single cell analysis directly through your browser
    including differential expression, downsampling analysis, and clustering.
License: MIT + file LICENSE
biocViews: SingleCell, GeneExpression, DifferentialExpression, Alignment,
    Clustering
LazyData: TRUE
Imports:
    ape,
    colourpicker,
    cluster,
    ComplexHeatmap,
    data.table,
    DESeq2,
    DT,
    ggplot2,
    ggtree,
    gridExtra,
    GSVA (>= 1.26.0),
    GSVAdata,
    limma,
    MAST,
    matrixStats,
    methods,
    multtest,
    plotly,
    RColorBrewer,
    Rtsne,
    S4Vectors,
    shiny,
    shinyjs,
    sva,
    reshape2,
    AnnotationDbi,
    shinyalert,
    circlize
RoxygenNote: 6.0.1
Suggests:
    testthat,
    Rsubread,
    BiocStyle,
    knitr,
    bladderbatch,
    rmarkdown,
    org.Mm.eg.db,
    org.Hs.eg.db
VignetteBuilder: knitr

Consider adding SSH keys to your GitHub account. SSH keys will are used to control access to accepted Bioconductor packages. See these instructions to add SSH keys to your GitHub account.

bioc-issue-bot commented 6 years ago

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.

bioc-issue-bot commented 6 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 build report for more details.

dfjenkins3 commented 6 years ago

Hi @vobencha,

The ERROR I'm getting on the Windows build is because the package suggests "Rsubread" which isn't available for windows. on the report it says:

Checking can be attempted without them by setting the environment
variable _R_CHECK_FORCE_SUGGESTS_ to a false value.

How should I move forward with that?

Thanks,

David

lawremi commented 6 years ago

Btw, you might consider moving your website to a more conventional TLD. The .science TLD is ~99% "shady" (see https://www.symantec.com/connect/blogs/shady-tld-research-gdn-and-our-2016-wrap). Thus, my company's firewall blocks access to all .science hosts.

bioc-issue-bot commented 6 years ago

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

cdb0ae2 switch to github hosted docs a5416e0 version bump

dfjenkins3 commented 6 years ago

@lawremi thanks for letting me know, that explains why it was so cheap! I changed the links to the github hosted docs.

bioc-issue-bot commented 6 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 build report for more details.

vobencha commented 6 years ago

Hi, Great job with the vignettes and package in general. Really nice.

1) man pages I would recommend combining these by family or topic area. It is much easier for the user to discover related functions if they are together on a single page.

2) function names You've got a mixture of camel case and underscores in the exported functions. Please pick one style (where possible) and change the names of the R/ code files as well as the man/ files.

3) Windows error I'll double check with Herve but I think our only option is to include a singleCellTK/.BBSoptions file that marks the package as unsupported on Windows:

UnsupportedPlatforms: win

Valerie

vobencha commented 6 years ago

Herve reminded me we can pass arguments such as _R_CHECK_FORCE_SUGGESTS_ through BBSoptions. The solution is to add a .BBSoptions file to your package with this line

CHECKprepend.win: set _R_CHECK_FORCE_SUGGESTS_=0&&

There should be no spaces between 0 and &&.

Valerie

dfjenkins3 commented 6 years ago

Hi Valerie,

Thanks. We only use Rsubread for a single function that aligns data and creates a SingleCellExperiment object directly, and the function checks for Rsubread before running (https://github.com/compbiomed/singleCellTK/blob/master/R/alignSingleCellData.R#L49-L52). All the other functions and the shiny app work well on windows.

I will clarify this in the error message for this function and add the .BBSoptions file.

I am working on your other two comments today, thanks for the feedback.

Thanks,

David

bioc-issue-bot commented 6 years ago

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

1c7ea90 support for loading the single cell datasets conta... c32ed94 Consistent use of camel case throughout the app

bioc-issue-bot commented 6 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 build report for more details.

bioc-issue-bot commented 6 years ago

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

94771a5 Reduced image sizes and removed datatables from vi... efab588 Version bump

bioc-issue-bot commented 6 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 build report for more details.

dfjenkins3 commented 6 years ago

Hi @vobencha,

Here are my responses to your comments:

  1. man pages: I would recommend combining these by family or topic area. It is much easier for the user to discover related functions if they are together on a single page.

We have the functions categorized by topic area on our pkgdown site. Is there a way to do this in the reference manual or somewhere else?

  1. function names: You've got a mixture of camel case and underscores in the exported functions. Please pick one style (where possible) and change the names of the R/ code files as well as the man/ files.

All functions in the package now consistently use camelCase.

  1. Windows error: Herve reminded me we can pass arguments such as _R_CHECK_FORCE_SUGGESTS_ through BBSoptions. The solution is to add a .BBSoptions file to your package with this line

I added a .BBSoptions file with the suggested formatting but the check function still shows an error. Did I use the right formatting?

Additionally, I'm now getting warnings about the size of the package. It looks like after building, all the compiled vignettes in inst/doc are > 1MB, possibly because of the screenshots I am using. I reduced the resolution of the screen shots but the files are still large.

Thanks,

David

vobencha commented 6 years ago

1) See 'Documenting multiple functions in the same file': http://r-pkgs.had.co.nz/man.html#multiple-man 2) Thanks. 3) Your .BBSoptions file is correct. @lshep , does the SPB respect the CHECKprepend options? @dfjenkins3 it's possible we have this implemented on the build system but not yet on the SPB.

lshep commented 6 years ago

The SPB has not yet implemented the CHECKprepend option, only the unsupported platforms. I can look at implementing in the near future.

vobencha commented 6 years ago

No problem @lshep , thanks for letting us know. @dfjenkins3 you can ignore the Windows error.

bioc-issue-bot commented 6 years ago

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

0be6f65 Consistent use of inSCE to refer to the input SCtk...

bioc-issue-bot commented 6 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 build report for more details.

vobencha commented 6 years ago

The package is too big by software size standards. Can you trim some data out? See the WARNING in the SPB output:

WARNING: Product from R CMD build exceeds 4 MB requirement:12.0336990356 MB.

dfjenkins3 commented 6 years ago

Hi @vobencha,

The only data in the package is a 291 KB 30 cell subset example https://github.com/compbiomed/singleCellTK/tree/master/data. We load the other example data sets from other packages. It looks like the real culprit for the large size is the html versions of the vignettes in inst/doc:

* checking installed package size ... NOTE
  installed size is 18.2Mb
  sub-directories of 1Mb or more:
    data   1.0Mb
    doc   16.6Mb

How can I make the vignettes smaller? I tried lowering the resolution of the screenshots from the shiny app but the vignettes still seem to be very large.

Thanks,

David

vobencha commented 6 years ago

There are quite a few .png files - just these alone almost bump into the 4MB limit without building.

~/repos/github/packageReview/singleCellTK/vignettes >du -hs img/
3.8M    img/

I myself don't have a recommendation for making the html smaller. I can ask others if they know of anything.

I might as well ask, do you really need all of the .png files? Would it be possible to have just a couple of summary graphics vs 10?

~/repos/github/packageReview/singleCellTK/vignettes/img >du -h *
112K    batch_01.png
36K batch_02.png
92K batch_03.png
236K    diffex_01.png
240K    diffex_02.png
60K diffex_03.png
196K    diffex_04.png
496K    diffex_05.png
576K    diffex_06.png
92K dr_01.png
108K    dr_02.png
100K    dr_03.png
308K    pathway_01.png
36K samplesize_01.png
32K samplesize_02.png
52K samplesize_03.png
128K    summary_01.png
68K summary_02.png
116K    summary_03.png
92K summary_04.png
44K summary_05.png
80K summary_06.png
52K summary_07.png
48K summary_08.png
24K summary_09.png
60K summary_10.png
104K    summary_11.png
148K    summary_12.png
108K    upload_01.png
bioc-issue-bot commented 6 years ago

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

55f37d5 Do not use floating TOC. Specify VignetteIndexEntr...

bioc-issue-bot commented 6 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 build report for more details.

bioc-issue-bot commented 6 years ago

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

1912a29 Remove pngs from repo, load them from online host

bioc-issue-bot commented 6 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 build report for more details.

bioc-issue-bot commented 6 years ago

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

e469318 Removed shiny app page specific vignettes b8a2d32 Version bump for vignette changes

bioc-issue-bot commented 6 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 build report for more details.

dfjenkins3 commented 6 years ago

I think the screenshots add context when I'm talking about available dropdowns, buttons, etc so I'd like to keep them in if possible.

I have 10 vignettes in the package describing different features. Even v01.. which contains no screenshots or images is 1MB in html form:

1.0M    v01-Introduction_to_singleCellTK.html
4.2M    v02-Processing_and_Visualizing_Data_in_the_SingleCellTK.html
1.2M    v03-tab01_Upload.html
2.2M    v04-tab02_Data-Summary-and-Filtering.html
1.4M    v05-tab03_Dimensionality-Reduction-and-Clustering.html
1.3M    v06-tab04_Batch-Correction.html
3.4M    v07-tab05_Differential-Expression.html
1.4M    v08-tab06_Pathway-Activity-Analysis.html
1.2M    v09-tab07_Sample-Size.html
1.0M    v10-Aligning_and_Quantifying_scRNA-Seq_Data.html

I was using the floating table of contents in each vignette so if I remove that it reduces the size of each vignette:

737K    v01-Introduction_to_singleCellTK.html
3.9M    v02-Processing_and_Visualizing_Data_in_the_SingleCellTK.html
874K    v03-tab01_Upload.html
1.9M    v04-tab02_Data-Summary-and-Filtering.html
1.1M    v05-tab03_Dimensionality-Reduction-and-Clustering.html
1.0M    v06-tab04_Batch-Correction.html
3.1M    v07-tab05_Differential-Expression.html
1.1M    v08-tab06_Pathway-Activity-Analysis.html
883K    v09-tab07_Sample-Size.html
734K    v10-Aligning_and_Quantifying_scRNA-Seq_Data.html

I then removed the img directory and loaded the images from a separate host which reduced the build size to ~8.44MB

I did a quick test. If all my vignettes were the size of v01 with no images the tar.gz from build is still ~6.3MB when I build it on my computer :(. The only thing I could think to do was to remove the shiny app page specific vignettes from the bioconductor source. We will continue to host these on our documentaton page and link to them in the intro vignette and in the shiny app.

vobencha commented 6 years ago

OK, that sounds fine. Thanks for finding a solution. Package is approved.

bioc-issue-bot commented 6 years ago

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!

vobencha commented 6 years ago

@mtmorgan Windows is throwing an error because the SPB has not yet implemented the .BBSoptions prepend checks. This should clear up when the package builds with the BBS code.

mtmorgan commented 6 years ago

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/dfjenkins3.keys is not empty), then no further steps are required. Otherwise, do the following:

  1. Add an SSH key to your github account
  2. Submit your SSH key to Bioconductor

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(\"singleCellTK\"). The package 'landing page' will be created at

https://bioconductor.org/packages/singleCellTK

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.