Support :week or :day units in Date.add/3

[sorentwo]

This adds Date.add/3, which accepts an optional unit value. The only acceptable units are :day and :week and it defaults to :day.

Unlike the recent changes to Time, this doesn't change Date.diff/2 because it doesn't currently accept any units.

[kipcole9]

Why only Calendar.ISO? We are heading down a path in which a generalised Calendar behaviour is no longer generalised but specialised. In thats the intention, then lets remove the behaviour and just make the Date/Time/DateTIme modules only for Calendar.ISO. There has been previous conversations around adding the concept of weeks to Calendar behaviour with the conclusion that this was not easily generalised. If weeks are now back on the drawing board should we not make weeks a full partner in the Calendar behaviour and solve the problem generally?

[josevalim]

As @kipcole9 said, we cannot add :week behaviour easily. Not only we would need to know how long a week lasts for each specific calendar, some calendars have a variable number of days per week. This means adding weeks can lead to dates that do not exist (such as adding a month to 31st January). I believe it is best to close this for now. We will likely address how to handle months before we address weeks. Thanks @sorentwo!

@kipcole9 I think the temptation to add Calendar.ISO specific behaviour is natural, we may need to help to keep us honest, so your input is appreciated. :)

[sorentwo]

Thanks for chiming in on this @kipcole9. Honestly the :week unit isn't particularly important to me. I'm far more interested in getting :minute/:hour/:day conversions into NaiveDateTime and DateTime. If there aren't any issues with those units I'll have the PR for those next.

[josevalim]

Wait a second. All of the add operations are already tied to the Calendar.ISO, so we can probably merge this branch because we are not adding new restrictions. Currently there is no way to add to a calendar type without using a ISO based unit. So I think we should go ahead with this and later figure out a way of making add generic.

@sorentwo can you please bring your branch back so we can merge it?

[josevalim]

Well, here is another concern though. Doing:

time
|> Time.add(1, :hour)
|> Time.add(30, :minute)

is not going to be efficient because we will need to convert these representations back and forth multiple times.

Perhaps it is best to add a shift function:

Time.shift(time, hours: 1, minutes: 30)

Or similar that can do all operations at once. We can delegate the interpretation of each key-value pair to the underlying calendar. We keep the add functions as is (and based on their underlying ISO counter-parts).

The only remaining question is: if we do have shift, do we need to add :week, :hour and friends to the add functions?

[wojtekmach]

Are there calendars where 1 day != 24 hours?

I think adding :day but not adding :week is a good start, we can always add the latter later. If 1 day is always 24 hours, then I think the logic is not ISO specific.

I also think it's a great point about time |> Time.add() |> Time.add() vs Time.shift().

[sorentwo]

Ok, I've restored the branch while we continue the discussion.

---

I really like the readability of Time.shift because it clearly identifies the units. Of course you can avoid repeated calls to add by turning 1 hour 30 minutes into 90 minutes, but that is counter to the spirit of this entire effort.

If we add Time.shift/2 instead I would vote to revert/undo the Time.add/3 changes. Also, would we add shift/2 to Time, Date, DateTime and NaiveDateTime?

[kipcole9]

The Hebrew calendar has a different notion of time although as I understand it civil time is most commonly used. From wikipedia:

In Judaism, an hour is defined as 1/12 of the time from sunrise to sunset, so an hour can be less than 60 minutes in winter, and more than 60 minutes in summer. This proportional hour is known as a sha'ah z'manit (lit. a time-related hour). A Jewish hour is divided into 1080 halakim (singular: helek) or parts. A part is 3⅓ seconds or 1/18 minute.

I am building a Mars calendar in the background, its day is not 24 hours (but I'm not for a moment suggestion non-earth calendars are in scope).

[kipcole9]

If the approach of Time.shift/2 which seems a solid approach, is the preferred candidate would you consider formalising then the idea of a "duration" since that is ultimately what is being represented in this case. That would now - or in the future - permit the idea of a difference calculation to return a duration which could then be applied to a another date/time.

[wojtekmach]

@kipcole9 re: Mars, :D (I think we should support it!)

Perhaps whether 1 day is or is not 24h is moot because we have date.day field that we'd increment. That would be another reason why I'd hold off on supporting :week, we don't have such field on the struct and so we necessarily need to calculate it somehow. I guess we don't have day field on the %Time{} struct either but it happens so that a %Time{} represents time in a day, so I believe for all time <- time(), days <- integer(), do: time == Time.shift(time, days: days)

[wojtekmach]

Good point about formalising the idea of duration, I always thought we'd eventually have something like this:

def shift(time, %Duration{} = duration) do
  ...
end

def shift(time, duration) when is_list(duration) do
  shift(time, struct!(Duration, duration))
end

[josevalim]

The biggest problem with periods/durations is on how we want to handle multiple calendars and timezones. Adding 1 day + 12 hours is not the same as adding 36 hours if there is daylight saving time. Java solves this problem by having two classes: periods and durations. One is only time aware, so 1 day is always 24 hours, while the other is date-aware and respects timezones.

So when talking about periods/durations, we need to be clear about which approach we are advocating for. In Java, both accept only ISO units, even though there is support for multiple calendars.

Implementation-wise, going with the time-based variant is much simpler. The internal representation is also straight-forward, it could be as simple as a tuple with {seconds, microseconds}.

@kipcole9 is it correct to say that a duration in a calendar always have an equivalent duration in another calendar? Because if so, supporting multiple calendars for a time-based duration is always a matter of simply doing this:

Time.add(time, Mars.Duration.to_iso_seconds(days: 1, hours: 3))

In any case, it seems that the best option is to not add these concerns to Time.add. At best, Time.add can support the {seconds, microseconds} tuple as argument and have all different calenders emit such normalized value when working with time-based durations.

For now, I will revert the previous addition and we can explore how to add time-based durations next.

[NickNeck]

Hello, I released Tox yesterday. The library deals with the topic of this issue. I have seen this topic before I release Tox, but at this time, it was closed. Maybe Tox can be used as an example of what we want and what we don't want.

To simplify things, the library assumes that every week consists of 7 days. I think that fits for many calendars. But it would be nice to make weeks a full partner in the Calendar behaviour and solve the problem generally, as @kipcole9 mentioned above.

Tox knows durations and periods. Duration is specified as a keyword list like [year: 1, month: 1, week: 1, minute: 1]. Period is specified as a struct with the same keys as durations.

Tox defines an add/2 function for Tox.DateTime, Tox.NaiveDateTime, Tox.Date, and Tox.Time. The following strategy is used to add and subtract months. When adding and subtracting months, the day may be updated.

iex> Tox.Date.add(~D[2020-01-31], month: 1)
~D[2020-02-29]

In other calendars, this can result in very different time differences.

iex> date = Date.convert!(~D[2020-09-05], Cldr.Calendar.Coptic)
~D[1736-12-30 Cldr.Calendar.Coptic]
iex> Tox.Date.add(date, month: 1)
~D[1736-13-05 Cldr.Calendar.Coptic]
iex> Tox.Date.add(date, month: 2)
~D[1737-01-30 Cldr.Calendar.Coptic]
iex> Tox.Date.add(date, month: 1) |> Date.diff(date)
5
iex> Tox.Date.add(date, month: 2) |> Date.diff(date)
35
iex> date |> Tox.Date.add(month: 1) |> Tox.Date.add(month: 1) |> Date.diff(date)
10

Durations will always be added from the largest to the smallest unit.

iex> datetime = DateTime.from_naive!(~N[2000-01-30 00:00:00], "Europe/Oslo")
iex> Tox.DateTime.add(datetime, month: 1, day: 1)
#DateTime<2000-03-01 00:00:00+01:00 CET Europe/Oslo>
iex> datetime |> Tox.DateTime.add(month: 1) |> Tox.DateTime.add(day: 1)
#DateTime<2000-03-01 00:00:00+01:00 CET Europe/Oslo>
iex> datetime |> Tox.DateTime.add(day: 1) |> Tox.DateTime.add(month: 1)
#DateTime<2000-02-29 00:00:00+01:00 CET Europe/Oslo>

[LostKobrakai]

The biggest problem with periods/durations is on how we want to handle multiple calendars and timezones. Adding 1 day + 12 hours is not the same as adding 36 hours if there is daylight saving time. Java solves this problem by having two classes: periods and durations. One is only time aware, so 1 day is always 24 hours, while the other is date-aware and respects timezones.

This is exactly why I like @wojtekmach's calendar_interval and the underlying concepts.

import CalendarInterval
datetime |> enclosing(:date) |> next() |> nest(:hour) |> next(12)
# vs.
datetime |> enclosing(:hour) |> next(36)

Those simply are not the same operations, but people like to think they're the same.

[dotdotdotpaul]

Maybe it's just me, but if I'm adding "1 day" to a DateTime, my gut instinct is that it's 24 hours. If the actual DateTime produced is +/-1 hour from the previous DateTime (due to stoopid Daylight Savings) I don't care too much. As a dev, I'm always working in UTC anyway, what "DateTime" it really is, is UI dressing. Almost all of my timers end up looking like "24 * 60 * 60 , :second" and I would way rather be able to do "1, :day" or "3, :hour" when I'm add()ing... I realize that's a problem for anything that can't be resolved to UTC before being operated on... But I'm not sure how you add/subtract anything from a Date or Time that isn't rooted in UTC -- seconds or otherwise.

[LostKobrakai]

But I'm not sure how you add/subtract anything from a Date or Time that isn't rooted in UTC -- seconds or otherwise.

It's nice if you can treat datetimes like you described, but this doesn't work if datetimes are meant for humans in places and not for computers. Sometimes adding "1 day" might mean "send me this notification again tomorrow" a.k.a. next day, same (wall-)time as today in the users timezone. There's a difference between adding a fixed chunk of time (DateTime.diff always the same) and doing something like my example, where the actual time difference is irrelevant, but consistent wall time is expected. I feel calendar_interval makes those different approaches quite clear.

[dotdotdotpaul]

If you're talking about human stuff, IMHO, that's not a standardizable kind of thing, whereas 1 week = 7 days, 1 day = 24 hours, 1 hour = 60 minutes, 1 minute = 60 seconds -- those are pretty constant. There IS precedent for this in legacy languages and libraries -- for example, how do SQL databases do it? it's not standard SQL, but the feature seems pretty standard -- MySQL, for example: https://dev.mysql.com/doc/refman/8.0/en/date-and-time-functions.html#function_timestampadd supports years, weeks, days, hours, minutes, even "quarters" (and I'm not entirely sure how you compute that last one). I guess when I think about adding time, I look to the consistent legacy of time-adding functions and libraries that came before. 😄