InSituType

[davidpross]

Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor

• Repository: https://github.com/Nanostring-Biostats/InSituType

Confirm the following by editing each check box to '[x]'

• [x] I understand that by submitting my package to Bioconductor, the package source and all review commentary are visible to the general public.

• [x] I have read the Bioconductor Package Submission instructions. My package is consistent with the Bioconductor Package Guidelines.

• [x] I understand Bioconductor Package Naming Policy and acknowledge Bioconductor may retain use of package name.

• [x] I understand that a minimum requirement for package acceptance is to pass R CMD check and R CMD BiocCheck with no ERROR or WARNINGS. Passing these checks does not result in automatic acceptance. The package will then undergo a formal review and recommendations for acceptance regarding other Bioconductor standards will be addressed.

• [x] My package addresses statistical or bioinformatic issues related to the analysis and comprehension of high throughput genomic data.

• [x] I am committed to the long-term maintenance of my package. This includes monitoring the support site for issues that users may have, subscribing to the bioc-devel mailing list to stay aware of developments in the Bioconductor community, responding promptly to requests for updates from the Core team in response to changes in R or underlying software.

• [x] I am familiar with the Bioconductor code of conduct and agree to abide by it.

I am familiar with the essential aspects of Bioconductor software management, including:

• [x] The 'devel' branch for new packages and features.
• [x] The stable 'release' branch, made available every six months, for bug fixes.
• [x] Bioconductor version control using Git (optionally via GitHub).

For questions/help about the submission process, including questions about the output of the automatic reports generated by the SPB (Single Package Builder), please use the #package-submission channel of our Community Slack. Follow the link on the home page of the Bioconductor website to sign up.

[bioc-issue-bot]

Hi @davidpross

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: InSituType
Type: Package
Title: An R package for performing cell typing in SMI and other single cell data
Version: 0.99.0
Authors@R: c(person("Patrick", "Danaher", email = "[email protected]", role = c("aut", "cre")),
   person("Zhi", "Yang", email = "[email protected]", role = c("aut")),
   person("David", "Ross", email = "[email protected]", role = c("aut")))
Description: Insitutype is an algorithm for performing cell typing in single cell 
   spatial transcriptomics data, such as is generated by the CosMx platform. 
   It can perform supervised cell typing from reference profiles, unsupervised clustering,
   or semi-supervised cell typing in which cells both reference cell types and de novo
   clusters are fit. 
biocViews: Clustering, SingleCell
Imports:
  Matrix,
  scales,
  umap,
  ggplot2,
  lsa,
  irlba,
  mclust,
  sparseMatrixStats,
  methods,
  rlang,
  grDevices,
  graphics,
  stats,
  utils,
  Rcpp (>= 1.0.9)
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
Suggests:
  knitr,
  testthat
VignetteBuilder: knitr
Depends:
  R (>= 3.5.0)
RoxygenNote: 7.2.1
LinkingTo: Rcpp, RcppArmadillo

[vjcitn]

Vignettes do not explain or give a reference to CosMx. Please give scientific role of the package/technology in the vignette introductions so that newcomers are given context.

[davidpross]

Vignettes do not explain or give a reference to CosMx. Please give scientific role of the package/technology in the vignette introductions so that newcomers are given context.

Hi @vjcitn, thanks for taking a look at our package and the feedback. Good timing, the CosMx paper was released online yesterday. I added a reference to the paper to the vignettes as well as a short description and a reference to the public dataset. I bumped the version of the package to 0.99.1 along with these changes.

Let me know if there are any other items to address to move package forward in submission process. Thanks!

[vjcitn]

Thanks for being so responsive. Now I feel we need to think about interoperability.

counts <- mini_nsclc$counts
str(counts)
#>  num [1:2198, 1:960] 0 0 0 0 0 0 0 0 0 0 ...
#>  - attr(*, "dimnames")=List of 2
#>   ..$ : chr [1:2198] "c_3_18_2" "c_3_18_3" "c_3_18_4" "c_3_18_5" ...
#>   ..$ : chr [1:960] "AATK" "ABL1" "ABL2" "ACE" ...
counts[25:30, 9:14]
#>           ACTA2 ACTG2 ACVR1 ACVR1B ACVR2A ACVRL1
#> c_3_18_39     0     0     0      0      0      0
#> c_3_18_40     0     0     0      0      0      0
#> c_3_18_41     1     0     1      0      0      0
#> c_3_18_45     2     0     0      0      0      0
#> c_3_18_46     0     0     0      0      0      0
#> c_3_18_47     0     0     0      0      0      0

is from the "clustering a small..." vignette. Bioconductor packages use SummarizedExperiment or related classes whenever possible to help bind metadata about experiments and features right to the numerical quantifications. Please consider adopting this discipline.

[davidpross]

Bioconductor packages use SummarizedExperiment or related classes whenever possible to help bind metadata about experiments and features right to the numerical quantifications. Please consider adopting this discipline.

I've opened an issue in our repository to track this suggestion. I don't think it is something that can be integrated for the timeline of the 3.16 Bioconductor release. Is it a requirement for inclusion in that release?

In the interim our package can be used in related workflows by extracting and transforming the counts matrix, t(assays(se)$counts).

[lshep]

Interop would need to be implemented in some fashion before inclusion.

[davidpross]

With this PR I added the option to use a SingleCellExperiment class object with the core package functions insitutype and insitutypeML and to retrieve the counts matrix from that object. In addition there is a new vignette detailing how to use insitutype with a SingleCellExperiment object.

[davidpross]

Hello @vjcitn and @lshep, with Bioconductor Release 3.16 just ahead, how can we move this forward in the review process? Thanks!

[vjcitn]

I'd suggest you look at your vignettes, which do not use BiocStyle, and compare them to others in the ecosystem. The early screens of NSCLC-clustering-SingleCellExperiment-vignette.html are dominated by loading messages and data dumps, and

# define cluster colors:
cols <-
  c(
    '#8DD3C7',
    '#BEBADA',
    '#FB8072',
    '#80B1D3',
    '#FDB462',
    '#B3DE69',
    '#FCCDE5',
    '#D9D9D9',
    '#BC80BD',
    '#CCEBC5',
    '#FFED6F',
    '#E41A1C',
    '#377EB8',
    '#4DAF4A',
    '#984EA3',
    '#FF7F00',
    '#FFFF33',
    '#A65628',
    '#F781BF',
    '#999999'
  )
cols <- cols[seq_along(unique(unsup$clust))]
names(cols) <- unique(unsup$clust)

par(mfrow = c(1, 2))
par(mar = c(0, 0, 3, 0))

plot(mini_nsclc$x, mini_nsclc$y, pch = 16, cex = .75, asp = 1, cex.main = 0.75,
          main = "cells in physical space",
     col = cols[unsup$clust], xlab = "", ylab = "", xaxt = "n", yaxt = "n")

plot(mini_nsclc$umap, pch = 16, cex = .75, asp = 1, cex.main = 0.75,
     main = "cells in UMAP space",     
     col = cols[unsup$clust], xlab = "", ylab = "", xaxt = "n", yaxt = "n")
legend("bottomleft", pch = 16, col = cols, legend = names(cols), cex = 0.7)

isn't indicative of a user-friendly procedure. Can you have plot methods with reasonable defaults that are callable with concise code?

[bioc-issue-bot]

A reviewer has been assigned to your package. Learn what to expect during the review process.

IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. It is required to push a version bump to git.bioconductor.org to trigger a new build.

Bioconductor utilized your github ssh-keys for git.bioconductor.org access. To manage keys and future access you may want to active your Bioconductor Git Credentials Account

[bioc-issue-bot]

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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to [email protected]:packages/InSituType to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[HelenaLC]

Hey there, as it's been 2 weeks, just a note in case this wasn't clear: We usually don't start formal review until there's a clean (warning- and error-free) build on on the Bioc system (see report above). If you have any questions regarding current issues, feel free to ask/discuss here.

[davidpross]

I've updated the package on GitHub to fix the errors in the build report. I am unable to push the changes to [email protected]:packages/InSituType though, I get the following error: FATAL: W any packages/InSituType patrickjdanaher DENIED by fallthru. Is it possibly because in the version on bioconductor.org I am not the package maintainer? The error mentions patrickjdanaher which is the GitHub handle for the maintainer listed in the DESCRIPTION file currently on the bioconductor.org git. In the changes I've recently made on the GitHub branch I am now the package maintainer.

[lshep]

Please try again. We have resolved the issue on our end and you should be able to push now.

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: 07ae239a0d4125327c00c5c30094f68f938a89ee

[bioc-issue-bot]

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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to [email protected]:packages/InSituType to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[HelenaLC]

Here a few initial comments. Note that some are suggestions and, although desirable, not strictly required (e.g., "encourage."). I marked some with !!! that are arguably more significant. Please comment back here with what has/not been addressed how/why not, or any comments regarding any needed clarification or questions you might have; happy to help/discuss. Cheers!

DESCRIPTION

• [ ] style/typos/grammar in the Description: field: Insitutype > InSituType, in which cells both > in which both

• [ ] perhaps worth adding "Spatial" under biocViews:

• [ ] LazsyData: TRUE should be removed or set to FALSE (see here)

• [ ] Depends: R (>= 3.5.0) doesn't seem very reasonable and should be removed as Bioc 3.17/.18 (where inSituType will end up) is incompatible with anything <4.2

• [ ] The umap package is currently used only in flightpath_layout.R. I'd suggest moving it from Imports: to Suggests: as to not have InSituType depend on it. See here for details as to why this might be desirable.

vignette

• [ ] We encourage using BiocStyle::html_document as rendering target.

• [ ] To highlight / distinguish code from normal text, I'd recommend consistently using back-ticks.

• [ ] Similarly, when mentioning R packages, BiocStyle has designated functions that highlight and hyperlink them, e.g., r Biocpkg("...") (there are similar commands for CRAN and GitHub packages). This is nice in that it distinguishes packages from plain code and directly directs users to external packages.

• [ ] Especially in the vignette, I would suggest sticking to a 80-character limit to assure code readability (especially when displayed in an online browser). Perhaps worth switching to 4-space tab indentation as opposed to vertical alignment as well to prevent large amounts of additional white space.

code

• [ ] Avoid sapply() and use vapply() instead.

• [ ] Avoid 1:... and use seq_len/along() instead.

• [ ] Avoid paste/0() in message/stop(); just this message/stop("text", "more text") works.

• [ ] For insitutype() (and any other function using message()), I'd suggest adding a verbose argument such that user can choose whether or not to print information of progress to the console.

• [ ] !!! I can see some S4 methods for SingleCellExperiment (e.g., insitutype/ML()), however, this is not consistently implemented. It'd be desirable to have functions work on both raw inputs (e.g., matrix) or Bioc objects (e.g., SCE) via extracting relevant pieces of data (see also comment below regarding mini_nsclc).

• [ ] I find the code-style currently quite difficult to read, largely due to vertical alignment and, consequently, 80+ characters in a lot of places... perhaps worth considering this in the future.

documentation

• [ ] Please add documentation for any data (e.g., source information under a ?data page, and/or inst/scripts code for how the data were generated).

• Just in case this wasn't on purpose, you can use @importFrom package function1 function2 instead of having a separate line per imported function.

• [ ] Throughout the DESCRIPTION, README, vignette etc., you refer to "insitutype". I'd suggest strictly distinguishing between "insitutype" and "InSituType" when referring to the function and package, respectively (in code-style / with back-ticks in either case).

other

• [ ] !!! The mini_nsclc example dataset is currently a list of target & negative probe counts, xy-coordinates, and a UMAP. This could easily be stored in a SingleCellExperiment (SCE) or SpatialExperiment (SPE) instead, which would make for a more comprehensible object that inter-operates nicely with existing Bioc infrastructure.

• [ ] !!! Related to the above, and as Vince already pointed out, the visualizations are currently very cluttered and far from user-friendly. Using a SCE/SPE instead would allow you to, for example, use scater for visualization (e.g., plotUMAP() or, more generally, ggcells()). Even constructing a data.frame of cell metadata and spatial coordinates, and plotting with ggplot2 would greatly improve this already.

• [ ] There are currently two licenses. The DESCRIPTION points to LICENCE (not the .md), though this one is empty and the .md it not. Please resolve this to keep one file only.

• [ ] In the README, please include Bioc installation instructions with BiocManager. I see there are place-holder installation instructions in the vignette; I'd say that these can be omitted as they will be included automatically on the Bioc website where the package vignette is to be found eventually.

• [ ] There are currently a few superfluous files in the package's base directory that should not be on Bioc's GH (e.g., by removing them or including them under .gitignore) e.g., reqs.md and specs.md, any vignettes/.html files (the .lintr should be sufficient to keep on you local but not the Bioc branch, I believe).

[bioc-issue-bot]

This issue is being closed because there has been no progress for an extended period of time. You may reopen the issue when you have the time to actively participate in the review / submission process. Please also keep in mind that a package accepted to Bioconductor requires a commitment on your part to ongoing maintenance.

Thank you for your interest in Bioconductor.