Skip to content

bugfix in _calculate_neighborhood_profile in gr/_niche.py - #1229

Closed
shashkat wants to merge 2 commits into
scverse:mainfrom
shashkat:bugfix_nhood_profile
Closed

bugfix in _calculate_neighborhood_profile in gr/_niche.py#1229
shashkat wants to merge 2 commits into
scverse:mainfrom
shashkat:bugfix_nhood_profile

Conversation

@shashkat

Copy link
Copy Markdown
Contributor

Description

I am currently working on refactoring the niche code and API to make more modular similar to how it was done for graph construction strategies in #1147 and discussed in #1192. However, before that, in a separate, smaller PR, I wanted to make aware of a bug in the niche code, and fix it. This is that PR.

The bug was in the function _calculate_neighborhood_profile() in gr/_niche.py. This is called when calculate_niche() is called with flavor = 'neighborhood', inside _get_nhood_profile_niches(). The goal of _calculate_neighborhood_profile() is to get a dataframe of shape num_cells x num_categories (categories could be something like celltype), where each entry stores the number of neighbors of a cell of a particular category. However, it wasn't working correctly when different cells had different number of neighbors. I believe it was written to be meant for grid-based data only.
Now, it handles correctly the case when cells have different number of neighbors.

How has this been tested?

I created a new dummy adata object (dummy_adata2 in tests/conftest.py) with explicitly defined locations of the cells, not necessarily organized in a grid. Then, I determined how the neighborhood profile for this object would look like and wrote a test function accordingly in tests/graph/test_niche.py called test_calculate_neighborhood_profile(). It failed with the original version of _calculate_neighborhood_profile() due to incorrect values and passed with the new version.

Other points

  • The purpose of this PR is to just make aware of this bug and fix it first. But it is not gonna be the final version of this function as there would be many changes with the niche-refactor PR.
  • I also believe that dummy_adata2 object can be useful for writing other tests cases too in the future for niche code and possibly graph building code too.

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.30435% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.23%. Comparing base (ac08971) to head (5a010bc).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/squidpy/gr/_niche.py 91.30% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1229      +/-   ##
==========================================
+ Coverage   77.08%   77.23%   +0.15%     
==========================================
  Files          63       63              
  Lines        9368     9384      +16     
  Branches     1576     1583       +7     
==========================================
+ Hits         7221     7248      +27     
+ Misses       1545     1532      -13     
- Partials      602      604       +2     
Files with missing lines Coverage Δ
src/squidpy/gr/_niche.py 38.01% <91.30%> (+0.48%) ⬆️

... and 1 file with indirect coverage changes

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

@selmanozleyen selmanozleyen self-assigned this Jun 25, 2026
@selmanozleyen
selmanozleyen self-requested a review June 26, 2026 22:38
@selmanozleyen

Copy link
Copy Markdown
Member

closing this since it's included in #1245

selmanozleyen added a commit that referenced this pull request Aug 11, 2026
* added changes from the bugfix in nhood profile (#1229)

* adding sanitize_adata before updating table in case of sdata in calculate_niche() so that writing test case for sdata is possible

* added some basic test cases for calculate_niche() before refactoring it

* updated the required, optional and unused params for different flavors

* initial framework for the niche refactor

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

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

* functions and classes for niche refactor

* minor  naming change in test_niche_calc_library_key_dummy_adata() as new implementation has differently named column

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

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

* reduced _calculate_neighborhood_profile arguments as it takes from class attributes

* Fix dummy adata test cases

Tests in test_niche.py involving dummy_adata fail with
scanpy<1.12.1 because of a change in handling of small edge
weights by sc.pp.neighbors (scverse#4031). Updated the values
in tests to match the expected values with scanpy>=1.12.1

* fix old test cases

* fixed spatialleiden not available in test envs

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

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

* warn before adding new column in adata.obs

* efficient handling of inplace arg in calculate_niche_custom and calculate_niche_spatialleiden

* fixed some issues with spatialleiden flavor and added test case for spatialleiden with library_key not None

* added docs for functions and classes

* type-suggestions, sensible defaults of args and exports done

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

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

* minor fixes

* small change in MaskPostprocessor and added a test case for postprocessors

* minor documentation update

* added niche functions and classes imports

* simplify

* sinmplifcation

* add niche-related functions and update documentation

* restore defaults

* docs

* remove lock

* deprecate calculate_niche in favor of flavor functions

* make custom niche pipeline private

* remove private tests and classes also prefixed

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: selmanozleyen <syozleyen@gmail.com>
Co-authored-by: Selman Özleyen <32667648+selmanozleyen@users.noreply.github.com>
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.

2 participants