Custom neighbor transformers with varying neighbor count and respect knn#3807
Draft
maltekuehl wants to merge 6 commits intoscverse:mainfrom
Draft
Custom neighbor transformers with varying neighbor count and respect knn#3807maltekuehl wants to merge 6 commits intoscverse:mainfrom
knn#3807maltekuehl wants to merge 6 commits intoscverse:mainfrom
Conversation
3 tasks
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3807 +/- ##
==========================================
+ Coverage 76.72% 76.85% +0.13%
==========================================
Files 116 116
Lines 12398 12413 +15
==========================================
+ Hits 9512 9540 +28
+ Misses 2886 2873 -13
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allows neighbor counts < k in
sc.pp.neighborsby adding masking to_ind_dist_slow, enabling adaptive neighbor transformers. Draft for now, as I could not run tests locally or on CI (following the exact installation instructions and not touching the GitHub actions, so it seems like the workflow is broken). I would also appreciate help in making the tests more idiomatic to scanpy and getting some feedback on if this could break any other part that might rely on connectivities having a certain shape.To make it work, I also had to take on #3014 (more discussion in that issue). Adding a
binaryconnectivity method like in this PR would be backwards compatible and unlock many different use cases. In my opinion, the current implementation is however quite confusing for end users and some documentation clarity, simplification and warning about unexpected behavior when usingknnwithmethod="umap"would be helpful. Since some of this could result in breaking changes, this PR can hopefully also serve as a place for discussion on how to implement this.The second part has also been discussed in:
sc.pp.neighbors#2587#3021 might also be related to the overall complex structure with multiple distance calculations and changes to the connectivities in
neighbors, though I have not been able to confirm it yet.Use cases of these changes:
Full comparability with use of transformers outside of scanpy
SNN transformers
Distance adaptive algorithms
Less duplication when implemeting methods like
bbknn, etc.Closes Custom neighbor transformers with variable neighbor count <= k fail #3806 and closes sc.pp.neighbors(..., knn=True, n_neighbors=k) does not threshold the adjacency #3014
Tests included: Needs additional tests to verify
n_neighborsin distances and connectivities for different methods and different n_obs (this test might have prevented Unexpected number of non-zero distances when runningsc.pp.neighborswithtransformer="pynndescent"#3809)