Let callers override fs/t0, and defer to SI's own clock when unset - #57
Open
oliche wants to merge 2 commits into
Open
Let callers override fs/t0, and defer to SI's own clock when unset#57oliche wants to merge 2 commits into
oliche wants to merge 2 commits into
Conversation
Adds a constructor-level fs/t0 override to AbstractDataModel, resolved once in get_sampling_frequency()/get_t0() (marked @Final) ahead of a per-backend _get_metadata_sampling_frequency()/_get_metadata_t0(). SpikeGLXDataModel, LFPackDataModel and SpikeInterfaceDataModel all gain fs/t0 kwargs; the GUI threads them from EphysBinViewer/LFPackBinViewer down to every data model constructed when opening a file or switching an lfpack recording. LFPackDataModel also gains a real _get_metadata_t0(), reading the sync-corrected LFPackReader.t0 (falling back to zero when there's no sync data, i.e. NaN) instead of silently treating every lfpack file as starting at zero.
Addresses Joe's PR #54 review comment: the base-class linear t0+sample/fs model should not silently override a user's own set_times() time vector on a SpikeInterface recording. Splits get_time_from_sample/get_sample_from_time so the linear model only applies when the caller has explicitly overridden fs/t0; otherwise they defer to a new _get_metadata_time_from_sample/_get_metadata_sample_from_time hook, whose default is still the linear metadata model (correct for SpikeGLX and LFPack) but which SpikeInterfaceDataModel overrides to call sample_index_to_time/time_to_sample_index directly, honoring a genuinely non-uniform SI clock exactly. An explicit fs/t0 override still forces the synthetic linear clock, since that's what asking for one means.
oliche
force-pushed
the
si-times-fs-t0-override
branch
from
August 8, 2026 03:00
b838ee7 to
2cec7ba
Compare
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
1 participant
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.
Summary
fs/t0override toAbstractDataModel, resolved viaget_sampling_frequency()/get_t0()ahead of a per-backend_get_metadata_*hook.SpikeGLXDataModel,LFPackDataModel, andSpikeInterfaceDataModelall take optionalfs/t0kwargs; the GUI threads them fromEphysBinViewer/LFPackBinViewerdown to every data model constructed when opening a file or switching an lfpack recording.LFPackDataModelgains a real_get_metadata_t0(), reading the sync-correctedLFPackReader.t0(falling back to zero when there's no sync data) instead of silently treating every lfpack file as starting at zero.get_time_from_sample/get_sample_from_timeonly use the lineart0 + sample/fsmodel when a caller explicitly overridesfs/t0. Otherwise they defer to_get_metadata_time_from_sample/_get_metadata_sample_from_time, whose default is still linear (correct for SpikeGLX/lfpack) but whichSpikeInterfaceDataModeloverrides to callsample_index_to_time/time_to_sample_indexdirectly — so a recording with a genuinely non-uniformset_times()vector still reports its true times.Test plan
uv run --with pytest-qt --with pyqt5 python -m pytest src/viewephys/tests/ -q— 61 passed.ruff check/ruff format --diffclean.t0sourced from sync metadata vs. defaulting to zero, and a SpikeInterface recording with a non-uniformset_times()vector (constant shift + per-sample ramp) confirming conversion matches SpikeInterface's own functions exactly rather than the linear approximation, and that an explicit override still forces the linear model.