Repository navigation
fix: set NaN density for tiny or degenerate groups in tl.embedding_density - #4431
Open
monsterlag wants to merge 3 commits into
Open
monsterlag wants to merge 3 commits into
monsterlag wants to merge 3 commits into
Conversation
…ensity` Categories with fewer than 3 cells, or whose cells lie on one line or point in the embedding, made `gaussian_kde` raise. They now get a density of NaN and a single warning names them. Fixes scverse#2043
monsterlag
marked this pull request as ready for review
October 22, 2026 17:55
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
AI-assisted. I wrote this change with an AI coding agent, and a separate AI review agent re-ran the tests described below. I review and test the final diff myself before marking it ready for review.
LLM usage
LLM or coding agent used: Claude Opus 5.5 via Claude Code
If one was used, I, a human, have reviewed, understood and tested every changed line
Closes sc.tl.embedding_density errors when a category has one observation #2043
Tests included or not required because:
Release notes not necessary because:
What was wrong
sc.tl.embedding_density(adata, groupby=...)runsscipy.stats.gaussian_kdeonce per category.A 2D KDE needs at least 3 points that do not all lie on one line or point.
Otherwise
gaussian_kderaises:ValueErrorfor a 1-cell category,LinAlgError("singular data covariance matrix") for 2 cells, or for 3+ cells that are collinear or identical (which identical expression profiles can produce in e.g. diffmap/PCA coordinates).One such category made the whole call fail.
The change
groupbyloop, a category with fewer than 3 cells, or whose KDE raisesnp.linalg.LinAlgError, getsNaNfor its cells.UserWarninglists those categories ("have too few cells or degenerate coordinates").NaN.PLR0915to the existing# noqa: PLR0912onembedding_density; the extra lines push it over ruff's statement limit (same pattern as e.g.marker_gene_overlap).docs/release-notes/.The
groupby=Nonepath is unchanged.Tests
New
test_embedding_density_groupby_degenerate_group, parametrized over a small category of 1 cell, 2 cells, 3 collinear cells and 3 identical cells, next to a 30-cell category.It expects a warning naming the small category,
NaNfor its cells, and values in [0, 1] for the large one.Commands run (Python 3.13, scipy 1.18.1, numpy 2.5.4):
python -m pytest tests/test_embedding_density.py -qagainst themainsource: the 4 new cases fail (ValueErrorfor 1 cell,LinAlgErrorfor 3 collinear and 3 identical, and for 2 cells aRuntimeWarning: invalid value encountered in dividethat the test config turns into an error); the 4 existing tests pass.prek run --fileson the three changed files: all hooks pass.towncrier build --draft: the fragment renders under "Bug fixes".sc.pl.embedding_densityruns forgroup="all",group="b"andgroup=["c"]. In the legacy plot, theNaNcells are drawn light gray (#d3d3d3, the same colour as the out-of-group background) at the foreground dot size.Open questions
NaNplus a warning the behaviour you want here, or would you rather raise a clearer error?NaNand listed in the warning. Before this change they raised as well.NaNgroup has the same colour as the background and stands out only by its larger dot size. The v2 (holoviews) plot was not checked.