Skip to content

feat: base without X_#4076

Open
flying-sheep wants to merge 22 commits into
mainfrom
naked-base
Open

feat: base without X_#4076
flying-sheep wants to merge 22 commits into
mainfrom
naked-base

Conversation

@flying-sheep

@flying-sheep flying-sheep commented Apr 20, 2026

Copy link
Copy Markdown
Member

TODO:

@codecov

codecov Bot commented Apr 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.32432% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.91%. Comparing base (ee7707b) to head (03c8a2a).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/scanpy/plotting/_tools/scatterplots.py 42.85% 8 Missing ⚠️
src/scanpy/_keys.py 88.88% 6 Missing ⚠️
src/scanpy/tools/_ingest.py 61.53% 5 Missing ⚠️
src/scanpy/external/tl/_trimap.py 20.00% 4 Missing ⚠️
src/scanpy/external/exporting.py 25.00% 3 Missing ⚠️
src/scanpy/_utils/__init__.py 83.33% 1 Missing ⚠️
src/scanpy/plotting/_utils.py 66.66% 1 Missing ⚠️
src/scanpy/tools/_utils.py 88.88% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4076      +/-   ##
==========================================
+ Coverage   79.74%   79.91%   +0.16%     
==========================================
  Files         120      121       +1     
  Lines       12852    12928      +76     
==========================================
+ Hits        10249    10331      +82     
+ Misses       2603     2597       -6     
Flag Coverage Δ
hatch-test.low-vers 78.96% <84.32%> (+0.14%) ⬆️
hatch-test.pre 79.76% <84.32%> (+0.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/scanpy/_settings/presets.py 90.84% <100.00%> (+7.89%) ⬆️
src/scanpy/experimental/pp/_normalization.py 94.56% <100.00%> (+0.24%) ⬆️
src/scanpy/experimental/pp/_recipes.py 100.00% <100.00%> (ø)
src/scanpy/neighbors/__init__.py 82.23% <100.00%> (+0.18%) ⬆️
src/scanpy/plotting/_tools/__init__.py 76.17% <100.00%> (ø)
src/scanpy/plotting/_tools/paga.py 64.26% <100.00%> (ø)
src/scanpy/preprocessing/_pca/__init__.py 91.92% <100.00%> (-0.05%) ⬇️
src/scanpy/tools/_diffmap.py 82.60% <100.00%> (+0.79%) ⬆️
src/scanpy/tools/_dpt.py 65.84% <100.00%> (+0.23%) ⬆️
src/scanpy/tools/_draw_graph.py 69.86% <100.00%> (+0.84%) ⬆️
... and 11 more

@flying-sheep flying-sheep changed the title WIP base without X_ base without X_ May 11, 2026
@flying-sheep flying-sheep added this to the 1.13.0 milestone May 11, 2026
@flying-sheep flying-sheep marked this pull request as ready for review June 23, 2026 13:32
@flying-sheep flying-sheep requested a review from ilan-gold June 23, 2026 13:33
@flying-sheep flying-sheep changed the title base without X_ feat: base without X_ Jun 23, 2026
@flying-sheep flying-sheep removed the request for review from ilan-gold June 23, 2026 13:57

@ilan-gold ilan-gold left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these the only two places that this is the case anymore i.e., prepend _X? It seems to have been done for PCA, but not UMAP:

key_obsm, key_uns = ("X_umap", "umap") if key_added is None else [key_added] * 2
or am I misreading this?

Comment thread src/scanpy/tools/_utils.py Outdated
@flying-sheep flying-sheep requested a review from ilan-gold June 26, 2026 10:06

@ilan-gold ilan-gold left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only real comment is both concrete and exploratory - I think stronger types make sense here regardless of what I say below (after all, it should be nicer than indexing into a tuple with an integer?).

Furthermore, if we have more strongly typed returns, we could also investigate differing key names more cleanly. So instead of the present controlling one central key i.e., adata.varm["pca"] and adata.obsm["pca"] being identical, it could control all the keys (maybe?). This is really high-level, but my motivating thought here is that the different keys currently have the nice property of being a bit more readable. For example, adata.varm["PCs"], while a little clunky, represents "principal components in gene space" meaningfully (compared to adata.varm["pca"]).

Thoughts?

Comment thread docs/release-notes/4076.feat.md Outdated
Comment thread src/scanpy/tools/_ingest.py Outdated
@flying-sheep

Copy link
Copy Markdown
Member Author

the different keys currently have the nice property of being a bit more readable

I agree, varm["PCs"] is more informative than varm["pca"]. That does apply only

  • to the default (I think it’s cleaner to directly use key_added when specified)
  • to PCA (the others don’t have a varm key)

So the question boils down to:

should we leave differing keys when calling sc.pp.pca() while ScanpyV2Preview is enabled?

Or did I miss something?

@ilan-gold

Copy link
Copy Markdown
Contributor

to PCA (the others don’t have a varm key)

This applies to diffmap too, for example:

https://github.com/scverse/scanpy/pull/4076/changes#diff-f25e128747ab8dc77cf527f9e972df2e5ddb32aa14f037d6d4b2d9219a99f397R70-R76

i.e., evals really mean "eigenvalues" which is different than X_diffmap in obsm

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.

Don't prepend "X_"

2 participants