scverse / squidpy

Spatial Single Cell Analysis in Python
https://squidpy.readthedocs.io/en/stable/
BSD 3-Clause "New" or "Revised" License
440 stars 82 forks source link

Unexpected behavior of `gr.spatial_neighbors` #735

Closed almaan closed 1 year ago

almaan commented 1 year ago

Description

Hej!

When using the function sq.gr.spatial_neighbors and specifying n_neighs=k with radius=None, my expectation is that the resulting connectivity matrix would give me the connectivities s.t. each cell is connected to its n_neighs nearest neighbors. That is, if I were to sum the connectivity matrix along the second dimension (a.k.a. along the cell axis), it should return k for every cell. However, this is not the case - upon inspection, the function returns a varying number of neighbors for the different cells.

I'm not sure if this is an intended feature or a bug, but from reading the documentation I wouldn't consider this as the expected outcome.

...

Minimal reproducible example

# Import packages

import squidpy as sq
import anndata as ad
import numpy as np

# set random seed
np.random.seed(42)

# create dummy data

adata = ad.AnnData(shape=(1000,1))
adata.obsm['spatial'] = np.random.random(size = (1000,2))

# get spatial connectivities
k = 10
sq.gr.spatial_neighbors(adata, n_neighs=k, coord_type='generic', radius = None)

# get and count connectivities for each cell
gr = adata.obsp['spatial_connectivities']
nn = np.array(gr.todense().sum(axis=1)).flatten()

# check if neighbors are equal to k
np.testing.assert_equal(nn,k)

Version

1.3.0

...

Cheers, Alma

eroell commented 1 year ago

See a response to closely related issue on scanpy.

giovp commented 1 year ago

thanks @eroell that's indeed the explanation. @almaan for the arguments you selected coord_type='generic', radius = None it's indeed just a call to NNeighbors from sklearn and then resetting the diagonal as explained by @eroell .

almaan commented 1 year ago

Hej @giovp ,

I'm a bit late catching up with this, but @eroell 's explanation made tons of sense - it was just me being confused with the names.

Thanks!

almaan commented 1 year ago

Hej @giovp ,

I'm a bit late catching up with this, but @eroell 's explanation made tons of sense - it was just me being confused with the names.

Thanks!

giovp commented 9 months ago

hi @almaan ,

there was indeed a bug in the construction of the spatial graph in knn mode, I've just made a new release with #792 included, that should solve the error you found, which was indeed not related to https://github.com/scverse/scanpy/issues/2587