JuliaTime / TimeZones.jl

IANA time zone database access for the Julia programming language
Other
86 stars 51 forks source link

Add support for new serialization format TZJFile #380

Closed omus closed 2 years ago

omus commented 2 years ago

Part of #359. From the lessons learned from #379 it was determined that we couldn't use the tzfile format for storing the TimeZones.jl internal representation and instead we needed a custom binary format mostly based upon the existing tzfile format.

This PR implements this new format as well as switches the time zone building and loading to utilize this new format. I've opt'd to have the read/write capabilities be a part of TimeZones.jl rather than push this into a new package as this code is just reading/writing the format into TimeZones.jl specific types.

omus commented 2 years ago

Mainly just need to write some tests for this

omus commented 2 years ago

Demonstration of the new tzjfile format in action. As our internal representation hasn't changed we can still use the TZData.compile function to go straight from tzsource files into the TimeZones representation. If we then do a round-trip through the tzjfile format we can identify any incompatibilities:

julia> using TimeZones

julia> using TimeZones: TZData, TZJFile

julia> tz_source = TZData.TZSource(joinpath.(TZData.TZ_SOURCE_DIR, readdir(TZData.TZ_SOURCE_DIR)));

julia> compiled = TZData.compile(tz_source)
595-element Vector{Tuple{TimeZone, TimeZones.Class}}:
 (tz"Europe/Zaporozhye", TimeZones.Class(:STANDARD))
 (tz"Australia/Lindeman", TimeZones.Class(:STANDARD))
 (tz"Europe/Kaliningrad", TimeZones.Class(:STANDARD))
 (tz"Pacific/Nauru", TimeZones.Class(:STANDARD))
 ⋮
 (tz"Europe/Guernsey", TimeZones.Class(:STANDARD))
 (tz"Australia/Tasmania", TimeZones.Class(:LEGACY))
 (tz"America/Santa_Isabel", TimeZones.Class(:LEGACY))
 (tz"Africa/Banjul", TimeZones.Class(:STANDARD))

julia> function convert_to_tzjf(tz, class)
           io = IOBuffer()
           TZJFile.write(seekstart(io), tz; class)
           return TZJFile.read(seekstart(io))(tz.name)
       end
convert_to_tzjf (generic function with 1 method)

julia> tzjf_compatible((tz, class)) = convert_to_tzjf(tz, class) == (tz, class)
tzjf_compatible (generic function with 1 method)

julia> filter(!tzjf_compatible, compiled)
Tuple{TimeZone, TimeZones.Class}[]
omus commented 2 years ago

Part of the issue with #359 is that the TimeZones.jl build step isn't always re-triggered. It seems prudent to avoid switching the internal format used until we bypass the build system altogether. I'll remove some of the current logic in this PR such that this PR only adds support for the TZJFile format but does switch to using it as the default.

codecov-commenter commented 2 years ago

Codecov Report

Merging #380 (f382a5e) into master (c19864a) will increase coverage by 0.32%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #380      +/-   ##
==========================================
+ Coverage   94.48%   94.80%   +0.32%     
==========================================
  Files          32       35       +3     
  Lines        1649     1753     +104     
==========================================
+ Hits         1558     1662     +104     
  Misses         91       91              
Impacted Files Coverage Δ
src/TimeZones.jl 100.00% <ø> (ø)
src/tzjfile/read.jl 100.00% <100.00%> (ø)
src/tzjfile/utils.jl 100.00% <100.00%> (ø)
src/tzjfile/write.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c19864a...f382a5e. Read the comment docs.