Skip to content

Multi-segment support for AmplitudesWidget #3801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
jakeswann1 opened this issue Mar 25, 2025 · 4 comments · May be fixed by #3805
Open

Multi-segment support for AmplitudesWidget #3801

jakeswann1 opened this issue Mar 25, 2025 · 4 comments · May be fixed by #3805
Labels
enhancement New feature or request widgets Related to widgets module

Comments

@jakeswann1
Copy link
Contributor

Are there plans to, or has anyone modified the AmplitudesWidget to handle multi-segment sorting analyzers? I'd like to be able to plot unit summaries for my multi-segment analyzers, but currently the AmplitudesWidget can only handle a single segment. Happy to draft a PR for this and incorporate people's thoughts, if we think this would be good to have. I'm imagining a plot across a single set of axes, with vertical dashed lines separating the segment boundaries? Would appreciate opinions on what should be the default setting (segment 0, as now, or all available segments) and generally with any considerations for maintaining backwards compatibility
@chrishalcrow
Copy link
Collaborator

Hey Jake,
There was recently a PR making a BaseRasterWidget, which AmplitudesWidget, RasterWidget, DriftRasterMapWidget and (soon) LocationsWidget inherit from. Just to say: if you can make it work there, it'll automatically work for all those, which would be great!!
I think your suggestion sounds good, and you should have a go at a PR!

@jakeswann1
Copy link
Contributor Author

Sounds good! The segment index param currently only applies to these subclasses, and isn't inherently handled by BaseRasterWidget - would your suggestion be to edit the base class to handle this, and just have the subclasses pass the segment indices on? Or to just handle segment concatenation in each subclass separately. I guess it might need to be the latter given that the various y axis data need to be computed differently, and they all require segment information

@chrishalcrow
Copy link
Collaborator

Hmmm, I reckon I'd edit the base class to handle multi-segments. Maybe allow for spike_train_data and y_axis_data to be dicts of dicts? Then edit the data-wrangling in each subclass, and finally edit the plotting itself in the BaseRasterWidget plotting functions.

Just to note, a few external things like sortingview depends on the Widgets. So, to start, you might have to keep the current behaviour as default and let the user specify that they want to plot multi-segments.

@jakeswann1
Copy link
Contributor Author

#3805 PR drafted - let me know what you think!

@jakeswann1 jakeswann1 linked a pull request Mar 25, 2025 that will close this issue
Open
@zm711 zm711 added enhancement New feature or request widgets Related to widgets module labels Mar 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request widgets Related to widgets module
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add multi-segment capability to BaseRasterWidget and children jakeswann1/spikeinterface
3 participants
@chrishalcrow @jakeswann1 @zm711