search

MarioniLab / miloDE

Framework for sensitive DE testing (using neighbourhoods)
Other
57 stars 2 forks source link

Milo-object check in assign_neighborhood misses milo-objects #39

Closed MaximilianNuber closed 1 month ago

MaximilianNuber commented 4 months ago

Dear all, Thank you for the great package. I want use miloDE with a milo-object previously constructed with miloR (to use BPPARAM, BSPARAM, etc.). The source code in https://github.com/MarioniLab/miloDE/blob/main/R/assign_neighbourhoods.R at line 68 uses if (is(x , "SingleCellExperiment")), where x is the input SingleCellExperiment and/or Milo-object.

I believe the problem is, that a milo-object is still a SingleCellExperiment (is(milo.obj, "SingleCellExperiment") == TRUE) therefore the function continues to construct a new neighborhood graph even though the input is a milo-object with existent graph. In some trial and error i found is(milo.obj, "SingleCellExperiment") && !is(milo.obj, "Milo") to be the best check here. Would it be alright to test this and possibly update?

Thank you and best regards, Max

amissarova commented 1 month ago

@MaximilianNuber I'm sorry, I completely missed this issue - I will do some tests and update shortly - but yeah, your suggestion seems to be a good fit

amissarova commented 1 month ago

@MaximilianNuber fixed and thanks for bringing it to my attention (and sorry for replying so late, this notification completely slipped through me)