Custom parser for dates.

[simonbyrne]

This replaces the default date parser with a custom-coded version. It is approximately 50x faster on the following snippet:

datestr = map(string,range(DateTime("2016-02-19T12:34:56"),Dates.Millisecond(123),100_000))
testDateTime(v) = map(DateTime,v)

@time testDateTime(datestr)

[simonbyrne]

cc: @ViralBShah @quinnj @JeffBezanson

[iamed2]

cc: @omus

[omus]

Something I would like to see regarding date parsing/formatting would be to parameterize the DateFormat type with the format being used. Something similar toDateFormat{Symbol("yyyy-mm-ddTHH:MM:SS")}. The advantage of this would be the ability to have specialized parsers for commonly used formats.

[omus]

Since this change is about performance there should probably be some performance tests added to https://github.com/JuliaCI/BaseBenchmarks.jl regarding parsing dates.

[JeffBezanson]

Yes, it would be nice to have a way to hack in parsers for various common formats as they come up. Either dispatch or just if/else statements that return DateFormats that know how to call the right function.

[simonbyrne]

I should have pointed to the discussion on the open issue #15888.

There is Dates.DateFormat for custom formats, but that's slow. Really the date parsing stuff probably needs quite a large overhaul. This was more of a stop-gap (and potentially back-portable) way to handle the default case.

[ViralBShah]

Isn't the parser tag for julia parser stuff?

[JeffBezanson]

There is Dates.DateFormat for custom formats, but that's slow.

Which is why I want, eventually, the ability to manually write parsers for specific formats. I'm thinking of something like:

function DateFormat(str)
    if str == "yyyy/mm/dd"
        return special_fast_parsing_function
    else if str == ...
        ...
    else
        return slow_fallback
    end
end

Isn't the parser tag for julia parser stuff?

Yes.

[omus]

Which is why I want, eventually, the ability to manually write parsers for specific formats

I also want this. My only issue with the if/else approach is that it won't be extensible by other packages. I'm specifically thinking about TimeZones.jl and the z/Z format.

[simonbyrne]

I've added an AbstractDateFormat and a FastDateFormat (suggestions for better names welcome), which should make this more easily extensible.

[tkelman]

this change needs to be made in the rst docs too

[simonbyrne]

Updated the docs, any other comments?

[omus]

Should this be dt::AbstractString to match the above signature?

[simonbyrne]

seems reasonable, updated.

[ViralBShah]

Should we merge this now, as we figure out improvements going forward?

[tkelman]

Let's make sure CI still passes, there have been a month of changes on master since it last ran here.

edit: I think the osx failure dlopen(libopenblas.dylib, 1): image not found is due to the branch's .travis.yml being a little out of date, and not a real issue

edit2: win64 appveyor failure is just #16555, also nothing new

[stevengj]

Bump?

[stevengj]

On an unrelated note, I just noticed that we are missing parse(DateTime, str) = DateTime(str)

[JeffBezanson]

We should put in an explicit check for better-supported format strings like "yyyy-mm-dd", so you get good performance if you make the "mistake" of asking for the format by string. Avoiding the performance landmine is worth some ugliness.

[stevengj]

(A benchmark of this was added to BaseBenchmarks.)

[simonbyrne]

@nanosoldier runbenchmarks(ALL, vs = ":master")

[nanosoldier]

Your benchmark job has completed - no performance regressions were detected. A full report can be found here. cc @jrevels

[iamed2]

Looks like no date parse benchmark was run?

[tkelman]

I think new benchmarks need to be manually deployed, it's not automatic from merging PR's

[jrevels]

The tuning process is running now, I'll retrigger benchmarks here once it finishes.

EDIT: Tuning process failed, so it'll be a little while longer. There was a bug in a recently added benchmark, I fixed it in JuliaCI/BaseBenchmarks.jl/pull/35. Once tests pass and I merge the fix, I'll restart the tuning process...

[jrevels]

@nanosoldier runbenchmarks(ALL, vs = ":master")

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

[omus]

I've done some iterating on this proposal and have some code which doesn't need @chk1 and improves performance further. I'll try to turn it into a PR soon.

[JeffBezanson]

@omus Any update, or should we just merge this?

[omus]

@JeffBezanson thanks for the bump. I'll try to get out my proposal tonight. If I can't we can merge this first.

[tkelman]

this is superseded by / contained in #19545, right?

[omus]

@tkelman correct