drighelli / SpatialExperiment

54 stars 18 forks source link

Why grobs? Why not raster? #27

Closed LTLA closed 3 years ago

LTLA commented 3 years ago

Is there a reason why you store the images as grobs and not as raster objects? The latter is present in base R and does not force downstream users into visualization with grid, while remaining trivially convertible into grobs via rasterGrob.

drighelli commented 3 years ago

This reminds me of the warning that we are sometimes having when we load_all() the package.

Warning message: class "rastergrob" is defined (with package slot ‘SpatialExperiment’) but no metadata object found to revise superClass information---not imported? Making a copy in package ‘SpatialExperiment’

Maybe @HelenaLC can better answer to your question.

nilseling commented 3 years ago

Great job! But why wouldn't you directly go for EBImage::Image?

https://bioconductor.org/packages/release/bioc/vignettes/EBImage/inst/doc/EBImage-introduction.html#3_Image_data_representation

That would make it easier to work with the stored images and would better integrate with existing classes for multiplexed images: https://bioconductor.org/packages/release/bioc/vignettes/cytomapper/inst/doc/cytomapper.html#5_The_CytoImageList_object

drighelli commented 3 years ago

Hi @nilseling we looked into your package and we thought about this and our choice was to not be stuck to a specific package for the images and the plots.

If I remember right the EBImage has not very good support for ggplot2, which is the most used package for the plots in R at the moment. (But I could be wrong on this)

Do you have any suggestions?

nilseling commented 3 years ago

Hey @drighelli, you are right that EBImage does not support ggplot2. On the other hand, ggplot2 does not support multi-channel images. So if you only want to display a single RGB image (e.g. H&E) as is, ggplot is the better choice. If you ever want to store IF images or go into spatial proteomics, you might want to look into EBImage.

drighelli commented 3 years ago

@nilseling thanks for the answer and the clarification!

At the moment, we thought that the SpatialExperiment will be mainly used for Spatial Transcriptomics data and in this field, we have only the 10x Visium technology which produces RGB images. Some spatial multimodal technologies are arising, but I'm not sure they have IF images at the moment.

Anyway, I'm sure that we can provide the EBImage support in the next future.

Do you think that our class can be useful for general IF images? I saw a few "voronoi-like" tessellation of IF images with your package.

nilseling commented 3 years ago

Cool, makes sense. EBImage support is anyway given as long as you store the images as something that can be coerced into an array. As for the usefulness of SpatialExperiment for the multiplexed imaging community: yes! The examples you saw in my packages are pretty standard multi-channel images (where you can adjust the intensity of each channel) on which cells are outlines based on the segmentation results. Segmentation can be done via voronoi tessellation, watershed or more complex approaches (ilastik/CellProfiler). I believe you would also use these things to get the counts of seqFISH, MERFISH etc data. The question is just if you want to support pixel-level in addition to cell/molecule-level information. But it seems you are happy with the cell/molecule-level info, which is of course absolutely fine and urgently needed to push the ST field. I'm happy to discuss more in the future!

drighelli commented 3 years ago

We thought about FISH images data, but it seems that the images are only raw data that no one uses. It seems that everyone starts from the molecule-based long-format file and this is obviously justified by the huge number of high-resolution images you have in a FISH experiment. So it'd be really difficult to store them in memory, I suppose.

I'm happy to discuss it too! :)

nilseling commented 3 years ago

Yeah, the meaning of a channel in smFISH is different from the meaning of a channel in multiplexed IF etc. Storing these images in memory is anyway a mess and I'm currently building an on-disk framework for multi-channel images. But more to come soon. So you can close this issue once you settle on how to store the images.

drighelli commented 3 years ago

looking forward to this framework!

LTLA commented 3 years ago

If the SpatialImage class hierarchy is defined as proposed in #24, there is no issue with compatibility. One simply needs to define a SpatialMultiChannelImage subclass that implements additional methods related to multiple channels. The only requirement is that it must support the base SpatialImage contract; currently, this is to define a width and height and emit a grob (but ideally raster instead) object on request.

HelenaLC commented 3 years ago

@LTLA can you tell me exactly what you mean by raster? Is the class literally called raster? I am only confused because it's currently not actually a grob, but the class is called rastergrob or something... There's no particular reason for this. I basically used it just because it's what magick was giving me I think... I am also confused because I actually call rasterGrob to make the grob - so are we already doing this and just the name (grob) is off?

LTLA commented 3 years ago

Check out ?as.raster in base.

HelenaLC commented 3 years ago

Issue resolved with new SpatialImage class hierarchy.