-
Notifications
You must be signed in to change notification settings - Fork 57
Introduce a Shortstring based Name type #324
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
Changes from 2 commits
9125776
f07d246
d5dd9bc
b12c7db
9e7b073
1c4527a
2ec09bf
49bbd30
e3d8d0d
faf0a29
68a4fb4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
struct SName | ||
region::ShortString15 | ||
locality1::ShortString15 | ||
locality2::ShortString15 | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe should be a subtype of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was considering it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just use a larger There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you think that is viable than sure. I also though that they would be slower, as i have found that they are much slower than expected once you get large, but it seems that that doesn't really kick in for ShortString63. Benchmarks== falsejulia> @btime x==y setup=(x=convert(TimeZones.SName, "America/Winnipeg"); y=convert(TimeZones.SName, "America/Argentina/ComodRivadavia"))
1.504 ns (0 allocations: 0 bytes)
false
julia> @btime x==y setup=(x=ShortString63("America/Winnipeg"); y=ShortString63("America/Argentina/ComodRivadavia"))
2.117 ns (0 allocations: 0 bytes)
false == true(basically exactly the same as the false case) julia> @btime x==y setup=(x=convert(TimeZones.SName, "America/Winnipeg"); y=convert(TimeZones.SName, "America/Winnipeg"))
1.505 ns (0 allocations: 0 bytes)
true
julia> @btime x==y setup=(x=ShortString63("America/Winnipeg"); y=ShortString63("America/Winnipeg"))
2.115 ns (0 allocations: 0 bytes)
true hashjulia> @btime hash(x)==hash(y) setup=(x=convert(TimeZones.SName, "America/Winnipeg"); y=convert(TimeZones.SName, "America/Argentina/ComodRivadavia"))
34.256 ns (0 allocations: 0 bytes);
julia> @btime hash(x)==hash(y) setup=(x=ShortString63("America/Winnipeg"); y=ShortString63("America/Argentina/ComodRivadavia"));
27.598 ns (0 allocations: 0 bytes) So I will make that change, since it is simpler |
||
|
||
function Base.print(io::IO, name::SName) | ||
print(io, name.region) | ||
if !isempty(name.locality1) | ||
print(io,"/", name.locality1) | ||
if !isempty(name.locality2) | ||
print(io,"/", name.locality2) | ||
end | ||
end | ||
end | ||
|
||
Base.convert(::Type{String}, name::SName) = string(name) | ||
function Base.convert(::Type{SName}, str::AbstractString) | ||
name = try_convert(SName, str) | ||
name isa Nothing && DomainError(str, "All timezone name parts must have length < 16") | ||
oxinabox marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return name | ||
end | ||
|
||
try_convert(::Type{SName}, name::SName) = name | ||
try_convert(::Type{String}, name::String) = name | ||
function try_convert(::Type{SName}, str::AbstractString) | ||
parts = split(str, "/"; limit=3) | ||
oxinabox marked this conversation as resolved.
Show resolved
Hide resolved
|
||
all(length(parts) < 16) ||return nothing | ||
return if length(parts) == 3 | ||
SName(parts[1], parts[2], parts[3]) | ||
elseif length(parts) == 2 | ||
SName(parts[1], parts[2], ss15"") | ||
else | ||
SName(parts[1], ss15"", ss15"") | ||
end | ||
omus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
end | ||
|
||
|
||
Base.isempty(name::SName) = isempty(name.region) # region being empty implies all empty | ||
|
||
name_parts(str::AbstractString) = split(str, "/") | ||
function name_parts(name::SName) | ||
# TODO this could be faster by returning an iterator but not really performance critial | ||
parts = [name.region] | ||
if !isempty(name.locality1) | ||
push!(parts, name.locality1) | ||
if !isempty(name.locality2) | ||
push!(parts, name.locality2) | ||
end | ||
end | ||
return parts | ||
end | ||
|
||
# Short strings are broken on 32bit: | ||
# TODO: https://github.com/JuliaString/MurmurHash3.jl/issues/12 | ||
const Name = Int === Int32 ? String : SName |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,22 +6,20 @@ using Dates: AbstractDateTime, argerror, validargs | |
# A `DateTime` that includes `TimeZone` information. | ||
# """ | ||
|
||
struct ZonedDateTime <: AbstractDateTime | ||
struct ZonedDateTime{T<:TimeZone} <: AbstractDateTime | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I learned from Intervals.jl changing the type parameters like this should be considered a breaking change. I'd probably punt this as part of this PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't achieve the goal of making a ZonedDateTime with a FixedTimeZone isbits without this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But i could remove it from this PR. though it would make it easier to make a PR that pushed the timezone as a value in the type-parameter. |
||
utc_datetime::DateTime | ||
timezone::TimeZone | ||
timezone::T | ||
zone::FixedTimeZone # The current zone for the utc_datetime. | ||
end | ||
|
||
function ZonedDateTime(utc_datetime::DateTime, timezone::TimeZone, zone::FixedTimeZone) | ||
return new(utc_datetime, timezone, zone) | ||
function ZonedDateTime( | ||
utc_datetime::DateTime, timezone::VariableTimeZone, zone::FixedTimeZone | ||
) | ||
if timezone.cutoff !== nothing && utc_datetime >= timezone.cutoff | ||
throw(UnhandledTimeError(timezone)) | ||
end | ||
|
||
function ZonedDateTime(utc_datetime::DateTime, timezone::VariableTimeZone, zone::FixedTimeZone) | ||
if timezone.cutoff !== nothing && utc_datetime >= timezone.cutoff | ||
throw(UnhandledTimeError(timezone)) | ||
end | ||
|
||
return new(utc_datetime, timezone, zone) | ||
end | ||
return ZonedDateTime{VariableTimeZone}(utc_datetime, timezone, zone) | ||
end | ||
|
||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,13 +77,13 @@ using Dates: Hour, Second, UTM, @dateformat_str | |
utc_dt = DateTime(1916, 1, 31, 23) | ||
|
||
# Disambiguating parameters ignored when there is no ambiguity. | ||
@test ZonedDateTime(local_dt, warsaw).zone.name == "CET" | ||
@test ZonedDateTime(local_dt, warsaw, 0).zone.name == "CET" | ||
@test ZonedDateTime(local_dt, warsaw, 1).zone.name == "CET" | ||
@test ZonedDateTime(local_dt, warsaw, 2).zone.name == "CET" | ||
@test ZonedDateTime(local_dt, warsaw, true).zone.name == "CET" | ||
@test ZonedDateTime(local_dt, warsaw, false).zone.name == "CET" | ||
@test ZonedDateTime(utc_dt, warsaw, from_utc=true).zone.name == "CET" | ||
@test string(ZonedDateTime(local_dt, warsaw).zone.name) == "CET" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implementing promotion rules should avoid all these changes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so. Happy to hear if i am wrong though. |
||
@test string(ZonedDateTime(local_dt, warsaw, 0).zone.name) == "CET" | ||
@test string(ZonedDateTime(local_dt, warsaw, 1).zone.name) == "CET" | ||
@test string(ZonedDateTime(local_dt, warsaw, 2).zone.name) == "CET" | ||
@test string(ZonedDateTime(local_dt, warsaw, true).zone.name) == "CET" | ||
@test string(ZonedDateTime(local_dt, warsaw, false).zone.name) == "CET" | ||
@test string(ZonedDateTime(utc_dt, warsaw, from_utc=true).zone.name) == "CET" | ||
|
||
@test ZonedDateTime(local_dt, warsaw).utc_datetime == utc_dt | ||
@test ZonedDateTime(local_dt, warsaw, 0).utc_datetime == utc_dt | ||
|
@@ -99,13 +99,13 @@ using Dates: Hour, Second, UTM, @dateformat_str | |
utc_dt = DateTime(1916, 5, 31, 22) | ||
|
||
# Disambiguating parameters ignored when there is no ambiguity. | ||
@test ZonedDateTime(local_dt, warsaw).zone.name == "CEST" | ||
@test ZonedDateTime(local_dt, warsaw, 0).zone.name == "CEST" | ||
@test ZonedDateTime(local_dt, warsaw, 1).zone.name == "CEST" | ||
@test ZonedDateTime(local_dt, warsaw, 2).zone.name == "CEST" | ||
@test ZonedDateTime(local_dt, warsaw, true).zone.name == "CEST" | ||
@test ZonedDateTime(local_dt, warsaw, false).zone.name == "CEST" | ||
@test ZonedDateTime(utc_dt, warsaw, from_utc=true).zone.name == "CEST" | ||
@test string(ZonedDateTime(local_dt, warsaw).zone.name) == "CEST" | ||
@test string(ZonedDateTime(local_dt, warsaw, 0).zone.name) == "CEST" | ||
@test string(ZonedDateTime(local_dt, warsaw, 1).zone.name) == "CEST" | ||
@test string(ZonedDateTime(local_dt, warsaw, 2).zone.name) == "CEST" | ||
@test string(ZonedDateTime(local_dt, warsaw, true).zone.name) == "CEST" | ||
@test string(ZonedDateTime(local_dt, warsaw, false).zone.name) == "CEST" | ||
@test string(ZonedDateTime(utc_dt, warsaw, from_utc=true).zone.name) == "CEST" | ||
|
||
@test ZonedDateTime(local_dt, warsaw).utc_datetime == utc_dt | ||
@test ZonedDateTime(local_dt, warsaw, 0).utc_datetime == utc_dt | ||
|
@@ -133,10 +133,10 @@ using Dates: Hour, Second, UTM, @dateformat_str | |
@test_throws NonExistentTimeError ZonedDateTime(local_dts[2], warsaw, true) | ||
@test_throws NonExistentTimeError ZonedDateTime(local_dts[2], warsaw, false) | ||
|
||
@test ZonedDateTime(local_dts[1], warsaw).zone.name == "CET" | ||
@test ZonedDateTime(local_dts[3], warsaw).zone.name == "CEST" | ||
@test ZonedDateTime(utc_dts[1], warsaw, from_utc=true).zone.name == "CET" | ||
@test ZonedDateTime(utc_dts[2], warsaw, from_utc=true).zone.name == "CEST" | ||
@test string(ZonedDateTime(local_dts[1], warsaw).zone.name) == "CET" | ||
@test string(ZonedDateTime(local_dts[3], warsaw).zone.name) == "CEST" | ||
@test string(ZonedDateTime(utc_dts[1], warsaw, from_utc=true).zone.name) == "CET" | ||
@test string(ZonedDateTime(utc_dts[2], warsaw, from_utc=true).zone.name) == "CEST" | ||
|
||
@test ZonedDateTime(local_dts[1], warsaw).utc_datetime == utc_dts[1] | ||
@test ZonedDateTime(local_dts[3], warsaw).utc_datetime == utc_dts[2] | ||
|
@@ -151,12 +151,12 @@ using Dates: Hour, Second, UTM, @dateformat_str | |
@test_throws AmbiguousTimeError ZonedDateTime(local_dt, warsaw) | ||
@test_throws AmbiguousTimeError ZonedDateTime(local_dt, warsaw, 0) | ||
|
||
@test ZonedDateTime(local_dt, warsaw, 1).zone.name == "CEST" | ||
@test ZonedDateTime(local_dt, warsaw, 2).zone.name == "CET" | ||
@test ZonedDateTime(local_dt, warsaw, true).zone.name == "CEST" | ||
@test ZonedDateTime(local_dt, warsaw, false).zone.name == "CET" | ||
@test ZonedDateTime(utc_dts[1], warsaw, from_utc=true).zone.name == "CEST" | ||
@test ZonedDateTime(utc_dts[2], warsaw, from_utc=true).zone.name == "CET" | ||
@test string(ZonedDateTime(local_dt, warsaw, 1).zone.name) == "CEST" | ||
@test string(ZonedDateTime(local_dt, warsaw, 2).zone.name) == "CET" | ||
@test string(ZonedDateTime(local_dt, warsaw, true).zone.name) == "CEST" | ||
@test string(ZonedDateTime(local_dt, warsaw, false).zone.name) == "CET" | ||
@test string(ZonedDateTime(utc_dts[1], warsaw, from_utc=true).zone.name) == "CEST" | ||
@test string(ZonedDateTime(utc_dts[2], warsaw, from_utc=true).zone.name) == "CET" | ||
|
||
@test ZonedDateTime(local_dt, warsaw, 1).utc_datetime == utc_dts[1] | ||
@test ZonedDateTime(local_dt, warsaw, 2).utc_datetime == utc_dts[2] | ||
|
@@ -172,12 +172,12 @@ using Dates: Hour, Second, UTM, @dateformat_str | |
utc_dts = (DateTime(1922, 5, 31, 21), DateTime(1922, 5, 31, 22)) | ||
@test_throws AmbiguousTimeError ZonedDateTime(local_dt, warsaw) | ||
|
||
@test ZonedDateTime(local_dt, warsaw, 1).zone.name == "EET" | ||
@test ZonedDateTime(local_dt, warsaw, 2).zone.name == "CET" | ||
@test string(ZonedDateTime(local_dt, warsaw, 1).zone.name) == "EET" | ||
@test string(ZonedDateTime(local_dt, warsaw, 2).zone.name) == "CET" | ||
@test_throws AmbiguousTimeError ZonedDateTime(local_dt, warsaw, true) | ||
@test_throws AmbiguousTimeError ZonedDateTime(local_dt, warsaw, false) | ||
@test ZonedDateTime(utc_dts[1], warsaw, from_utc=true).zone.name == "EET" | ||
@test ZonedDateTime(utc_dts[2], warsaw, from_utc=true).zone.name == "CET" | ||
@test string(ZonedDateTime(utc_dts[1], warsaw, from_utc=true).zone.name) == "EET" | ||
@test string(ZonedDateTime(utc_dts[2], warsaw, from_utc=true).zone.name) == "CET" | ||
|
||
@test ZonedDateTime(local_dt, warsaw, 1).utc_datetime == utc_dts[1] | ||
@test ZonedDateTime(local_dt, warsaw, 2).utc_datetime == utc_dts[2] | ||
|
@@ -283,14 +283,14 @@ using Dates: Hour, Second, UTM, @dateformat_str | |
|
||
# Make sure that the duplicated hour only doesn't contain an additional entry. | ||
@test_throws AmbiguousTimeError ZonedDateTime(DateTime(1935,9,1), dup) | ||
@test ZonedDateTime(DateTime(1935,9,1), dup, 1).zone.name == "DTDT-2" | ||
@test ZonedDateTime(DateTime(1935,9,1), dup, 2).zone.name == "DTST" | ||
@test string(ZonedDateTime(DateTime(1935,9,1), dup, 1).zone.name) == "DTDT-2" | ||
@test string(ZonedDateTime(DateTime(1935,9,1), dup, 2).zone.name) == "DTST" | ||
@test_throws BoundsError ZonedDateTime(DateTime(1935,9,1), dup, 3) | ||
|
||
# Ensure that DTDT-1 is completely ignored. | ||
@test_throws NonExistentTimeError ZonedDateTime(DateTime(1935,4,1), dup) | ||
@test ZonedDateTime(DateTime(1935,4,1,1), dup).zone.name == "DTDT-2" | ||
@test ZonedDateTime(DateTime(1935,8,31,23), dup).zone.name == "DTDT-2" | ||
@test string(ZonedDateTime(DateTime(1935,4,1,1), dup).zone.name) == "DTDT-2" | ||
@test string(ZonedDateTime(DateTime(1935,8,31,23), dup).zone.name) == "DTDT-2" | ||
end | ||
|
||
@testset "equality" begin | ||
|
@@ -430,4 +430,20 @@ using Dates: Hour, Second, UTM, @dateformat_str | |
@test typemin(ZonedDateTime) <= ZonedDateTime(typemin(DateTime), utc) | ||
@test typemax(ZonedDateTime) >= ZonedDateTime(typemax(DateTime), utc) | ||
end | ||
|
||
# TODO: isbits is not working on 32 bit because of not using SName type, because of | ||
# https://github.com/JuliaString/MurmurHash3.jl/issues/12 | ||
Int==Int64 && @testset "isbits" begin | ||
utc_zdt = ZonedDateTime(1, 2, 3, 4, 5, 6, 7, utc) | ||
@test isbits(utc) | ||
|
||
var_zdt = ZonedDateTime(Date(2011, 6, 1), tz"America/Winnipeg") | ||
@test !isbits(var_zdt) # we might like this, but we don't have it. | ||
@test isbits(var_zdt.utc_datetime) | ||
@test isbits(var_zdt.zone) | ||
@test isbits(var_zdt.utc_datetime) | ||
@test isbits(var_zdt.timezone.cutoff) | ||
@test isbits(var_zdt.timezone.name) | ||
@test isbitstype(eltype(var_zdt.timezone.transitions)) | ||
end | ||
end |
Uh oh!
There was an error while loading. Please reload this page.