Skip to content

Fix ks4 tests and support ks4>=4.0.34#3900

Merged
alejoe91 merged 4 commits intoSpikeInterface:mainfrom
alejoe91:new-ks-versions
May 6, 2025
Merged

Fix ks4 tests and support ks4>=4.0.34#3900
alejoe91 merged 4 commits intoSpikeInterface:mainfrom
alejoe91:new-ks-versions

Conversation

@alejoe91
Copy link
Copy Markdown
Member

@alejoe91 alejoe91 commented May 3, 2025

@alejoe91 alejoe91 added the sorters Related to sorters module label May 3, 2025
@alejoe91 alejoe91 added this to the 0.102.3 milestone May 3, 2025
Copy link
Copy Markdown
Member

@zm711 zm711 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me and tests are passing :)

I only wish we could find a way to help deal with the changing number of arguments. Your idea of setting a kwargs_dict seems cool, I wonder if we can apply that in more places such that the dict changes with the versions so when we unpack it, it returns the correct things. Maybe for a future PR we think about that :)


if self.whiten_mat is not None:
pass
return X
Copy link
Copy Markdown
Member

@zm711 zm711 May 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is brutal..... Sorry. didn't highlight what I wanted it to. I meant the fact you had to duplicate the function because an used argument change.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully this doesn't happen too soon! Anyways, kudos to @JoeZiminski since the tests to check the arguments of different KS functions are very useful to understand what's changed!

@alejoe91 alejoe91 merged commit 4e90730 into SpikeInterface:main May 6, 2025
35 checks passed
@alejoe91 alejoe91 deleted the new-ks-versions branch March 20, 2026 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sorters Related to sorters module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Kilosort v4.0.34 added shank_idx to set_files(), which breaks support with spikeinterface

2 participants