Respect requested unit order for templates and sparsity in SortingAnalyzer.select_units - #4791
Merged
alejoe91 merged 5 commits intoSep 17, 2026
Conversation
When calling SortingAnalyzer.select_units(), the new analyzer's unit_ids follow the order passed to select_units(), but select_units() copies the templates and the channel sparsity from the parent in the parent's order. The analyzer reads row i of those arrays as unit_ids[i], so a selection that is not in the parent's order gives units each other's templates and sparsity without raising an error. The solution is to index both by ids_to_indices(unit_ids), like the other per-unit extensions.
The selected ACGs were stored under "ccgs", so calling get_data() on an
analyzer produced by select_units() raised KeyError("acgs"). It looks
like this was introduced in SpikeInterface#4307.
(AnalyzerExtensionCommonTestSuite only tested a reversed-order select_units() on unit_locations).
In these cases, mapping unit ids to indices with np.flatnonzero(np.isin(analyzer.unit_ids, unit_ids)) produced correct behavior, but since this is the pattern that caused the bugs fixed two commits prior, it seemed prudent to just use sorting.ids_to_indices(unit_ids) everywhere so there's only one, safe idiom in use.
chrishalcrow
approved these changes
Sep 17, 2026
chrishalcrow
left a comment
chrishalcrow
left a comment
Member
There was a problem hiding this comment.
This is great, thanks Graham!
I stress test all our curation tools in the GUI, which always follows the parent order, so I totally missed this.
alejoe91
approved these changes
Sep 17, 2026
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
3 participants
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.
Commit 1
Bug: When calling
SortingAnalyzer.select_units(), the new analyzer'sunit_idsfollow the order passed toselect_units(), butselect_units()copies the templates and the channel sparsity from the parent in the parent's order. The analyzer reads rowiof those arrays asunit_ids[I], so a selection that is not in the parent's order gives units each other's templates and sparsity without raising an error.Fix: index both by
ids_to_indices(unit_ids), like the other per-unit extensions.Commit 2
Bug: The selected ACGs were stored under
"ccgs", so callingget_data()on an analyzer produced byselect_units()raisedKeyError("acgs").Fix: Store ACGs under "acts".
Commit 3
Problem:
AnalyzerExtensionCommonTestSuiteonly tested a reversed-orderselect_units()onunit_locations.Fix: Add the missing tests.
Commit 4
There were a bunch of places where mapping unit ids to indices with
np.flatnonzero(np.isin(analyzer.unit_ids, unit_ids))produced correct behavior, but since this is the pattern that caused the earlier bug, it seemed prudent to just usesorting.ids_to_indices(unit_ids)everywhere so there's only one, safe idiom in use.