JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.44k stars 5.46k forks source link

Allow DateTime and Dates.ISODateTimeFormat to use a trailing "Z" #23049

Open cormullion opened 7 years ago

cormullion commented 7 years ago

On Julia v0.7 and v0.6, the ISODateTimeFormat doesn't like the trailing "Z".

julia> DateTime("2017-07-17T14:10:36.000", Dates.ISODateTimeFormat)
2017-07-17T14:10:36

julia> DateTime("2017-07-17T14:10:36.000Z", Dates.ISODateTimeFormat)
ERROR: ArgumentError: Invalid DateTime string
Stacktrace:
 [1] parse(::Type{DateTime}, ::String, ::DateFormat{Symbol("yyyy-mm-dd\\THH:MM:SS.s"),Tuple{Base.Dates.DatePart{'y'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'m'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'d'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'H'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'M'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'S'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'s'}}}) at ./dates/parse.jl:265
 [2] DateTime(::String, ::DateFormat{Symbol("yyyy-mm-dd\\THH:MM:SS.s"),Tuple{Base.Dates.DatePart{'y'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'m'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'d'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'H'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'M'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'S'},Base.Dates.Delim{Char,1},Base.Dates.DatePart{'s'}}}) at ./dates/io.jl:431

I'm not sure whether it's officially a part of the ISO8601 standard to have the trailing "Z" (it looks like it might be, but I'd need to read the standard and you've got to buy a copy :)). But even if it isn't, it would be nice to accept it, since a lot of places use them for time stamps (e.g. YouTube).

ruffianeo commented 4 years ago

julia> Dates.ISODateTimeFormat dateformat"yyyy-mm-ddTHH:MM:SS.s"

is not ISO6801 compliant. It lacks the time zone part. ("+|-HH:MM" | "Z"). This makes data exchange with compliant code difficult. E.g. .NET DateTime does it right and Julia cannot parse, e.g. this:

"2019-12-13T19:33:22.5093173+01:00"

Since this issue is now open close to 2 years... someone should address it.

See https://en.wikipedia.org/wiki/ISO_8601 as reference.

omus commented 4 years ago

Time zone support is added by TimeZones.jl.

julia> using Dates, TimeZones

julia> ZonedDateTime("2019-12-13T19:33:22.509+01:00", dateformat"yyyy-mm-ddTHH:MM:SS.sssz")
2019-12-13T19:33:22.509+01:00

Support however isn't perfect as I think your exact example uses sub-millisecond precision which unfortunately isn't handled by DateTime/ZonedDateTime.

ruffianeo commented 4 years ago

Would it not be better to fix instead of adding? (extend Dates package, deprecate TimeZones package? For the past hour I inspected the sources of Dates package and here is the idea I had. Unfortunately I cannot volunteer to implement it as my Julia experience is still too limited. (It would produce less than perfect code, I assume).

https://github.com/JuliaLang/julia/blob/master/stdlib/Dates/src/io.jl

Extend parse() and other functions to deal with (new) time zone field.

https://github.com/JuliaLang/julia/blob/master/stdlib/Dates/src/accessors.jl

Add accessor for (new) time zone field "offset".

https://github.com/JuliaLang/julia/blob/master/stdlib/Dates/src/adjusters.jl

tozulu()

d1 : DateTime , d2 : DateTime = tozulu(d1)
------------------------------------------------------------------------------------
d2.offset == 0
, [d2.year, d2.month, d2.day, d2.hour, d2.minute] adjusted by d1.offset
, DateTime.offset in signed minutes.

totimezone()

d1 : DateTime, timezoneoffset : Int, d2 : DateTime = totimezone(d1,timezoneoffset)
------------------------------------------------------------------------------------
d2.offset == timezoneoffset
, [d2.year, d2.month, d2.day, d2.hour, d2.minute] adjusted by timezoneoffset
, timezoneoffset in signed minutes

Please note, that

d1 : DateTime, d2 = tozulu(d1), d3 = totimezone(d1,0)
------------------------------------------------------------------------------------
d2 == d3

So, tozulu() is redundant but maybe nice to have.

https://github.com/JuliaLang/julia/blob/master/stdlib/Dates/src/types.jl

AbstractTime now also needs to contract an offset (in minutes).

function DateTime(...) needs another optional argument (offset).

function DateTime(dt::Date, t::Time) needs another argument (offset).

Gotchas:

Use case 1: Create a DateTime in GMT+1, noon. The time and date given should not be adjusted. (GMT+1 noon != GMT+0 noon.)

dt1 = DateTime(Date("2019-01-01"), Time("12:00"), 60 ) => "2019-01-01T12:00:00.0+01:00"

Use case 2: Create a Zulu noon.

dt2 = DateTime(Date("2019-01-01"), Time("12:00"), 0 ) => "2019-01-01T12:00:00.0Z"

totimezone(dt2,60) => "2019-01-01T13:00:00.0+01:00"

dt1 !== totimezone(dt2,60)

Depending on what user tries to do, this might be confusing.

Summary

Other files in Dates package might be affected as well.

Since there were no time zones before, old code should not break if default offset is ZULU (offset = 0). Only new arguments to existing functions and new functions.

Except for code which tries to parse a compliant ISO6801 string and relies on it to fail to enter a special handling code path.

omus commented 4 years ago

It should be noted that there isn't anything special about Dates being a stdlib and TimeZones as a package. The Julia DateTime type is meant for use without time zones which is why there is a different type ZonedDateTime which includes use of time zones. This separation of types is useful and it's doubtful that will change.

I'd start by using the existing support for time zones before attempting to reinvent it.

simonbyrne commented 4 years ago

I think we should add offset support to Dates (equivalent to the FixedTimeZone type from TimeZones.jl, since it is so common, e.g. it is required as part of the TOML spec.

We can (and probably should) leave timezones themselves in the TimeZones.jl, since they can change very frequently.

cc: @KristofferC