Add model layer#30
Open
JoeZiminski wants to merge 7 commits into
Open
Conversation
oliche
reviewed
May 28, 2026
Collaborator
Author
|
Hey @oliche thanks for the review! I have added the type hints. I'll make an issue to convert the geometry |
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.
This PR exposes an interface class that is planned to also support a spikeinterface backend. The idea is to introduce an interface layer (
SpikeGLXDataModel) that has all necessary public functions forviewephysbut wraps calls to the spikeglx reader. For this PR its functionally redundant, but I will follow up with a PR that extends it to spikeinterface.Some things to discuss:
At the moment
on_horizontalSliderReleaseddoes a bit of juggling of therawdata, to avoid repeated copies as in the old implementation. For spikeinterface, this pattern does not make sense and so the easiest fix from an interface perspective is to get the raw data once for each viewer. In practice I'm not sure if this adds any overhead, I assume that when a mem-mapped chunk is accessed it is copied into the relevant virtual memory address and is available going forward without reloading. Maybe in practice however it does not persisnt beyond the garbage collection of the relevant variable. I'm not 100% sure what numpy is doing there.Nonetheless even though there will be multiple data access with potential copies from hard disk to RAM, for small data snippets this shouldn't be an issue? Alternatively we can just have a conditional like:
as the spikeinterface reader would not have need of the raw data, though this would then result in a dummy variable called to
SpikeInterfaceDataModelmaybe
self.interfaceandDataInterfaceis a better name thanself.data,DataModelas you already have the model construction ineasyqcand that usage more neatly fits to MVC.