TimeZones.jl icon indicating copy to clipboard operation
TimeZones.jl copied to clipboard

Add Accessors for Day and higher

Open amsingh17 opened this issue 1 year ago • 3 comments

During a development project, I noticed that I was not able to run Day (or higher) on ZonedDateTime objects. The fix in this PR addresses that issue. Below is a minimum working example to show the issue

               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.9.3 (2023-08-24)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> using TimeZones, Dates

julia> x = ZonedDateTime(today(), tz"UTC")
2024-09-23T00:00:00+00:00

julia> Hour(x)
0 hours

julia> Day(x)
ERROR: MethodError: no method matching Day(::ZonedDateTime)

Closest candidates are:
  (::Type{T})(::Period) where T<:Period
   @ Dates /Applications/Julia-1.9.app/Contents/Resources/julia/share/julia/stdlib/v1.9/Dates/src/periods.jl:384
  Day(::Number)
   @ Dates /Applications/Julia-1.9.app/Contents/Resources/julia/share/julia/stdlib/v1.9/Dates/src/types.jl:55
  Day(::AbstractString)
   @ Dates /Applications/Julia-1.9.app/Contents/Resources/julia/share/julia/stdlib/v1.9/Dates/src/periods.jl:21
  ...

Stacktrace:
 [1] top-level scope
   @ REPL[3]:1


amsingh17 avatar Sep 23 '24 20:09 amsingh17

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.57%. Comparing base (2bc8f50) to head (edf1e2a). Report is 17 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #478      +/-   ##
==========================================
- Coverage   92.79%   92.57%   -0.23%     
==========================================
  Files          39       38       -1     
  Lines        1818     1831      +13     
==========================================
+ Hits         1687     1695       +8     
- Misses        131      136       +5     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Sep 25 '24 14:09 codecov[bot]

I originally added this accessor code when Dates.jl narrowed the types used for these sub-day accessors: https://github.com/JuliaTime/TimeZones.jl/commit/61d8299a50a54cb3e194dc3e27ce851deca5edd2.

omus avatar Sep 25 '24 14:09 omus

@omus I've added a block of tests to use the Period of a ZonedDateTime. Open to suggestions on expanding additional coverage. Thanks!

amsingh17 avatar Sep 25 '24 16:09 amsingh17

In looking over the code again I realized that part of the original implementation was avoiding defining additional methods where they weren't required. Defining methods for year, month, etc. aren't necessary as they use TimeType so I've refactored the code to define only the necessary accessors and constructor methods. I've also expanded the test suite to ensure that changes to Dates.jl don't break accessor support here.

omus avatar Oct 30 '24 15:10 omus

Looks like I'll be dealing with LTS issues before getting this in.

Update: CI WeakDeps failure was just a fluke

omus avatar Oct 30 '24 15:10 omus