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

update time_slice documentation, add additional option for the exact … #4658

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ahnitz
Copy link
Member

@ahnitz ahnitz commented Feb 28, 2024

…index identification threshold

# to be some epsilon different in floating point value.
# (2) identify if we intended the start / end to be at an exact sample
# based on identifying only a trivial difference.
if exact_indices_threshold:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a further explainer why this is needed. One might ask, why not just use the 'nearest' mode. Is that not the same? This block is primarily useful when doing the 'floor' rounding. You don't want it to accidentally round down say a value of 3.9999999 to the previous sample if the difference was just floating point error. If you do then depending on the situation you might get a different length in the same situation. This is intended to make it more robust against this scenario.

@spxiwh
Copy link
Contributor

spxiwh commented Mar 4, 2024

Beyond the bunch of whitespace that needs removing, I'm happy with this, but I think I'd prefer Tito review this as the request for this came from him.

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

Successfully merging this pull request may close these issues.

None yet

2 participants