Skip to content
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

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

Open
cormullion opened this issue Jul 31, 2017 · 5 comments
Open

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

cormullion opened this issue Jul 31, 2017 · 5 comments
Labels
domain:dates Dates, times, and the Dates stdlib module

Comments

@cormullion
Copy link
Contributor

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
Copy link

ruffianeo commented Dec 13, 2019

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
Copy link
Member

omus commented Dec 13, 2019

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
Copy link

ruffianeo commented Dec 13, 2019

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
Copy link
Member

omus commented Dec 13, 2019

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
Copy link
Contributor

simonbyrne commented May 22, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:dates Dates, times, and the Dates stdlib module
Projects
None yet
Development

No branches or pull requests

5 participants