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

Date Rounding #17037

Merged
merged 5 commits into from
Jul 9, 2016
Merged

Date Rounding #17037

merged 5 commits into from
Jul 9, 2016

Conversation

spurll
Copy link
Contributor

@spurll spurll commented Jun 20, 2016

Adds methods to allow rounding Date and DateTime values to a specified resolution (1 month, 15 minutes, etc.) with floor, ceil, and round.

While we at Invenia believe that this functionality is useful and appropriate for base Julia, it could also be put in an external package in JuliaTime if the community feels otherwise.

@tkelman tkelman added the dates Dates, times, and the Dates stdlib module label Jun 20, 2016
@quinnj
Copy link
Member

quinnj commented Jun 20, 2016

We already have trunc, so I think this makes sense.

@StefanKarpinski
Copy link
Member

Yes, huge 👍 to this. Great PR, @spurll


This means that rounding seconds, minutes, hours, or years (because the ISO 8601
specification includes a year zero) to an even multiple will result in that field having
an even value, while rounding to an even multiple of months will result in that field
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the difference here? rounding to an even multiple (of what) results in even, when rounding to an even multiple of months results in odd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I think I worded that poorly. It should read, "This means that rounding a DateTime to an even multiple of seconds, minutes, ... will result in that field having an even value in the resulting DateTime, while rounding a DateTime to an even multiple of months will result in the months field having an odd value."

I'm not sure if the use case would be sufficiently common for it to warrant comment, but I figured somebody might wonder why rounding a Date(Time) to Month(2) returns an odd-numbered month instead of an even-numbered month (as might be expected).

@spurll
Copy link
Contributor Author

spurll commented Jun 21, 2016

Hm. Of the three Travis jobs, one was successful (Linux 32), one ran out of storage space (Linux 64), and one was terminated because it took too long (OS X).

@tomasaschan
Copy link
Member

Nice!

Will this also be able to handle when the provided strings are overspecified? See http://stackoverflow.com/questions/37971649/parsing-zulu-dates-in-julia-throws-inexacterror for an example of what I mean.

@spurll
Copy link
Contributor Author

spurll commented Jun 22, 2016

@tlycken: That behaviour will not be affected by this PR. The round/floor/ceil functions operate on existing Date and DateTime values; how strings are parsed into DateTimes is another matter. (ETA: That's not to say that string overspecification shouldn't be supported in some way; it's just not implemented here.)

@kshyatt
Copy link
Contributor

kshyatt commented Jun 28, 2016

Hi @spurll, can you rebase to make this mergeable?

@spurll
Copy link
Contributor Author

spurll commented Jun 28, 2016

Absolutely. Doing that now. Just had to add a quick fix to disallow rounding to non-positive Period values.

spurll added a commit to invenia/TimeZones.jl that referenced this pull request Jul 4, 2016
Throw DomainError on rounding to an invalid (non-positive) resolution
Clean up test cases for rounding dates that don't need rounding
Add test cases for rounding to invalid (non-positive) resolutions
@iamed2
Copy link
Contributor

iamed2 commented Jul 8, 2016

This is good to go now; would be nice to get it in for 0.5.

@tkelman tkelman merged commit c65e5fd into JuliaLang:master Jul 9, 2016
mfasi pushed a commit to mfasi/julia that referenced this pull request Sep 5, 2016
* Added floor, ceil, round for Date and DateTime.

* Refactoring and documentation for date rounding.

* Added PR reference in NEWS.md.

* Bugfix for date rounding test in 32-bit. Doc clarifications.

* Round to nonpositive resolution throws DomainError

Throw DomainError on rounding to an invalid (non-positive) resolution
Clean up test cases for rounding dates that don't need rounding
Add test cases for rounding to invalid (non-positive) resolutions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dates Dates, times, and the Dates stdlib module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants