spatial_neigbours extensibility features and clarification by selmanozleyen · Pull Request #1147 · scverse/squidpy

selmanozleyen · 2026-04-02T11:16:36Z

fixes: #1102 and #1047

It's backward compatible and I am curious what the community might bring to this!

I clarified the docs on argument precedence
Added extensibility features
Added tests to ensure the function behaves as written in the docs
No breaking changes

for more information, see https://pre-commit.ci

…leyen/squidpy into feat/spatial_neighbours

for more information, see https://pre-commit.ci

…leyen/squidpy into feat/spatial_neighbours

codecov · 2026-04-02T12:06:15Z

Codecov Report

❌ Patch coverage is 87.46518% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.82%. Comparing base (373228d) to head (13768e2).

Files with missing lines	Patch %	Lines
src/squidpy/gr/_build.py	80.41%	24 Missing and 4 partials ⚠️
src/squidpy/gr/neighbors.py	92.01%	11 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1147      +/-   ##
==========================================
+ Coverage   73.56%   73.82%   +0.26%     
==========================================
  Files          44       45       +1     
  Lines        6929     7098     +169     
  Branches     1174     1178       +4     
==========================================
+ Hits         5097     5240     +143     
- Misses       1347     1369      +22     
- Partials      485      489       +4

Files with missing lines	Coverage Δ
src/squidpy/_docs.py	`94.44% <100.00%> (+0.24%)`	⬆️
src/squidpy/gr/neighbors.py	`92.01% <92.01%> (ø)`
src/squidpy/gr/_build.py	`81.21% <80.41%> (-7.31%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…leyen/squidpy into feat/spatial_neighbours

for more information, see https://pre-commit.ci

…leyen/squidpy into feat/spatial_neighbours

grst

Thanks @selmanozleyen, that goes in the right direction. I put up some design questions up for discussion!

for more information, see https://pre-commit.ci

timtreis · 2026-04-11T16:59:08Z


+    Notes
+    -----
+    ``spatial_neighbors`` has 4 graph-construction modes:


Do we have a good explanation of these somewhere?

timtreis · 2026-04-11T16:59:49Z

+        "`spatial_neighbors_delaunay`, `spatial_neighbors_grid`, or "
+        "`spatial_neighbors_from_builder` instead.",
+        FutureWarning,
+        stacklevel=2,


I think your other future warning was stacklevel=3, how do you decide this?

stacklevel 2: user code -> spatial_neighbors() -> warnings.warn(...)

vs

stacklevel 3: user code -> spatial_neighbors() -> _resolve_graph_builder() -> warnings.warn(...)

Do we have this written down somewhere? I haven't really thought about this yet so I wonder if we should add that to some "how to Squidpy" doc somewhere?

timtreis · 2026-04-11T17:00:11Z

-    }
+@d.dedent
+def spatial_neighbors_from_builder(
+    adata: AnnData | SpatialData,


see other comment

timtreis · 2026-04-11T17:01:00Z

+
+
+def _prepare_spatial_neighbors_input(
+    adata: AnnData | SpatialData,


see other comment

timtreis · 2026-04-11T17:01:21Z

+def _prepare_spatial_neighbors_input(
+    adata: AnnData | SpatialData,
+    *,
+    spatial_key: str,


Align annotations across functions

timtreis · 2026-04-11T17:01:57Z

-    Return D^{-1/2} * A * D^{-1/2}, where D = diag(degrees(A)) and A = adj.
+@d.dedent
+def spatial_neighbors_grid(
+    adata: AnnData | SpatialData,


see other comment

timtreis · 2026-04-11T17:03:10Z

+
+    def __init__(
+        self,
+        radius: float | tuple[float, float] | None = None,


is this used?

yes by _filter_by_radius_interval. If I did this from scratch maybe I'd make it more complicated by having this filtering as a separate transform but this feels more appropiate for old implementation

timtreis · 2026-04-11T17:05:32Z

@@ -0,0 +1,440 @@
+"""Graph construction strategies for spatial neighbor graphs.


Quite some duplicates code, can you use a factory or prototcol instead?

Not sure if I agree. What exactly do you think needs deduplication here?

The two typevars here aren't modified anywhere. It could just be an ABC.

apply_percentile is the only function using the coord_type. Since at the point of _resolve_graph_builder we decide on the downstream path, we don't need that guard. With that guard removed, the coord_type becomes superflous and would further simplify the class.

Both apply_filters are identical, could be deduplicated by moving it into build of the ABC.

KNNBuilder.build_graph and RadiusBuilder.build_graph share quite some overlap in building their nn-graphs. Just the delauny stuff would need to be routed differently.

but I guess the biggest win would be removing the non-CSR build option? Is this sth we'll realistically ever encounter?

The factory/protocol comment doesn't make much sense in hind-sight after having gone through it in detail - still, quite some opportunity to remove LOCs

but I guess the biggest win would be removing the non-CSR build option?

I'd like to also give possibility to support cupy sparse or whatever sparse types jax might have for example

But I noticed I use scipy operations also in the builder path so let me just have a second look.

for more information, see https://pre-commit.ci

* [pre-commit.ci] pre-commit autoupdate updates: - [github.com/biomejs/pre-commit: v2.4.9 → v2.4.10](biomejs/pre-commit@v2.4.9...v2.4.10) - [github.com/tox-dev/pyproject-fmt: v2.20.0 → v2.21.0](tox-dev/pyproject-fmt@v2.20.0...v2.21.0) - [github.com/astral-sh/ruff-pre-commit: v0.15.8 → v0.15.9](astral-sh/ruff-pre-commit@v0.15.8...v0.15.9) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

for more information, see https://pre-commit.ci

shashkat

It looks amazing to me now! Just a few more minor suggestions. I really liked the idea of using specialized Postprocessor classes!

shashkat · 2026-04-20T01:06:55Z

    n_neighs
-        Number of neighboring tiles. Defaults to ``6``.
+        Number of neighboring tiles used to form the base grid connectivity.
+        Defaults to ``6``. On a Visium-like hexagonal grid, ``6`` corresponds to
+        the immediate surrounding spots, while smaller values such as ``3`` make
+        the first-ring graph deliberately sparser.


Maybe it would be useful to mention that there is no guarantee of what subset of neighbors would be chosen from a given ring, if n_neighs is less than the actual number of neighbors of a cell in the grid.
Just like this snippet from the latest-commit documentation of spatial_neighbors function:

- values such as ``n_neighs=4`` and ``n_neighs=6`` are the intended square-grid and hex-grid choices, respectively; - other values are accepted for backward compatibility, but their geometric interpretation is not guaranteed to match a continuous ring on the grid; - no clockwise or other within-ring ordering is part of the public API.

shashkat · 2026-04-20T01:13:35Z

+    def postprocessors(self) -> Sequence[GraphPostprocessor[GraphMatrixT]]:
+        """Return post-build processing steps for ``(adj, dst)``."""
+        return self._postprocessors


I am not sure if this method is offering any functionality.
We can possibly call the attribute _postprocessors as postprocessors and replace line 77 from for postprocessor in self.postprocessors(): to for postprocessor in self.postprocessors:.
What I understand is that this method can allow one to reorder or filter out the entities in _postprocessors attribute. But that can anyways be done when the postprocessors entities are created in the init method of a subclass of GraphBuilder.

shashkat · 2026-04-20T01:15:13Z

    def __init__(
        self,
        transform: str | Transform | None = None,
        set_diag: bool = False,
        percentile: float | None = None,
+        postprocessors: Sequence[GraphPostprocessor[GraphMatrixT]] = (),
    ) -> None:
        self.transform = Transform.NONE if transform is None else Transform(transform)
        self.set_diag = set_diag
        self.percentile = percentile
+        self._postprocessors: list[GraphPostprocessor[GraphMatrixT]] = list(postprocessors)


Is the transform argument used here? I think the transformation requirements would be encoded in postprocessors anyways, so we can remove it.

shashkat · 2026-04-20T01:20:44Z

+        if isinstance(radius, tuple):
+            postprocessors.append(DistanceIntervalPostprocessor(tuple(sorted(radius))))


If someone passes a single float or int to radius, DelaunayBuilder doesn't do anything with it I believe. Only if radius is a tuple[float, float], does it add the filter. I think we can fix this by doing something like the following.
change this:

if isinstance(radius, tuple): postprocessors.append(DistanceIntervalPostprocessor(tuple(sorted(radius))))

to this:

if isinstance(radius, (int, float)): radius = (0, radius) postprocessors.append(DistanceIntervalPostprocessor(tuple(sorted(radius))))

Then, we can also change the type suggestion for radius in spatial_neighbors_delaunay to float | tuple[float, float].

selmanozleyen and others added 9 commits April 2, 2026 13:06

init

5c08081

doc clarifications

5847d84

don't expose the builders

179ba06

[pre-commit.ci] auto fixes from pre-commit.com hooks

e187aba

for more information, see https://pre-commit.ci

deduplicate

cdb5699

Merge branch 'feat/spatial_neighbours' of https://github.com/selmanoz…

847a46b

…leyen/squidpy into feat/spatial_neighbours

[pre-commit.ci] auto fixes from pre-commit.com hooks

69cda1a

for more information, see https://pre-commit.ci

arrange builders

fddc027

Merge branch 'feat/spatial_neighbours' of https://github.com/selmanoz…

a30988a

…leyen/squidpy into feat/spatial_neighbours

selmanozleyen and others added 6 commits April 2, 2026 14:09

better behaviour

f194b10

move classes

ae1eba1

Merge branch 'main' into feat/spatial_neighbours

a86572b

cleanup

039b852

Merge branch 'feat/spatial_neighbours' of https://github.com/selmanoz…

4699993

…leyen/squidpy into feat/spatial_neighbours

[pre-commit.ci] auto fixes from pre-commit.com hooks

b5dcf9f

for more information, see https://pre-commit.ci

selmanozleyen requested a review from grst April 2, 2026 12:30

selmanozleyen added 5 commits April 2, 2026 14:36

add docs

4c11adf

Merge branch 'feat/spatial_neighbours' of https://github.com/selmanoz…

2b937e9

…leyen/squidpy into feat/spatial_neighbours

remove leftover

ea1ce93

resolution in reolve func

ebb7fd5

reduce dup code

69a9394

selmanozleyen requested a review from timtreis April 2, 2026 12:53

selmanozleyen self-assigned this Apr 2, 2026

grst reviewed Apr 2, 2026

View reviewed changes

selmanozleyen and others added 5 commits April 2, 2026 17:11

deprecate invalid n_neighs

77755ed

expose new functions and deprecate the flat one

d8a8145

move branches to classes

2e75b3f

[pre-commit.ci] auto fixes from pre-commit.com hooks

8c1c2ef

for more information, see https://pre-commit.ci

remove radius abstraction

268d30c

timtreis reviewed Apr 11, 2026

View reviewed changes

Comment thread src/squidpy/gr/_build.py Outdated

timtreis reviewed Apr 11, 2026

View reviewed changes

Comment thread src/squidpy/gr/_build.py Outdated

timtreis reviewed Apr 11, 2026

View reviewed changes

Comment thread src/squidpy/gr/_build.py Outdated

timtreis reviewed Apr 11, 2026

View reviewed changes

timtreis requested changes Apr 11, 2026

View reviewed changes

selmanozleyen and others added 16 commits April 13, 2026 07:46

mark as TODO's

5a02b59

rename sugg

3af98cc

add combine into API

e8a19d7

[pre-commit.ci] auto fixes from pre-commit.com hooks

ac682a3

for more information, see https://pre-commit.ci

update docs

0f3329c

[pre-commit.ci] auto fixes from pre-commit.com hooks

00ec2b0

for more information, see https://pre-commit.ci

Functions to QC histopathology images (scverse#1036)

1a87841

remove coord_type abstraction

1cb6fde

[pre-commit.ci] auto fixes from pre-commit.com hooks

8afcd03

for more information, see https://pre-commit.ci

Merge branch 'main' into feat/spatial_neighbours

3c4dffb

make builder positional consistently

21f02c3

make more abstractions

744c757

[pre-commit.ci] auto fixes from pre-commit.com hooks

dbc7f0d

for more information, see https://pre-commit.ci

graphmatrixT exposure

d7e5e5f

[pre-commit.ci] auto fixes from pre-commit.com hooks

13768e2

for more information, see https://pre-commit.ci

selmanozleyen requested a review from timtreis April 13, 2026 22:33

shashkat reviewed Apr 20, 2026

View reviewed changes



		def _prepare_spatial_neighbors_input(
		adata: AnnData \| SpatialData,

		@@ -0,0 +1,440 @@
		"""Graph construction strategies for spatial neighbor graphs.

		if isinstance(radius, tuple):
		postprocessors.append(DistanceIntervalPostprocessor(tuple(sorted(radius))))

Conversation

selmanozleyen commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

grst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timtreis Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shashkat left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

selmanozleyen commented Apr 2, 2026 •

edited

Loading

codecov bot commented Apr 2, 2026 •

edited

Loading

timtreis Apr 11, 2026 •

edited

Loading