Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

cytomapper #1441

Closed nilseling closed 4 years ago

nilseling 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 @nilseling

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: cytomapper
Version: 0.99.0
Date: 2020-03-30
Title: Visualization of highly multiplexed imaging cytometry data in R
Description: 
    Highly multiplexed imaging cytometry acquires the single-cell expression of
    selected proteins in a spatially-resolved fashion. These measurements can be
    visualized across multiple length-scales. First, pixel-level intensities
    represent the spatial distributions of feature expression with highest
    resolution. Second, after segmentation, expression values or cell-level
    metadata (e.g. cell-type information) can be visualized on segmented cell
    areas. This package contains functions for the visualization of multiplexed
    read-outs and cell-level information obtained by multiplexed imaging
    cytometry. The main functions of this package allow 1. the visualization of
    pixel-level information across multiple channels and 2. the display of
    cell-level information (expression and/or metadata) on segmentation masks.
Authors@R: 
    c(person("Nils", "Eling", role = c("aut", "cre"), 
      email = "nils.eling@dqbm.uzh.ch",
      comment = c(ORCID = "0000-0002-4711-1176")),
      person("Nicolas", "Damond", role = c("aut"), 
      email = "nicolas.damond@dqbm.uzh.ch", 
      comment = c(ORCID = "0000-0003-3027-8989")))
License: GPL (>= 2)
Depends:
    R (>= 4.0),
    EBImage,
    SingleCellExperiment,
    methods
Imports:
    S4Vectors,
    RColorBrewer,
    viridis,
    utils,
    SummarizedExperiment,
    tools,
    graphics,
    raster,
    grDevices,
    stats
Suggests:
    BiocStyle,
    knitr,
    rmarkdown,
    testthat
biocViews: ImmunoOncology, Software, SingleCell, OneChannel, TwoChannel, MultipleComparison, Normalization, DataImport
VignetteBuilder: knitr
URL: https://github.com/BodenmillerGroup/cytomapper
BugReports: https://github.com/BodenmillerGroup/cytomapper/issues
RoxygenNote: 7.1.0
Encoding: UTF-8

Add 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 4 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.

mtmorgan commented 4 years ago

Please make sure to remove .Rhistory files from your git repository.

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, 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:

89d95a8 New version bump on master

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:

e483dea Changed R dependency 3d36263 Fixed linking to functions edb658a Added link to EBImage::normalize to seealso sectio... 580e3d8 Removed generic plotting function 00db5c5 Merge branch 'devel' c9e0064 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: "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:

58971c6 Removed plot import from graphics 8a78d56 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: "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.

nilseling commented 4 years ago

Hi @lshep,

I can't quite figure out where the last warning on the Windows server is coming from. When searching for help online, most posts recommend re-installing the package dependencies. Would you know anything about this warning and could you point me in the right direction to fix this? Thanks for your input! Best,

Nils

lshep commented 4 years ago

I would ignore this Warning for now. Bioconductor packages will be reinstalled on there own. In makes me think there might be conflicting plot methods (maybe not necessarily from your package could also be from dependencies) --

nilseling commented 4 years ago

Yeah, I thought the same and removed the exported plot method from my package. But that didn't seem to fix the problem.

lshep commented 4 years ago

so ignore for now.

lshep commented 4 years ago

Thank you for your submission. Please see initial review below:

README

Description

inst/extadata

vigenttes

I'm coming from a naive user perspecetive since I don't readily work with this type of data

R code / man pages

Everything looks well written.

Please respond to the few comments above and the few minor adjustments. When ready please do a version bump and respond back here with a summary of changes and any responses to the inquires above.

Cheers. Looking forward to getting this into Bioconductor.

bioc-issue-bot commented 4 years ago

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

80ae6f7 Removed channelNames generic for Image class 4553897 Changed defaults for scale bar d8505b0 Revert back R version ccbed5f Aesthetic adjustment to scale bar b97d170 Aesthetic changes to image title 77b35c2 Aesthetic changes to legend titles bdaee09 pass tests 4655e74 Start on making normalize function more eficient b3b728c Fixed bug when saving individual files 08a8eb0 Changed normalize function d38db32 Fixed subsetting bug 52679ff Removed subset_images parameter 6d3fe0d Added unit tests for .colourMaskByMeta 5fdd414 Added .colourMaskByFeature unit tests ad365cf Added .colourImageByFeature unit test 564ca7e Unit tests for .outlineImageByMeta 0943cbc Unit test for .selectColours 3b3eb5b Tests for colour mixing d3c734a Added logical to pancreasSCE 018af4d logical and factor bug fix 6714da4 Fixed additive colour mixing problem de415b1 First fixes to address reviews ecf8dd3 Finished unit tests on helper functions a8a61ec Added scripts to generate the example datasets - ... 7365d63 Unit tests for plotting-param validity 269b9de Changed channel and image names The channel and i... 8fc8053 Updated the toy dataset and documentation - Updat... a8a048b Merge branch 'devel' of https://github.com/Bodenmi... 4c828d7 Resolved conflicts + updated NEWS 5b647e2 Added better toy data and explanation 5159240 Unit tests on validity functions 12cc547 Update scripts with Mendeley Data links Scripts i... f3df73d Merge branch 'devel' of https://github.com/Bodenmi... b36ff89 Revert "Merge branch 'devel' of https://github.com... 7954a4b Revert "Revert "Merge branch 'devel' of https://gi... 7afa369 Revert "Revert "Revert "Merge branch 'devel' of ht... bf967ab reverted changes on validity checks 7b7321f Removed temp solution 4eeb2e2 Fixes to pass BiocCheck 03f6dee 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.

nilseling commented 4 years ago

Hi @lshep,

thanks a lot for your comments! We have now worked on the package/docs and submitted the reviews. In detail, we have addressed the individual points as described below:

Thank you for your submission. Please see initial review below:

README

  • [ ] Include Bioconductor installation instructions

We have added this to the README.

Description

  • [ ] Please remove date field. This should be generated automatically so it doesn't get out-dated

inst/extadata

  • [ ] Please include an inst/scripts directory that has a file describing the ata in inst/extdata. Ideally code but could be sudo code or text description but should include relevant source/reference information and any sort of processing or subsetting

We have now added .Rmd files to inst/scripts that download the data and fully process them to form the toy dataset. Furthermore, we have also added a README.md to inst/scripts that give further instructions on the data used. This data is published and we reference the correct publication and external processing scripts that are needed to process raw data into files that can be used with cytomapper.

vigenttes

I'm coming from a naive user perspecetive since I don't readily work with this type of data

  • [ ] Just for my own clarity. The image files, they come directly from some instrumentation?

The toy dataset used in the package comes from the Fluidigm Hyperion system. Unfortunately, the system does not return files that can be directly read into R. In the vignette, we link to standard pre-processing scripts that return the data in a format that can directly be used with cytomapper. We have now added further explanations to the "The provided toy dataset" section to indicate how raw data is commonly pre-processed.

  • [ ] I guess I'm still slightly confused at the jump from 4.1 to 4.2. If I assume the .tiff files come directly from instrumentation, but you already have the SingleCellExperiment object created? Does this come from the same instrumentation or is this a different step? There is no indication of how the SCE object was created so I'm not exactly getting the connection. Maybe someone in this field and working with this data would already understand this?

As mentioned above, pre-processing is unfortunately needed to generate data formats that can be directly used with cytomapper. However, the package reads in the standard output of the pre-processing pipeline. As you can see from the inst/scripts/1_LoadPancreasData.Rmd file, it is a bit tricky to read all files into a SingleCellExperiment object. Nevertheless, generating the SingleCellExperiment object from the pre-processed data is feasible when following the OSCA workflow (as mentioned in the vignette). My plan for the fall Bioconductor release is to write an extensive workflow how to process and analysis this sort of data using Bioconductor.

  • [ ] So is the object data(pancreasMasks) created from
path.to.images <- system.file("extdata", package = "cytomapper")
all_masks <- loadImages(path.to.images, pattern = "_mask.tiff")
all_masks

In general, yes. The only step missing here is to scale the masks due to a scaling issue when pre-processing data using CellProfiler:

all_masks <- scaleImages(all_masks, value = (2 ^ 16) - 1)

This last step will scale the pixel-values in the masks to match the actual object IDs generated during segmentation.

R code / man pages

Everything looks well written.

Please respond to the few comments above and the few minor adjustments. When ready please do a version bump and respond back here with a summary of changes and any responses to the inquires above.

Cheers. Looking forward to getting this into Bioconductor.

Thanks again for the comments! It helped us to organise the processing of the toy data, which we will also use for an associated publication. And please let me know if we should add other things or how to proceed. Best,

Nils

lshep commented 4 years ago

Thank you. Everything else looks good to me and the added inst/scripts and sections to the vignette make the connections more clear. Welcome to bioconductor.

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

https://bioconductor.org/packages/cytomapper

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.