feat: Relax UnixTime construction from String

[Beforerr]

Together with the ability to compare UnixTime with DateTime

Close #18

P.S.: There is some autoformatting from Runic.jl, I can remove if not wanted

[Beforerr]

It would be nice that we get a version bump after this merge

[ancapdev]

Thanks, happy to merge this change in!

If you could remove the format and style changes first, that would be good. I prefer expression style to explicit return statements.

You can bump the version in the Project.toml, and I'll register it after merge.

[Beforerr]

Thank you @ancapdev , I have splitted the PR with a format commit and added a version bump.

[Beforerr]

Oh sorry, I read your formatting message wrong:) I removed the formatting at all and included your suggesttion for DATEFORMAT

[ancapdev]

Oh sorry, I read your formatting message wrong:) I removed the formatting at all and included your suggesttion for DATEFORMAT

No worries, I've merged it, will release it soon

[thirtysixbananas]

Is the promotion rule intentional?

https://github.com/ancapdev/UnixTimes.jl/pull/19/files#diff-7c943b7f2a3b5f3c5a789753de9109a1d8770f499f53f696ed3862542764885eR102

[ancapdev]

Is the promotion rule intentional?

https://github.com/ancapdev/UnixTimes.jl/pull/19/files#diff-7c943b7f2a3b5f3c5a789753de9109a1d8770f499f53f696ed3862542764885eR102

Good point, I glanced over and missed that. It's probably a bit unsafe, since it loses range on DateTime. Thoughts?

[thirtysixbananas]

I don't have an opinion on whether this should go in or not, your call.

The only comment is that I believe the promotion rule shouldn't be necessary for this specific feature. If a conversion was needed, it could just as well be done explicitly. It was probably added for the last testcase, named "comparison with DateTime".

[ancapdev]

@Beforerr what was your rationale with that?

[Beforerr]

Oh I just think this is a useful feature as without this promotion we can not directly compare DateTime with UnixTime (as they are both AbstractDateTime, and UnixTime has better precision so promoting DateTime to UnixTime is a fair operation?). Is this causing any bugs on your side @thirtysixbananas ?

[Beforerr]

Without it many functions like searchsorted would need an additional wrapping.

[ancapdev]

The issue I see with the promote rule is you can have DateTime instances out of range with UnixTime, which will then fail conversion. Failing conversion shouldn't mean failing comparison. I think promotion should be to a strict superset type, or comparison implemented directly.

NanoDate.jl for example implements comparisons directly: https://github.com/JuliaTime/NanoDates.jl/blob/main/src/compare.jl.

I wrote UnixTimes.jl because I wanted something that was more efficient, limited to 64 bits of nanoseconds since epoch, it serves a lot of practical use cases (financial data, logging, telemetry, sensors, etc), but it's quite incomplete as far as features like this.

[Beforerr]

I understand. Yes it would be nice to have direct comparison. On the other hand, this promotion is quite similar to what Base.Dates does for DateTime and Date (I am not saying this is a right way).

julia> DateTime(2011) > Date(2010)
true

julia> typemax(DateTime)
146138512-12-31T23:59:59

julia> typemax(Date)
252522163911149-12-31

julia> typemax(Date) > typemax(DateTime)
false

[Beforerr]

NanoDates also has promote_rule https://github.com/JuliaTime/NanoDates.jl/blob/71523adaada46ddcb2a4a7b002d22a551722696d/src/interop.jl#L4

[thirtysixbananas]

I understand. Yes it would be nice to have direct comparison. On the other hand, this promotion is quite similar to what Base.Dates does for DateTime and Date (I am not saying this is a right way).

julia> DateTime(2011) > Date(2010)
true

julia> typemax(DateTime)
146138512-12-31T23:59:59

julia> typemax(Date)
252522163911149-12-31

julia> typemax(Date) > typemax(DateTime)
false

Looks like a bug, no?

[Beforerr]

Indeed like a bug (I felt like this kinda of overflow/underflow things are quite common though :<

[ancapdev]

NanoDates also has promote_rule https://github.com/JuliaTime/NanoDates.jl/blob/71523adaada46ddcb2a4a7b002d22a551722696d/src/interop.jl#L4

It does yes, but NanoDate contains all the range and precision of types it promotes from, so that's safe.

From the manual

In this sense, the term "promotion" is appropriate since the values are converted to a "greater" type – i.e. one which can represent all of the input values in a single common type.

That said, I think we can leave this in for now and refine it later. I think it's unlikely that anyone is using UnixTime in a context where its range is a limiting factor for the data being processed, even if that data is represented with DateTime.