Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

schex #1253

Closed SaskiaFreytag closed 4 years ago

SaskiaFreytag commented 4 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 4 years ago

Hi @SaskiaFreytag

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: schex
Type: Package
Title: Hexbin plots for single cell omics data
Version: 0.0.1
Author: Saskia Freytag
Maintainer: Saskia Freytag <saskia.freytag@perkins.uwa.edu.au>
Description: Builds hexbin plots for variables and dimension reduction stored
    in single cell omics data such as SingleCellExperiment and SeuratObject. The
    ideas used in this package are based on the excellence work of Dan Carr,
    Nicholas Lewin-Koh, Martin Maechler and Thomas Lumley.
Depends: SingleCellExperiment (>= 1.7.4),
    Seurat,
    ggplot2
Imports: hexbin,
        stats,
  methods,
  cluster,
  dplyr,
  entropy
Suggests: 
    ggrepel,
    knitr,
    rmarkdown,
    testthat (>= 2.1.0),
    covr
URL: https://github.com/SaskiaFreytag/schex
BugReports: https://github.com/SaskiaFreytag/schex/issues
License: GPL-3
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.1.1
biocViews: Software, Sequencing,
      SingleCell, DimensionReduction, Visualization
VignetteBuilder: knitr
lshep commented 4 years ago

You will receive a build report shortly. Please set up your webhook so that future version bumps on the package will trigger an automatic rebuild.

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

lshep commented 4 years ago

Please be advised Bioconductor only allows packages dependencies that are on CRAN or Bioconductor. SeuratData does not appear to be on either. This package will need to be submitted or not be used in your package.

bioc-issue-bot commented 4 years ago

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

8b989d1 version bump

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

bioc-issue-bot commented 4 years ago

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

2fcf900 add .Rprojec to gitignore

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

bioc-issue-bot commented 4 years ago

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

7ce478b remove rproj be3eeed bumped version

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

lshep commented 4 years ago

Thank you for your submission. I really enjoyed the plotting aspects of your package. Please see some comments below:

Build reports

DESCRIPTION

README

NEWS

vignettes

_usingschex

_Seuratschex

_shinyschhex

R / man

schex.R

_utilityfunctions.R

_plot_hexbinmeta.R

_plot_hexbin_interact.R/plot_hexbin_gene.R/plot_hexbin_feature.R/plot_hexbin_density.R/make_hexbinlabel.R

General comment

Please address the above issues and comment back here with a summary of changes or comments to questions. When ready for a re-review please also do a version bump to trigger a new build of the package. Cheers!

SaskiaFreytag commented 4 years ago

Thank your for your kind review. I have tried to address your points as I will outline below.

Build reports

Fix notes for global variables/functions

I have fixed global variables/functions.

Fix coding practice notes

I have fixed most coding practice notes.

DESCRIPTION

Packages used in your vignette I believe should be in the Suggest field of your DESCRIPTION (which will also clean up the build report note)

I have added all packages required in the vignette to the Suggest field.

We would recommend removing lazyData: true. We have rarely found it useful and have found it can slow installation.

Done.

README

Are both the .Rmd and .md really necessary?

Yes they are both necessary, as I modify the .Rmd file to obtain the .md file. This is the workflow suggested by the usethis package.

Please include Bioc installation instructions of the package too.

Done.

NEWS

Please see group naming convention in ?news. Headers should be formatted changes in version x.y.z and not include the package name.

Done

vignettes

_usingschex

For this demo there is no need to download the Github repo SeuratData as this is demonstrating SingleCellExperiment.

I have removed mention of this package. Thank you for pointing out this mistake.

Is there a random component somewhere? When I run through the code when I get to plot_hexbin_density I get a slightly different plot? Similar but different

Yes there is a random component in the UMAP plotting. I have used set.seed to fix this issue.

_shinyschex

Is the double h in the name a typo?

Yes I have fixed this. Thank you for pointing out this error.

If iSEE and shinydashboard are utilized in this vignette, please include as suggest in DESCRIPTION.

Done.

I don't know if its a limitation in the shiny code or somewhere in your code but when I run the app to get a shinyApp, if I close the window it doesn't close the connection on the code terminal (I have to cntr-C to get back to a working R terminal)

In my experience this is normal shiny behaviour.

It would be nice to show the iSEE plot with some interactive features. The plot that shows up seems to be pretty static with no options to switch

I have added explanations to the vignette to explain how the plot can be changed in an interactive manner. However, please note that the interactive feature for iSEE for schex plots is very basic. I have had extensive discussions with the maintainers around this topic, but for now there is no better way in facilitating interactivity through iSEE in this instance. I still believe that the inclusion of iSEE in the vignette is useful, as iSEE is a widely used tool and schex plots may want to be integrated by users for completion sake.

R/man

schex.R

Maybe the package help page could use a little more detail and/or link to how to get started using the package?

I have added more detail.

_utilityfunctions.R

should this be a scaler || or the vector option used?

I am not sure what this comment refers to.

remove the excess paste0 in the stop function.

Done and also taken care of in all other functions.

further functions

The setMethod plot_hexbin_meta for SingleCellExperiment and Seurat object is essentially the same code except for within the first four lines of code on how to access the cID, out, and colnames of the two objects. Please eliminate duplicate code by have these tow setMethods extract that specific data and pass to a function to do the rest of the analysis. Code duplication should always be avoid to make code easier to read and maintain. We would recommend applying this to all methods defined for the two objects calling a common function when the code starts to duplicate.

I have followed this advice for all functions to the best of my abilities.

General comment

Is it worth having a save option for any of these plots or are you intending them to be strictly interactive/investigative?

I have added explanations in the vignettes to the connection between schex and ggplot2. Within these explanations I show how the plot can be easily saved using standard ggplot2 grammar.

bioc-issue-bot commented 4 years ago

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

1571984 hexsticker 2847fd3 included bioc-release c83921a included fix for travis 9336b8f added uwot package 88e86a3 fix travis ac9fbd1 fix mode 612a631 fix mode really 513c46a make mode function hidden 37684fe fix travis again 02bcbad build 7fe757a use jef works travis d8e1cfc add example to readme 86c97bf more tests bbf13a2 test cf5241a so many more tests 08fa0cf more tests again ecbeeb9 tests for sce 1f9f39e new vignette and fix genes 266195d delete html d7ac2f1 animation 4d12282 Merge branch 'master' of https://github.com/Saskia... a5438f7 add installation b35a606 Seurat vignette 103cacf enables plotting data from default assay; fixes da... e3b468e Merge pull request #3 from joseah/master enables ... 89ecdbb query dimensions 6763a66 remove tidyr, change make_hexbin_lables 1343f4a Fix reducedDims accessor for SingleCellExperiment ... 96fa09b Update make_hexbin.R 4b9e03c Merge pull request #6 from robertamezquita/patch-1... 8813afb make new function for proteins; Seurat only 0a54396 new interact function 849a9af documentation 26fbd86 vignette fixed ec48178 chnages required for extended version 7be184c change travis and put hexsticker in right spot 3ae0730 change travis again baef1c8 cleaned out cache 4c309e6 explain development version needed 75f3618 benchmarking and interaction d8638ac multi-modal guide 688af27 fix digit at gene start bug 9e6be25 moved misc folder out 23dbbd3 change file paths 34a59e5 delete line cce657c adhere to BiocCheck 090b5c8 remove .git from build d29ab1c better removing of .git from build 76ee183 replace SeuratData with TENxPBMCData aed5aa3 new namespace ba43122 version bump 9ee978b add .Rprojec to gitignore 4cb64ee remove rproj fada014 bumped version de93724 fix mistakes related to vignettes dbbacb2 reduce duplicated code 5e3ac08 Change code to remove duplicated code, vignettes i... 1318354 git cache cleared 2fefc3c incremented version, tried fixing indents 0b14da1 Resolved merge conflicts

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

bioc-issue-bot commented 4 years ago

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

d6d9d72 Further resolved merge conflicts

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

bioc-issue-bot commented 4 years ago

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

129e6f3 version bump

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

lshep commented 4 years ago

Thank you. Looks really good and ready to accept. Could you fix the last two NOTES from the build report



Thanks for making all the other changes. 
bioc-issue-bot commented 4 years ago

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

b8c78bd Resolved merge conflicts 3bc7b8e Further resolved merge conflicts 930c2b1 updated link to Seurat-class 0a5d470 version bump f81936d substitute sapply with vapply df97cfb resolved merge conflicts

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

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

mtmorgan commented 4 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/SaskiaFreytag.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 BiocManager::install("schex"). The package 'landing page' will be created at

https://bioconductor.org/packages/schex

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.