https://twitter.com/mjskay/status/1660689831809171469

• Set up Bioc Badges
• Add Bioc Check to GitHub Action
  • #5
• Invert spot coordinates such that the orientation of the spots aligns with images
  • #13

Here are some suggestion on improving the clarity of documentation from @cindyfang70.

• Clarify what the purpose for the argument reverse_y
• Add syntax image to the getting started with escheR vigenette.

Thanks @cindyfang70 for the feedback!

Suggested by Byron Jaeger.

Current code default the names for the spatialCoords to be "pxl_row_in_fullres", "pxl_col_in_fullres" without giving a warning.

Need to have more flexible implementation.

Replicating Example

n <- length(z <- letters)
y <- matrix(nrow = n, ncol = n)
cd <- DataFrame(x = seq(n), y = seq(n), z)

spe1 <- SpatialExperiment(
    assay = y, 
    colData = cd, 
    spatialCoordsNames = c("x", "y"))

THere are error message that has not been updated

Error in make_escheR.SpatialExperiment(spe[, spe$sample_id %in% c("V13M06-342_D1")]) : 
  make_escheR currently only supports spe object with 1 sample. Please find tips multiple sample plots in [TODO] vignettes.

• Add bioconductor installation instruction
• Add a subsection introducing the installation method for the R version before 4.3.0.

Contributor: @Erik-D-Nelson

• A function to normalize continuous variables to RGB color space

  • Document the arguments (parameters) of the function

• A case study applying this concept to a Visium dataset

if (!exists("spe"))
spe <- fetch_data("spatialDLPFC_Visium")

Package 'escheR' Review

Thank you for submition your package to Bioconductor. The package passed check and build. It is in pretty good shape. However there are several things need to be fixed. Please try to answer the comments line by line when you are ready for a second review.
Code: Note: please condsider; Important: must be addressed.

The DESCRIPTION file

• R version is not clear in DESCRIPTION.

Response: Added R(>=3.6) as dependency

The NAMESPACE file

• Selective imports using importFrom instead of import all with import.
  • in line 8 import(SpatialExperiment)
  • in line 9 import(ggplot2)
  • in line 10 import(spatialLIBD)

Response: Imported individual functions instead of entire packages

R code

• Important: No paste in message(), message, stop
  • In file R/add_fill.R:
    • at line 34 found ' stop(paste0("Please add the variable ", var, " to colData(spe)."))'
  • In file R/add_ground.R:
    • at line 37 found ' stop(paste0("Please add the variable ", var, " to colData(spe)."))'
  • In file R/add_symbol.R:
    • at line 36 found ' stop(paste0("Please add the variable ", var, " to colData(spe)."))'

Response: removed all paste functions in stop etc. Thanks for this tip.

• NOTE: :: is not suggested in source code unless you can make sure all the packages are imported. Some people think it is better to keep ::. However please note that you need to manully double check the import items when you make any change in the DESCRIPTION file during development. My recommendation is to remove one or two repeats to force the dependency check.
  • In file R/make_escheR.R:
    • at line 58 found ' d <- as.data.frame(cbind(colData(spe), SpatialExperiment::spatialCoords(spe)), optional = TRUE)'
    • at line 64 found ' SpatialExperiment::imgRaster('
    • at line 86 found ' x = pxl_col_in_fullres * SpatialExperiment::scaleFactors(spe, sample_id = sampleid, image_id = image_id) - adjust$x,'
    • at line 87 found ' y = pxl_row_in_fullres * SpatialExperiment::scaleFactors(spe, sample_id = sampleid, image_id = image_id) - adjust$y,'

Response: Thank you very much for this information. I followed your suggestion to make some functions without :: such that the source code for the dependency check. But I would still like to leave some of them in the code to enhance readability for people who are not familiar with these packages. Thanks for sharing this trick!

• Important: 1:n is not suggested in source code. Use seq_along or seq.int instead.
  • In file R/add_fill.R:
    • at line 59 found ' # setNames(c(paste0("L", 1:6), "WM", "NA", "WM2"))'

Response: removed the commented code and hence this line. But I will be more careful with my coding practice to use seq.int etc instead of 1:n. Thank you for this tip!

• Important: Remove unused code.
  • In file R/add_fill.R:
    • at line 31 found ' # browser()'
    • at line 37 found ' # if(is.numeric(p$data[,var])) {'
    • at line 38 found ' # stop("")'
    • at line 39 found ' # p <- suppressMessages({p + scale_fill_continuous()})'
    • at line 40 found ' # } else{'
    • at line 41 found ' # p <- suppressMessages({p + scale_fill_discrete()})'
    • at line 42 found ' # }'
    • at line 45 found ' # browser()'
    • at line 54 found ' # if(!is.numeric(p$data[,var]))'
    • at line 55 found ' # tmp <- tmp +'
    • at line 56 found ' # scale_fill_manual('
    • at line 57 found ' # name = "",'
    • at line 58 found ' # values = libd_layer_colors |>'
    • at line 59 found ' # setNames(c(paste0("L", 1:6), "WM", "NA", "WM2"))'
    • at line 60 found ' # )'
    • at line 63 found ' # tmp$scales <- tmp$scales[[-1]]'
  • In file R/make_escheR.R:
    • at line 51 found ' ## (http://spatial.libd.org/spatialLIBD/)'

Response: removed the commented code.

• NOTE: Avoid 'suppressWarnings'/'*Messages' if possible
  • In file R/add_fill.R:
    • at line 39 found ' # p <- suppressMessages({p + scale_fill_continuous()})'
    • at line 41 found ' # p <- suppressMessages({p + scale_fill_discrete()})'

Response: removed the commented code. But I will be more careful with the practice in general in the future. Thank you for this tip!

• NOTE: Functional programming: code repetition.
  • repetition in add_fill and add_ground and add_symbol
    • in add_fill
      • line 1: point_size = 2, ...)
      • line 2:{
      • line 3: if (!is.character(var) || length(var) != 1) {
      • line 4: stop("The argument var must be character of length one.")
      • line 5: }
      • line 6: if (!var %in% colnames(p$data)) {
      • line 7: stop(paste0("Please add the variable ", var, " to colData(spe)."))
      • line 8: }
      • line 9: p + geom_point(aes(fill = !!sym(var)), shape = 21, stroke = 0,
    • in add_ground
      • line 1: var, stroke = 0.5, point_size = 2, ...)
      • line 2:{
      • line 3: if (!is.character(var) || length(var) != 1) {
      • line 4: stop("The argument var must be character of length one.")
      • line 5: }
      • line 6: if (!var %in% colnames(p$data)) {
      • line 7: stop(paste0("Please add the variable ", var, " to colData(spe)."))
      • line 8: }
      • line 9: p + geom_point(aes(color = !!sym(var), ), shape = 21, fill = "transparent",
    • in add_symbol
      • line 1: var, size = 1, ...)
      • line 2:{
      • line 3: if (!is.character(var) || length(var) != 1) {
      • line 4: stop("The argument var must be character of length one.")
      • line 5: }
      • line 6: if (!var %in% colnames(p$data)) {
      • line 7: stop(paste0("Please add the variable ", var, " to colData(spe)."))
      • line 8: }

Response: Thank you for the suggestion! I am aware of this situation, and currently working on a check_var function for this purpose and some more checks. This improvement will be fully implemented in the next update of the package.

• Note: move theme in function make_escheR.SpatialExperiment to a sub_function.

Response: Thank you for the suggestion! I'll reserve this as an option in the next update of the package.

• Note: Use dots to pass parameters to geom_point in add_symbol function and add_fill function and let user to overwrite the defaults.

Response: Thank you for the suggestion! I had a similar idea, but I would like to have some more time to think about the design of this function. I will prioritize this tasks in the next update of this package.

Documentation

• Important: Please include Bioconductor installation instructions using BiocManager.
  • rmd file vignettes/SRT_eg.Rmd

Response: Added BiocManager installation instructions

• Note: Vignette includes motivation for submitting to Bioconductor as part of the abstract/intro of the main vignette.
  • rmd file vignettes/SRT_eg.Rmd

Response: Remove mentioning of Bioconductor submission

Originally posted by @jianhong in Bioconductor/Contributions#2988 (comment)

As noted in the vignette Beyond Sequence-based Spatially-Resolved Data, the binning strategy proposed to address overplotting could lead to distorted interpretation due intermixing of data points within each bin.

A possible solution to address this is to display the proportion of majority membership within each bin using either transparency (alpha) or boundary size (stroke).

Before v0.99.7, the plots generated by escheR looks upside down comparing to underlying tissue images

Hello!

I am trying to change the spot size of the points using the example you have online but have been unable to do so. See below the code I am running.. is there something I am doing wrong? Thanks so much!

R code

library(STexampleData)
spe <- Visium_humanDLPFC()
make_escheR(spe, spot_size=0.1) |> add_fill(var = "in_tissue")
make_escheR(spe, spot_size=5) |> add_fill(var = "in_tissue")
make_escheR(spe, spot_size=10) |> add_fill(var = "in_tissue")

All these look the same