-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Minor issues in milopy tools #181
Comments
@emdann I'd be in favor of dropping the |
Hopefully the new tutorial covers the use of contrasts to specify comparisons. I could add an example on subsetting to a certain lineage or something. |
@emdann It would also be great to add plot_milo_diagnostics into milopy. I found this function is pretty helpful and has been used for several times in best-practice notebook. |
@emdann do you think that you'd have time to tackle this soonish? |
In principle I agree with adding the diagnostic plots to the package, although in my head that's more material for extended tutorials since it needs some guidance for interpretation. But up to you guys to decide what fits in the package and what doesn't! Unfortunately I am going to be slumped in response-to-reviewer work for the next ~ 4 weeks. |
@emdann let's tackle this at the hackathon, OK? |
Noting down issues to fix and potential improvements to milopy, encountered while writing tutorial (scverse/pertpy-tutorials#4) - issues entirely inherited from my code ofc :)
milo.add_covariate_to_nhoods_var
add_covariate_to_nhoods_obs
and update docscategory
toobject
and thennhood_counts_by_cond
complainspt.pl.milo.nhood_counts_by_cond
add_covariate_to_nhoods_var
)milo.da_nhoods
:subset_samples
andsubset_nhoods
is buggy, in some edge cases this messes up the SpatialFDR calculation. While the bug is easy enough to catch with diagnostic plots (randomized SpatialFDR) and to fix (Misc bug fixes emdann/milopy#30), I'm wondering whether these parameters should be dropped completely in the new implementation in pertpy. The use case ofsubset_samples
is essentially covered by specifying contrasts (+ adding a new column in adata.obs if needed to specify groups). Usingsubset_nhoods
is somewhat of a pathological hack (I don't think I've ever used it and I can't even remember the original use case for adding this parameter). In practice if one wants to test on a specific subset of cell phenotypes the best solution is to subset cells and restart from KNN graph building.Happy to open a PR to implement these changes.
The text was updated successfully, but these errors were encountered: