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

Suspected implementation bug in index_from_time(sample_rate, span::TimeSpan) #28

Closed
ericphanson opened this issue Jan 25, 2022 · 0 comments

Comments

@ericphanson
Copy link
Member

ericphanson commented Jan 25, 2022

The implementation of the semantics isn't correct when the right endpoint of the span doesn't fall exactly on a sample; we always throw away the right-endpoint even when it is well within the span:

j = i == j ? j : (j - 1)

(unless that would make an empty interval, in which case we keep it).

@kleinschmidt's suggestion

I think the solution might be to convert the final index back to time, and see if it needs to be adjusted down by 1 (e.g., if time_from_index(rate, index_from_time(rate, stop(span))) == stop(span) then you use j-1, otherwise use j

would fix that, I think.

Originally posted by @ericphanson in #26 (comment)

ericphanson added a commit that referenced this issue Jan 28, 2022
ericphanson added a commit that referenced this issue Jan 31, 2022
This reverts commit b938543.
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