Skip to content
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.

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

Add merge_aligned_spans #22

Open
ericphanson opened this issue Sep 27, 2023 · 0 comments
Open

Add merge_aligned_spans #22

ericphanson opened this issue Sep 27, 2023 · 0 comments

Comments

@ericphanson
Copy link
Member

This would be https://github.com/beacon-biosignals/TimeSpans.jl/blob/9f80d852a51b6df3d263b848c51c306c0d43fa40/src/TimeSpans.jl#L311-L365C72 but with a new shortest_aligned_span_containing instead of shortest_timespan_containing, and with checks that all spans have the same sample rate, and using by=x -> x.first_index instead of by=start where applicable.

If indeed all spans have the same sample rate, this should be the same as

merge_aligned_spans(f, aligned_spans) = AlignedSpan.(sample_rate, merge_spans(f, aligned_spans))

except more efficient and such that f gets aligned spans rather than timespans (or a mix of aligned spans and timespans).

Alternatively, we could try to update TimeSpans.jl to allow shortest_timespan_containing to be configurable in merge_spans, and could pass in our own shortest_aligned_span_containing instead. (And just pay the time for computing times with start rather than first_index, but that's probably fine).

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

No branches or pull requests

1 participant