Skip to content

Add model layer#30

Open
JoeZiminski wants to merge 7 commits into
mainfrom
add-model-layer
Open

Add model layer#30
JoeZiminski wants to merge 7 commits into
mainfrom
add-model-layer

Conversation

@JoeZiminski
Copy link
Copy Markdown
Collaborator

@JoeZiminski JoeZiminski commented May 27, 2026

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 for viewephys but 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_horizontalSliderReleased does a bit of juggling of the raw data, 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:

    if isinstance(self.data, SpileGLXDataModel):
        raw = self.data.get_raw(...)
    elif isinstance(self.data, SpikeInterfaceDataModel)::
        raw = None
    

    as the spikeinterface reader would not have need of the raw data, though this would then result in a dummy variable called to SpikeInterfaceDataModel

  • maybe self.interface and DataInterface is a better name than self.data, DataModel as you already have the model construction in easyqc and that usage more neatly fits to MVC.

Comment thread src/viewephys/data_model.py Outdated
Comment thread src/viewephys/data_model.py
@JoeZiminski JoeZiminski requested a review from oliche May 28, 2026 17:44
@JoeZiminski
Copy link
Copy Markdown
Collaborator Author

Hey @oliche thanks for the review! I have added the type hints. I'll make an issue to convert the geometry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants