-
Notifications
You must be signed in to change notification settings - Fork 18k
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
proposal: time: Duration should have .Ceil method #39726
Comments
Can you elaborate on this use case? I'm having a hard time envisioning what exactly you mean. |
@JAicewizard can you respond to @mdempsky's question? I too am having a little bit of trouble understanding why this is important to add to the API. |
I believe OP wants to replace this code with a method
|
Imagine you have a task taking
This would all be solved by being able to round up to the neared multiple of Y instead of just being able to round down/nearest. |
I'm sorry but I still don't understand. It still seems like very hypothetical. Can you give a more concrete reason why this would come up in a real Go program? Note that |
This is a real world example of where this is usefull. lesson.startTime/endTime are just timestamps and their week/year is irrelevant. Here we calculate how much these lesson.startTime/endTime should be offset by in order to be the first occurance of this timerange withing this term start/end time range.
This is an example of a real world program, or well if it ware not scraped it would be running in the real world. You are right that |
OK, I think I understand what you are trying to write. Thanks for bearing with me. I wrote a running demo at https://play.golang.org/p/GUreZdb5pYI:
Note that while it works on the playground in UTC, it actually fails on my local system, in that the time of the event changes from 14:30 to 15:30 because of a daylight savings change. Not all weeks are 247time.Hour long. Note also that (d+D-1).Truncate(D) is not even correct when d < 0, so if we change example to be the year 2100, first will end up being August 14 instead of August 7. (d+D-1).Floor(d) would be correct, but we don't have Floor, only Truncate. It would be weird to have Ceil and Truncate but not Floor, so really this proposal would be to add both Ceil and Floor. And I'm also not convinced Ceil and Floor would be the right names since those are so tied to real -> integer conversions. It would be better to call them RoundUp and RoundDown. So Duration would have Truncate, Round, RoundUp, and RoundDown. It's not too bad to consider adding those two extra methods. They are simple enough. But since every new method incurs cost for users reading docs and learning the APIs, I'd still like to understand whether there's a widespread need, ideally in an example that doesn't depend on daylight savings time not happening. I looked at C#, Java, and Python to see if there was evidence of need in other languages, and as far as I can tell those all don't have any kind of rounding methods whatsoever. |
I dont know if there is a widespread need, but considering there is a
Wouldnt daylight savings be a new timezone (in the go timezone sense)? ₣om what I can see a go timezone is always the same offset to GMT. |
Go timezones do not have a fixed offset from GMT. They do support daylight savings time. As the docs say: "For many Locations the time offset varies depending on whether daylight savings time is in use at the time instant." |
If there were other use cases for RoundUp/RoundDown that come up the future, then it seems like it would be fine to revisit this. But this use case doesn't seem like it justifies them, since the use does not result in a correct program. Are there any other use cases? If not, we should probably decline this. |
I dont know, maybe there are, maybe not. If this is closed, I think we should invite people to reopen if they find a good use-case for this. |
Based on the discussion above, this seems like a likely decline. |
No change in consensus, so declined. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputHaving a ceil method is usefull for cases where you have a minimum duration, but in reality it needs to be on a per X basis, so the duration might only be 0.5 seconds, but in-reality it can only finish once aa second so you want to round it up to 1 second.
The Time package provides a time round method for rounding to the nearest X and a truncate method for rounding down(towards 0) but not the other way around (away from 0) making it incomplete.
The text was updated successfully, but these errors were encountered: