Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #221 +/- ##
===========================================
- Coverage 92.57% 82.01% -10.57%
===========================================
Files 41 69 +28
Lines 1280 3192 +1912
===========================================
+ Hits 1185 2618 +1433
- Misses 95 574 +479 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4f9d525 to
ef344cb
Compare
There was a problem hiding this comment.
Hi @AtheerAS,
Thanks, looks great overall. Left only minor comments.
Major points to discuss:
- Unit tests for each new functions (I suggest starting with those; then changes to the rest would be easier)🙏 🙏 🙏 PS. I added tests for inflow, so that I can extend it to consider compositions.
- Can we try to think of some global summaries to add to
lrdata.var? For example, simply the average or sum; a cell-type frequency scaled average, and e.g. one that represents the mean / var ratio for each interaction; compute_global_score- I'm not sure if this should be constrained to lrdata objects; I also moved it toli.mt.sp; also,compute_global_specificity- to be more ... specific :)- Would it make more sense to filter non-zero variance for
lsrather than lrdata at the end? plot_spatial_inflow_score_single_interaction- let's try to pick a shorter and broader name; not limited to lrdata. More generally, is there any way to simplify a bit, maybe not all optional parameters are needed; i'd have nightmares that a user encountered an issue with it 😄 - see minor comments :)- Would be cool to include a function for the gradient plot also :)
- Inflow tutorial: Also, something is off with the logic where the resource is filtered. The order can also be improved a bit - e.g. show a simple interaction, and explain it how you did in the presentation (with each variable side-by-side), then go into permutations, complex plots by cell type, etc.
| import warnings | ||
| from typing import List, Tuple, Optional, Union | ||
|
|
||
| def plot_spatial_inflow_score_single_interaction( |
There was a problem hiding this comment.
Lets try to think of a snappier name, this is a bit excessive :) I would also not limit it to inflow scores; you are plotting multiple features across cell types on a slide; spatial_features_by_obs or spatial_features_by_group
src/liana/plotting/_plot_spatial_inflow_score_single_interaction.py
Outdated
Show resolved
Hide resolved
src/liana/plotting/_plot_spatial_inflow_score_single_interaction.py
Outdated
Show resolved
Hide resolved
| # Build observed df | ||
| parts = [n.split(lr_sep) for n in full_names.tolist()] | ||
| df = pd.DataFrame(parts, columns=["source", "ligand", "receptor", "target"]) | ||
| df["lr_mean"] = values |
There was a problem hiding this comment.
changing lr_mean to specificity_rank to be compatible with liana_res format and function name.
| """Helper for splitting complex names.""" | ||
| toks = name.split(complex_sep) | ||
| if len(toks) > 1: | ||
| return toks[0], complex_sep.join(toks[1:]) |
There was a problem hiding this comment.
Edited this so complex name includes primary L/R as well (it was not).
src/liana/plotting/_heatmap.py
Outdated
| # This should technically not be hit if df_satisfy wasn't empty, but kept for robustness | ||
| raise ValueError(f"No interactions found for ligand_complex={ligand_complex}, receptor_complex={receptor_complex} after groupby-level filtering.") | ||
|
|
||
| heatmap_df = df.pivot(index="source", columns="target", values="specificity_rank") |
| from liana._logging import _logg | ||
|
|
||
| @d.dedent | ||
| def spatial_inflow( |
dbdimitrov
left a comment
There was a problem hiding this comment.
Hey, @AtheerAS, I left a second round of comments now that I tested re-running things.
LMK if I can clarify anything 🙏
src/liana/plotting/_heatmap.py
Outdated
|
|
||
|
|
||
| # convert p-values to stars | ||
| def p_to_star(p): |
There was a problem hiding this comment.
This did not work in the tutorial. Also, let's try to make more general - e.g. see heatmap with star in test_inflow.ipynb
There was a problem hiding this comment.
Would also make sense to have this function passed as a parameter to the function - I guess this would fix it; also make it a '_annotate_fun' and pass it to a annotate_lambda parameter; which is broader, not just pval -> then the function would work for e.g. cell pair proximity also
Add inflow module and tutorials (incl. plotting functions & global score function).