Skip to content

fix: ensure radius values stored in AnnData.uns are writable - #1249

Merged
timtreis merged 8 commits into
scverse:mainfrom
selmanozleyen:fix/neighbours-uns-writable
Aug 11, 2026
Merged

fix: ensure radius values stored in AnnData.uns are writable#1249
timtreis merged 8 commits into
scverse:mainfrom
selmanozleyen:fix/neighbours-uns-writable

Conversation

@selmanozleyen

Copy link
Copy Markdown
Member

Closes #1247

RadiusBuilder and DelaunayBuilder stored an interval radius in
adata.uns['spatial_neighbors']['params'] as the tuple it was passed as.
anndata has no I/O spec for tuples, so any object built with a tuple radius
raised IORegistryError on write_h5ad / write_zarr.

Store a list instead, via a small _radius_to_uns helper. Scalar radii are
unchanged. Note that DelaunayBuilder already normalizes a scalar r to
(0.0, r) in __init__, so spatial_neighbors_delaunay(radius=5.0) now records
[0.0, 5.0] — the same value as before, just writable.

The abstract GraphBuilder.uns_params docstring now states that values must be
writable by anndata, so custom builders don't reintroduce the same problem.

Regression test covers both builders with scalar and interval radii, and asserts
on an actual write_h5ad rather than only on the stored value.

@selmanozleyen
selmanozleyen requested a review from timtreis August 2, 2026 20:30
@codecov

codecov Bot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.55%. Comparing base (9b81fe7) to head (8e9fa54).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1249      +/-   ##
==========================================
+ Coverage   77.08%   78.55%   +1.47%     
==========================================
  Files          63       63              
  Lines        9436     9505      +69     
  Branches     1585     1586       +1     
==========================================
+ Hits         7274     7467     +193     
+ Misses       1561     1448     -113     
+ Partials      601      590      -11     
Files with missing lines Coverage Δ
src/squidpy/gr/neighbors.py 91.66% <100.00%> (+0.87%) ⬆️

... and 3 files with indirect coverage changes

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

@timtreis timtreis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

Comment thread src/squidpy/gr/neighbors.py Outdated
GraphPostprocessor = Callable[[GraphMatrixT, GraphMatrixT], tuple[GraphMatrixT, GraphMatrixT]]


def _radius_to_uns(radius: float | tuple[float, float] | None) -> float | list[float] | None:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For clarification - we're doing this do stay backwards compatible? I'm wondering if we need this converter at all or if we should just fix it at creation time.

@timtreis
timtreis merged commit d2cd242 into scverse:main Aug 11, 2026
23 of 24 checks passed
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.

radius tuple passed to spatial_neighbors_delaunay cannot be stored in adata.uns

2 participants