scverse / scverse/squidpy

Bugs and problems in calculate_niche and in unreleased main.

Open
#1,293 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
598
Forks
121
Avg merge
3d 11h
Merged PRs (30d)
3

Description

This is NOT AI generated, I wrote these one by one. I usually disclose when I do. So I am a bit confident with these but I kept it brief. So please consult your AI assistant for digging where these items are coming from in the code. I wanted to keep it high level here. This will be a ledger also for the changes https://github.com/scverse/squidpy/pull/1284 introduces.

vs v1.8.3 here is the version before we started with this refactor. So these were what was already broken and require breaking changes:

  • n_hop_weights shorter than distance pads silently, my proposal raises
  • Hop rings in cellcharter tried to do BFS via thresholded matrix powers but it was wrong. I wrote a BFS in numba which made the features differ.
  • cellcharter GMM seed: now one rng spawned per clusterer
  • use_rep skipped the spatial aggregation entirely. (was in 1.8.3 also, hops were computed and ignored completely, now use_rep should only replace the default pca and shouldn't mean anything else, also matches cellcharter). Now it doesn't skip it and the spatial aggregation is done on the scvi embeddings etc when provided. Which should've been the case already
  • n_components used to set GMM(n_components) but also for some reason truncated adata.obsm[use_rep][:,:n_components]? but when use_rep wasn't given, we calculated sc.pp.pca with no n_components passing?? I basically decided to take n_pca_components and n_clusters and not use n_components in the new calculate_niche_cellcharter. Not technically breaking against released version but it is against main (related #831 and #978)
  • mask was in the docs but was actually unused in all but neighborhood which is crazy and shows we are doing good with this splitting refactor.
  • duplicate, empty and cross-flavor resolutions failed silently or obscurely. [0.5, 0.5] produced one column instead of two with no warning, a tuple reached scanpy as must be real number, not tuple, and an ndarray as only 0-dimensional arrays...

vs main

  • remove prefix removed from calculate_niche_spatialleiden. Never released, so no deprecation

(still in progress of finding the bugs)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the calculate_niche entry points, especially calculate_niche_cellcharter and calculate_niche_spatialleiden, then compare v1.8.3 with main and review PR #1284. Trace the listed behavior around hop weights, BFS rings, GMM seeding, use_rep, component counts, masks, resolutions, and the removed prefix. Done means the reported discrepancies are resolved or explicitly accounted for without silent failures.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
data, machine-learning
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.