Skip to content

ManifoldMap app#19

Merged
maximlt merged 58 commits into
mainfrom
featuremap
Jun 23, 2025
Merged

ManifoldMap app#19
maximlt merged 58 commits into
mainfrom
featuremap

Conversation

@droumis

@droumis droumis commented Mar 24, 2025

Copy link
Copy Markdown
Contributor
image
Get Data
anndata_file_path = pooch.retrieve(
    url="https://datasets.cellxgene.cziscience.com/ad4aac9c-28e6-4a1f-ab48-c4ae7154c0cb.h5ad",
    fname="ad4aac9c-28e6-4a1f-ab48-c4ae7154c0cb.h5ad",
    known_hash="00ee1a7d9dbb77dc5b8e27d868d3c371f1f53e6ef79a18e5f1fede166b31e2eb",
    path="data-download"
)
adata = ad.read_h5ad(anndata_file_path)

TODO in this PR:

  • Ensure the legend is to the right when datashade and overlaid labels are disabled -> Bokeh bug, fixed in Fix positioning of DOM rendered Legend annotations bokeh/bokeh#14457, released in version 3.7.3
  • Separate selection of different color-by collections (thousands of genes in adata.var_names vs typically tens of result cols in adata.obs.columns)
  • Fix issue when continuous gene expression measure from e.g. adata.var_names[0] is input for color_by... for some reason it's not always being found and quietly resorts to choosing the first color_by option from adata.osbm.. Looks like there's the plotting is being double-triggered and it's correct at first but then gets messed up.. -> Haven't been able to reproduce
  • Clean up the agg tooltip for cat counts
  • Utilize new advances with datashader inspections to clean up the hover tooltips
  • Add tests
  • Fix precommit errors
  • Add colormap selection
  • Switch to non-datashading at certain zoom in range (e.g. use 'apply_when')
  • Enable lasso select
  • Center the labels over points in viewport range
  • Add a check for gene symbols (e.g. 'ctla4') somewhere as a .var column and use them instead of the 'ENS...' strings for the color_by variable options if they exist.

Future TODOs and Wishlist Items moved here

@flying-sheep

Copy link
Copy Markdown
Collaborator

Hi @droumis, sorry for making you fight Ruff so much! I fixed the rest.

@droumis droumis changed the title Add a featuremap application Add a ManifoldMap app Apr 24, 2025
@droumis
droumis marked this pull request as ready for review April 24, 2025 00:13
@droumis droumis changed the title Add a ManifoldMap app Initialize ManifoldMap plot Apr 24, 2025
@codecov

codecov Bot commented Apr 24, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.35294% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.38%. Comparing base (15783b4) to head (b6f2abd).
⚠️ Report is 50 commits behind head on main.

Files with missing lines Patch % Lines
src/hv_anndata/manifoldmap.py 82.15% 24 Missing and 14 partials ⚠️
tests/test_manifold.py 96.72% 0 Missing and 4 partials ⚠️
tests/conftest.py 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #19       +/-   ##
===========================================
+ Coverage   70.21%   80.38%   +10.17%     
===========================================
  Files           6        8        +2     
  Lines         235      571      +336     
  Branches       28       60       +32     
===========================================
+ Hits          165      459      +294     
- Misses         58       82       +24     
- Partials       12       30       +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@droumis
droumis requested a review from maximlt April 24, 2025 00:42
@droumis

droumis commented Apr 24, 2025

Copy link
Copy Markdown
Contributor Author

Need to ensure the legend is to the right when datashade and overlaid labels are disabled

image

@droumis

droumis commented Apr 24, 2025

Copy link
Copy Markdown
Contributor Author

Fix issue when continuous gene expression measure from e.g. adata.var_names[0] is input for color_by... for some reason it's not always being found and quietly resorts to choosing the first color_by option from adata.osbm

image

@droumis droumis self-assigned this Apr 24, 2025
@droumis

droumis commented Jun 11, 2025

Copy link
Copy Markdown
Contributor Author

I added the following task, but feel free to punt it to the next PR if there's no time to address it by EOW

"Add a check for gene symbols (e.g. 'ctla4') somewhere as a .var column and use them instead of the 'ENS...' strings for the color_by variable options if they exist."

Context:

  • Hugo gene names are unique, descriptive identifiers for human genes, often reflecting the gene's function or associated disease. They are typically long strings of words, like "cytotoxic T-lymphocyte-associated protein 4," with a corresponding gene symbol (CTLA4). In contrast, ENS strings are Ensembl stable IDs, which are unique numerical or alphanumeric identifiers for genes, transcripts, or other genomic features. They are not descriptive and serve as a persistent identifier for genomic elements across different Ensembl releases.

@maximlt

maximlt commented Jun 13, 2025

Copy link
Copy Markdown
Contributor

Switch to non-datashading at certain zoom in range (e.g. use 'apply_when')

I thought implementing this would be straightforward. Apparently, I haven't yet done this long enough yet to guess that right!

First, I've hit an issue similar to this one holoviz/hvplot#1414, because I added apply_when only to the rasterize operation, which made dynspread pretty unhappy. Worked around this by applying both operations at the same time (apply_when(obj, operation=rasterize_and_dysnpread, predicate=...)), which immediately failed because the code would then call .opts on the returned object, with options not appropriate for the object involved when the predicate is false (i.e. no rasterization). Workaround seems to be to apply the .opts() in rasterize_and_dynspread directly.

Then, I tried the above with categorical color data and hit two errors, reported in holoviz/holoviews#6620, with a Bokeh front-end error and a HoloViews python error. Looked a bit into it but would need more time to figure out what's going on there.

I kept going with the continuous color data and managed to get something working (f9a5689). I have tested it a bit and it seemed to work, though it would need more testing.

@maximlt

maximlt commented Jun 13, 2025

Copy link
Copy Markdown
Contributor

"Add a check for gene symbols (e.g. 'ctla4') somewhere as a .var column and use them instead of the 'ENS...' strings for the color_by variable options if they exist."

On this, looking into the dataset I've been using (https://cellxgene.cziscience.com/collections/efd94500-1fdc-4e28-9e9f-a309d0154e21), I can see it has a .var["feature_name"] column:

ENSG00000121410       A1BG_ENSG00000121410
ENSG00000148584       A1CF_ENSG00000148584
ENSG00000175899        A2M_ENSG00000175899
ENSG00000166535      A2ML1_ENSG00000166535
ENSG00000184389    A3GALT2_ENSG00000184389
                            ...           

Is this "feature_name" a convention of some sort? If so, we could make it a default, or at least by default look for it.

It looks like each entry is formatted as {HUGO gene name}_{ENS string}. In this dataset, it appears that the HUGO gene names are not unique; there are a few duplicates (NPIPA9, LINCO0325, etc.).

red = adata.var["feature_name"].apply(lambda r: r.split("_")[0])
adata.var.loc[red.duplicated(keep=False), "feature_name"]
ENSG00000183889       NPIPA9_ENSG00000183889
ENSG00000276462    LINC03025_ENSG00000276462
ENSG00000188626      GOLGA8M_ENSG00000188626
ENSG00000237667    LINC01115_ENSG00000237667
ENSG00000233024       NPIPA9_ENSG00000233024
ENSG00000254319    LINC03021_ENSG00000254319
ENSG00000277526    LINC03021_ENSG00000277526
ENSG00000234394    LINC03025_ENSG00000234394
ENSG00000261480      GOLGA8M_ENSG00000261480
ENSG00000272342    LINC01115_ENSG00000272342

So I've decided to let the name as is, instead of splitting feature_name by '_' to get the HUGO gene name. Implemented in acab66f.

image

@droumis

droumis commented Jun 16, 2025

Copy link
Copy Markdown
Contributor Author

Nice! yea I think exposing it is sufficient, we can always change it later if a standardized unique hugo-esque convention is field is formed.

Are you working on holoviz/holoviews#6620 or does it need a team discussion to move forward?

@droumis

droumis commented Jun 16, 2025

Copy link
Copy Markdown
Contributor Author

Looks like the Bokeh tools differ between categorical and continuous color_by options... we definitely always want box and lasso selection tools.

Also, can we make the selection tool persistent so that selected area is visible even while datashading points?

image image

@maximlt

maximlt commented Jun 20, 2025

Copy link
Copy Markdown
Contributor

Are you working on holoviz/holoviews#6620 or does it need a team discussion to move forward?

Simon has fixed (merged already) the Python error I reported. The front-end error is still there. I'm not sure to what extent he looked into it holoviz/holoviews#6620 (comment).


Looks like the Bokeh tools differ between categorical and continuous color_by options... we definitely always want box and lasso selection tools.

That's now fixed. I think it is related to holoviz/holoviews#6240, i.e. setting tools via .opts on an overlay has no effect.

Also, can we make the selection tool persistent so that selected area is visible even while datashading points?

I've enabled that in c138b28


At the moment, apply_when is enabled only for the continuous case. I had never used apply_when "for real" and did not anticipate so many papercuts. I reported a few already:

The categorical case is giving me more trouble:

I need to spend a bit of time creating an MRE for each one of these issues (making sure it's not PEBKAC!).

@droumis if you need this PR merged soon, I'd suggest postponing using apply_when for the categorical case, and possibly reverting for the continuous one to simplify things.

@droumis

droumis commented Jun 20, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for catching all of these! I think it's great that we're finally road-testing apply_when. Let's revert and save apply_when for the next PR so we can merge this one.

@droumis

droumis commented Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

@maximlt , I made the plot sizing responsive. the widthandheightargs will be used asmin_widthandmin_heightifresponsiveisTrue`. Let me know if you think this is bad idea/implementation for some reason.

@maximlt

maximlt commented Jun 23, 2025

Copy link
Copy Markdown
Contributor

@maximlt , I made the plot sizing responsive. the widthandheightargs will be used asmin_widthandmin_heightifresponsiveisTrue`. Let me know if you think this is bad idea/implementation for some reason.

LGTM!

I'm going to fix the test suite now and I guess we can merge soon after that.

@droumis

droumis commented Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

feel free to merge when you're ready with the tests

@maximlt
maximlt merged commit 93ab705 into main Jun 23, 2025
5 of 6 checks passed
@maximlt
maximlt deleted the featuremap branch June 23, 2025 13:50
@github-project-automation github-project-automation Bot moved this from In Progress to Done in NIH-NCI Jun 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants