drighelli / SpatialExperiment

55 stars 20 forks source link

read10xVisium should handle missing images more gracefully #23

Closed LTLA closed 3 years ago

LTLA commented 3 years ago

I was testing this out and I didn't want to run Spaceranger, so I just downloaded files from 10X's webside and reconstructed the SRanger output directory structure. This worked okay up to a bizarre error:

out <- read10xVisium("whee", type="sparse")
## Error in switch(iid, lowres = "tissue_lowres_scalef", "tissue_hires_scalef") :
##   EXPR must be a length 1 vector

After some debugging, I realized it was because I didn't pull down the full resolution image, because it was annoying large. A more user-friendly approach would be for read10xVisium() to skip over images that aren't present, emitting a warning as it does so. An informative error would also be fine, at least I wouldn't have spent a few minutes trying to debug:

https://github.com/drighelli/SpatialExperiment/blob/79a94c8063c087151c7a0bdf6bb874eca4ee4e17/R/readImgData.R#L53-L73

(Which is very hard, by the way, I can't get into the inner functions. Suggest you unravel this into a nested for loop.)

drighelli commented 3 years ago

I think you're right on this, we can easily provide more informative error messages.

I'll do it.

HelenaLC commented 3 years ago

In the current devel, I skip over missing images with a message, and stop if no images are matched at all - open to suggestions for improvement.

LTLA commented 3 years ago

That sounds fine. One could argue over whether to use a message() or a warning(), but I suppose it's fine.