search

satijalab / seurat-object

https://satijalab.github.io/seurat-object/
Other
24 stars 25 forks source link

Unexpected values returned by FetchData() / unsafe programming / change help #201

Open vertesy opened 7 months ago

vertesy commented 7 months ago

Dear Seurat Team,

I used FetchData(object = obj, vars = "UMAP_1") to retrieve UMAP coordinates in Seurat.utils::AutoNumber.by.UMAP().

This worked until Seurat v5. After Seurat v5 it still executed without a warning, but the coordinates returned did not correspond to the umap plotted.

What happens?

  1. In v5 UMAP_1 got renamed to umap_1, in @reductions$umap
  2. FetchData now silently returns UMAP_1 from obj@reductions$ref.umap instead from obj@reductions$umap.
  3. I did not even know about ref.umap, and getting values from it is very unexpected when asking for a umap coordinate. I needed to look through 2MB of "str(obj)" output to get this info.
  4. The solution is that I will not use FetchData, bc I rather break the code then get meaningless values to figure out much later.

Solutions

  1. FetchData is unsafe currently, even if it "works correctly". Please add some reporting or break early.
  2. Please change help. Stop recommending in ?FetchData to use as: pc1 <- FetchData(object = pbmc_small, vars = 'PC_1'), bc apparently you would need to specify the PCA slot to be sure what to get.
  3. I encountered now multiple times such unsafe programming in Seurat, where sth is returned without message/warning/error, so this is also a general request to increase prudency. See example: NA values in DiscretePalette needed to be handled by quick and dirty Seurat.utils::DiscretePaletteSafe().

Thank you for your time sharing and maintaining the code. Abel

samuel-marsh commented 7 months ago

Hi Abel,

Not member of dev team but hopefully can be helpful. I don't know that this is actually an error in FetchData. FetchData will not return anything with FetchData(object = obj, vars = "UMAP_1") if the object does not have a reduction with "UMAP" as key in dimreducs. So that isn't actually returning "meaningless" values but rather the values that it is supposed to return.

test <- FetchData(object = pbmc_small, vars = "UMAP_1")
Error in `FetchData()`:
! None of the requested variables were found: UMAP_1
Run `rlang::last_trace()` to see where the error occurred.

I don't know that there is a way to make that "safe" other than for the user to to know the keys and dimreducs of their object before calling functions.

Also it would seem safer way to get info for what you seem to be looking for to simply retrieve the embeddings user the provided accessor function (Embeddings) for which you specify the reduction name and will come with the appropriate dim names. This ensures the correct reduction is accessed and correct dimnames and embeddings are returned.

Also just curious how do you get NAs from DiscretePalette? That isn't something I've ever encountered.

Best, Sam

vertesy commented 7 months ago

Hi Sam, thanks for your response, I will try to better explain my original post, but I think you are not referring to a situation that I described.