Elixir: Add Date/Time/NaiveDateTime.compare/2

[josevalim]

It must return :eq | :lt | :gt.

/cc @lau for sanity checking purposes

/cc @JEG2 in case he is interested in tackling this one too :D

[lau]

I have a good idea for an implementation for this.

[lau]

Making erlang-style tuples and comparing them should work.

The following could be a new issue: We could also compare all DateTime structs - with any time zone - using the offsets. Using the offsets instead of calling tzdata means we will use the rules that were in place at the time the struct was created. Just like how it is with DateTime.to_unix/2.

[josevalim]

@lau oh, so we can implement DateTime.compare/2 without relying on tzdata? If so, we should definitely include it too. Thank you!

[lau]

Yes

[JEG2]

I would love to help, but I'm currently selling one house and building another. My hair is a little on fire. Sorry!

[josevalim]

@JEG2 thanks for the heads up and good luck! :D

[ottolin]

I can help on this one if no one is working on it.

To make sure I'm understanding correctly, we would like to use to_erl and then compare the tuples directly for Date/Time/NaiveDateTime. And for DateTime which contains utc/std offsets, we can compare the converted number of seconds after adjusting the offsets.
Is that correct?

[josevalim]

@ottolin yes. also keep in mind that all of those except Date have microseconds and they are not in the erlang tuple, so you would have to consider them too. If you prefer, you can start with the date and time ones. :)

[ottolin]

According to the documentation, precision of microseconds is being used when representing the microseconds to external format.
So probably when comparing two Time structs with same values but of different precisions should return :eq:

Time.compare(Time.from_erl!({16, 4, 16}, {123456, 6}), Time.from_erl!({16, 4, 16}, {123456, 3}))
:eq

Or do you prefer the comparison is done after trimming the microseconds according to precision, like:

Time.compare(Time.from_erl!({16, 4, 16}, {123456, 6}), Time.from_erl!({16, 4, 16}, {123456, 3}))
:gt

So it is a question about what is the meaning of compare. It is comparing the internal representation or something that the user tries to print.

[josevalim]

@ottolin theoretically the values above are not possible. If the precision for the second one is three digits, it would at best be: {123000, 3}. I think we can simply compare the microseconds value and ignore the precision field. @lau, what do you think?

[lau]

@josevalim Yes, I think it makes sense to ignore the precision field.
Erlang style tuples, but with the microsecond as a fourth element of the time part should work.

An example of what I mean by such a tuple is found in Ecto here: https://github.com/elixir-ecto/ecto/blob/e2dca0e208c9d5239b319977765980c6bf680349/lib/ecto/date_time.ex#L644

Maybe it is easiest and fastest to make the tuple "manually" without using the to_erl functions, since they omit the microsecond.

[josevalim]

Yup. Building the tuple manually should be fine too. We can build one big tuple with all 7 elements.

[lau]

@josevalim related to this: Should we add the possibility of passing e.g. a NaiveDateTime struct to the Date.compare/2 function? Then you could see if the NaiveDateTime is on the same date as another Date/NaiveDateTime/DateTime.

Date.compare(~N[2016-01-01 16:00:00], ~D[2016-01-01]) == :eq
Date.compare(~N[2016-01-01 16:00:00], ~N[2016-01-01 03:00:00]) == :eq

In the Calendar package this kind of functionality with protocols, but they are not necessary in this case. Instead additional function definitions could pattern match to transform eg. a NaiveDateTime into a Date.

[josevalim]

@l4u I would like to keep the Date compare specific to only dates. We could add cross data-type comparison to the Calendar module. What do you think?

[l4u]

@lau see above 😄

[lau]

@josevalim Accepting multiple data types is useful for many functions. I saw on the mailing list aday_of_the_week function mentioned. It would be useful to pass Date, NaiveDateTime and DateTime structs to that too. So if you have two different functions for everything in different modules, won't you end up with twice as many functions? It might make sense, but I'm just worried there will be duplicated functions that do basically the same thing spread over different modules.

[josevalim]

@lau that's a very good point. However I am slightly worried about semantics and API confusion. For example, if we allow Date, NaiveDateTime and DateTime in day_of_week. Should we also allow Date.compare(%Date{}, %DateTime{})? If so, how the comparison would work? Should we consider the timezone offset? Also, should we also allow Date.to_erl(%DateTime{})?

Similarly, should we allow DateTime to be given to NaiveDateTime APIs or would those be strictly forbidden? I.e. should we allow DateTime and NaiveDateTime only on Date and Time APIs but NaiveDateTime and DateTime won't be allowed on each other APIs?

If we can come up with a clear rule, then I am all in. :)

[lau]

Here is a principle that is quite simple: If a type contains all the needed information it can be transformed/downgraded to a "lesser" type and used as if it were that lesser type.

That's it. Now on to where that gets us:

DateTime and NaiveDateTime structs can be used with NaiveDateTime.compare/2, because both contain sufficient information to make up a NaiveDateTime. Any function in the NaiveDateTime module that expect a NaiveDateTime struct can also receive a DateTime instead. When a DateTime is provided, all of the extra data that the DateTime has compared to a NaiveDateTime is simply ignored.

To get the day of the week you could pass a DateTime, NaiveDateTime or Date to to Date.day_of_the_week/1. As they all contain a year, month, day, they can be cast to a Date and then be used. The functions in the Date module do not care about hours or time zones.

Imagine you have a DateTime from New York and another from London. One is 17:00 and another 18:00. If you just want to see which is greater/after in local time terms and don't care about time zones you can use NaiveDateTime.compare/2 instead of DateTime.compare/2.

Lesser types will not be "upgraded". They cannot automatically be transformed to types that require more information. E.g. Time cannot be used in the Date module because it does not contain year and so on. NaiveDateTime, Date or Time structs cannot be used in the DateTime module because they do not contain time zone information.

An example of this can be found in the Calendar library:

In the Calendar library there are protocols for types that can "contain" sufficient data to represent a date/time type. For instance for Date there is a protocol Calendar.ContainsDate :

defprotocol Calendar.ContainsDate do
  def date_struct(data)
end

and Date, DateTime and NaiveDateTime implements that:

defimpl Calendar.ContainsDate, for: DateTime do
  def date_struct(%{calendar: Calendar.ISO}=data), do: %Date{day: data.day, month: data.month, year: data.year}
end
defimpl Calendar.ContainsDate, for: NaiveDateTime do
  def date_struct(%{calendar: Calendar.ISO}=data), do: %Date{day: data.day, month: data.month, year: data.year}
end
defimpl Calendar.ContainsDate, for: Date do
  def date_struct(%{calendar: Calendar.ISO}=data), do: %Date{day: data.day, month: data.month, year: data.year}
end

In effect the data that is needed is taken and everything else is ignored. We don't need protocols to implement this principle, but I mention these protocols to illustrate the principle of "downgrading" types.

[josevalim]

Extremely helpful as always. Thank you @lau! :heart:

[josevalim]

Commit 35f492b746d0446b28274382ea359ffd1d55c278 allows NaiveDateTime and DateTime in most of the Date and Time functions as well as DateTime in some of the NaiveDateTime functions.