Skip to content

Conversation

@melocery
Copy link
Contributor

Close #879

Added support for spatialleiden as a flavor in sq.gr.calculate_niche, following up on the idea mentioned in #879. The method is now integrated via the flavor="spatialleiden" option, wrapping the spatialleiden.spatialleiden() function.

@melocery
Copy link
Contributor Author

@niklasmueboe FYI as the author of spatialleiden.

This PR adds spatialleiden as an option in sq.gr.calculate_niche() via the flavor argument.
Note: spatialleiden depends on leidenalg, which was removed as a hard dependency in PR #972 to keep BSD3 compliance. So, there might be some overlap or conflicts to consider regarding dependencies.

@codecov-commenter
Copy link

codecov-commenter commented Jul 31, 2025

Codecov Report

❌ Patch coverage is 16.00000% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.14%. Comparing base (d4f256f) to head (44817e7).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/squidpy/gr/_niche.py 14.28% 34 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1019      +/-   ##
==========================================
- Coverage   66.45%   66.14%   -0.31%     
==========================================
  Files          44       44              
  Lines        7115     7158      +43     
  Branches     1199     1218      +19     
==========================================
+ Hits         4728     4735       +7     
- Misses       1913     1944      +31     
- Partials      474      479       +5     
Files with missing lines Coverage Δ
src/squidpy/_constants/_constants.py 100.00% <100.00%> (ø)
src/squidpy/gr/_niche.py 32.28% <14.28%> (-2.23%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@LLehner
Copy link
Member

LLehner commented Nov 20, 2025

Hi @melocery sorry for my late response. We were a bit inactive regarding the update of the niche function, but we are getting back into it now. This looks like a great PR and was certainly needed! Thank you for the contribution.

Will check it asap. Shouldn't take too long to merge.

@LLehner LLehner self-assigned this Nov 20, 2025
@LLehner LLehner added graph 🕸️ feature PR introduces a new feature labels Nov 20, 2025
@timtreis
Copy link
Member

Hey @melocery, thank you for PR! However, there's a licensing issue with it, so we can't accept it in this state.

Your PR adds a hard dependency on SpatialLeiden. While it mostly mirrors our dependencies, it has leidenalg as a hard dependency, which is licensed under GPL3. GPL3 is a copy-left license, and therefore your package is actually also GPL3 (not MIT), and ours would be too. That'd prevent most commercial entities from using Squidpy, which we don't want to prevent.

For us to depend on SpatialLeiden, you'd have to find a way how people can install and run your algorithm without the leidenalg package.

@niklasmueboe
Copy link

... it has leidenalg as a hard dependency, which is licensed under GPL3. GPL3 is a copy-left license, and therefore your package is actually also GPL3 (not MIT) ...

Thanks for the hint, I didn't properly check the licenses before apparently.

Another question (that may be misplaced here but is still in scope of the scverse); scanpy is also BSD3 but does also depend on leidenalg and igraph which are both GPL licensed. Is this because they are only used in optional dependencies?

For us to depend on SpatialLeiden, you'd have to find a way how people can install and run your algorithm without the leidenalg package.

Currently, i don't think there is a way short of a reimplementation of leidenalg. The Leiden implementation in igraph does not implement the necessary functionality and even if it did it is also GPL licensed.

@timtreis
Copy link
Member

scanpy is also BSD3 but does also depend on leidenalg and igraph which are both GPL licensed. Is this because they are only used in optional dependencies?

Yes, exactly 👍

I just pushed some changes that make it an optional dependency, similar to how scanpy handles it. I'll play around with it a bit and then give feedback.

@niklasmueboe
Copy link

@timtreis before you merge, I would push a new spatialleiden version today reflecting the license changes (otherwise it will be the same).
Maybe it then makes sense to include a dependency specifier in the pyproject.toml as either >=0.4.0 or ~=0.4.0 (depending on your preferences/conventions) to make sure the version with the correct license is pulled

@niklasmueboe
Copy link

Ok, so the new release is published to PyPI (conda will follow soon).

@niklasmueboe
Copy link

The package is available on both PyPI and bioconda.

Is there anything I can support you with from my side?

@timtreis timtreis merged commit 538427b into scverse:main Dec 17, 2025
13 checks passed
@timtreis
Copy link
Member

merged! Thank you for the contribution @niklasmueboe !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature PR introduces a new feature graph 🕸️

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add SpatialLeiden as clustering method

6 participants