Repository navigation
fix: avoid reading signal values in time_diff - #665
Merged
gviejo merged 1 commit intoOct 12, 2026
Merged
Conversation
gviejo
approved these changes
Oct 12, 2026
Contributor
|
It's good thanks merging it. |
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.
Why
time_diff()only needs timestamps, but selecting each epoch with.get()also reads signal values. This causes unnecessary I/O for disk-backed time series.This is the separate I/O follow-up requested in #640, without changes to the correlogram API.
What changed
Use the existing
get_slice()interval selection to sliceself.index.valuesdirectly. The signature, epoch boundaries, alignment options, output timestamps, values, and time support are unchanged. The docstring now states that signal values are not loaded.Add 18 HDF5 regression cases covering
Tsd,TsdFrame, andTsdTensor, all three alignments, and default or explicitly supplied disjoint epochs. The tests reject both dataset indexing and array materialization, and check the resulting values, timestamps, and time support.Validation notes
The HDF5 regressions pass with the change and fail on the upstream implementation because it indexes the signal dataset.
The local full doctest run on Python 3.12, NumPy 2.5.3, and SciPy 1.18.1 still encounters the existing full-precision representation mismatch in
apply_hilbert_transform. The same mismatch was reproduced on the base implementation; its documentation fix is not included here.