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

feat: Floor division between TimeDeltas #197

Open
injust opened this issue Jan 8, 2025 · 7 comments
Open

feat: Floor division between TimeDeltas #197

injust opened this issue Jan 8, 2025 · 7 comments
Labels
enhancement New feature or request

Comments

@injust
Copy link
Contributor

injust commented Jan 8, 2025

It would be useful to support floor division between TimeDeltas, like how it works with datetime.timedelta.

TimeDelta implements __truediv__ right now, but not __div__.

@ariebovenberg
Copy link
Owner

Agree, this makes sense. Let me know if you'd like to take a stab at it yourself. Otherwise, I'll include it in an upcoming release.

@ariebovenberg ariebovenberg added the enhancement New feature or request label Jan 8, 2025
@ariebovenberg
Copy link
Owner

ariebovenberg commented Jan 8, 2025

It'd also be consistent to implement __floordiv__ for int and float.

That said, it may also be worth reconsidering whether __truediv__ even makes sense for int and float considering a timedelta always needs to be truncated to the nearest micro/nanosecond.

Observe this potentially confusing behavior in the stdlib:

>>> timedelta(microseconds=4) / 3
timedelta(microseconds=1)
>>> timedelta(microseconds=4) / 7
timedelta(microseconds=1)
>>> timedelta(microseconds=4) / 8
timedelta(microseconds=0)

Perhaps only __floordiv__ should be available for timedelta/number divisions 🤔

@injust
Copy link
Contributor Author

injust commented Jan 8, 2025

I feel like timedelta // number isn't a very well defined operation -- you're flooring, but to the nearest what?

Perhaps only __floordiv__ should be available for timedelta/number divisions 🤔

Hmm, that seems more confusing.

@ariebovenberg
Copy link
Owner

ariebovenberg commented Jan 8, 2025

I feel like timedelta // number isn't a very well defined operation -- you're flooring, but to the nearest what?

It'd truncate to nanoseconds, but you're right that this is probably too implicit.


In theory, I think the purest solution is to have a method like TimeDelta.div(n, /, *, round_mode, round_unit="nanosecond") where round mode can be any of floor, ceil, half_floor, half_ceil, half_even

For the users that value practicality over purity, the / operator can remain available, with a big fat warning about its limitations due to nanasecond truncation.

@injust
Copy link
Contributor Author

injust commented Jan 8, 2025

Let me know if you'd like to take a stab at it yourself. Otherwise, I'll include it in an upcoming release.

I probably won't be able to work on a PR for this.

In theory, I think the purest solution is to have a method like TimeDelta.div(n, /, *, round_mode, round_unit="nanosecond") where round mode can be any of floor, ceil, half_floor, half_ceil, half_even

I would rather leave the timedelta / number division to the / operator, and move rounding into a separate method.

Something like (hours(12) / 7).round(mode="floor", unit="seconds") seems nice.

@ariebovenberg
Copy link
Owner

ariebovenberg commented Jan 8, 2025

I would rather leave the timedelta / number division to the / operator, and move rounding into a separate method.

There would be a separate round() added anyway. But unfortunately you'd strictly need one built in to the division operator as well to customize the rounding of nanoseconds. (e.g. what is nanoseconds(3) / 2, 0.00000001 or 0.000000002?)

edit: although...maybe Python's default rounding is good enough, and if somebody wants to round differently at nanosecond level, they should just use ints/floats 🤔 ...not worth adding a separate method indeed...

@injust
Copy link
Contributor Author

injust commented Jan 8, 2025

edit: although...maybe Python's default rounding is good enough, and if somebody wants to round differently at nanosecond level, they should just use ints/floats 🤔 ...not worth adding a separate method indeed...

I'm inclined to agree with this, but I haven't worked with anything nanosecond-precision so I can't comment on that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants