Add rounding operations for ZonedDateTimes
#29
Conversation
Dependent on [PR #17037](JuliaLang/julia#17037) in Base Julia
| straightforward fashion. | ||
|
|
||
| Rounding is not an entirely "safe" operation for `ZonedDateTime`s, as in some cases | ||
| historical transitions for some time zones (`Asia/Colombo, for example) occur at midnight. |
Documentation changes were made based on PR feedback.
`ceil(ZonedDateTime(1996, 10, 26, 0, 35, TimeZone("Asia/Colombo")), Dates.Day)`
no longer results in an AmbiguousTimeError.
Added test cases for rounding to nearest week.
|
Rounding for Date/DateTime have been merged into base (JuliaLang/julia#17037). Looking into this PR again. |
|
@spurll Julia 0.4 is failing since the rounding functions don't exist in base for that version. Probably the best approach here is to conditionally include the rounding functionality based upon the Julia version. You can do this by determining the commit which includes the rounding functionality in Base then using Compat's version.sh script to generate a version number literal. |
Current coverage is 89.82%@@ master #29 diff @@
==========================================
Files 15 16 +1
Lines 673 678 +5
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 607 609 +2
- Misses 66 69 +3
Partials 0 0
|
|
Appveyor failure on 0.5 is due to JuliaPackaging/WinRPM.jl#78 |
| and should generally behave as expected. When `VariableTimeZone` transitions are involved, | ||
| however, unexpected behaviour may be encountered. | ||
|
|
||
| Instead of performing rounding operations on the `ZonedDateTime`'s internal UTC `DateTime`, |
There was a problem hiding this comment.
"internal UTC representation" or "internal UTC DateTime representation"
There was a problem hiding this comment.
Okay. ZonedDateTimes do have an internal DateTime that's in UTC of course, but if you think this is more clear that's your call.
There was a problem hiding this comment.
I'd rather not reference how the internals of ZonedDateTime work at all.
There was a problem hiding this comment.
That's totally fair. I considered that when I was writing it, but I figured that some people who do know about the internals might wonder why we're not just using UTC, so I wanted to put this explanation somewhere.
Do you think this should be moved to a comment in the code, perhaps? Or just removed entirely?
There was a problem hiding this comment.
I think you can mention the two strategies of either performing rounding in the UTC time zone or the local time zone but I wouldn't mention which is used internally
Minor tweaks to documentation as requested in PR comments. Replaced calls to `TimeZone` with calls to `resolve` in test cases.
|
@spurll PR looks good. I'm ready to approve but I'm going to wait until JuliaPackaging/WinRPM.jl#78 is addressed. |
Dependent on PR #17037 in Base Julia.
Checks will fail and need to be rerun once that PR is merged into Base.