Add DateTime/NaiveDateTime.beginning_of_day and DateTime/NaiveDateTime.end_of_day

[thiamsantos]

Addresses https://groups.google.com/g/elixir-lang-core/c/dfAn00VCA9Q

[josevalim]

Hi @kipcole9, could you also please review this PR and let us know your thoughts? Thank you!

[wojtekmach]

Is it safe for datetimes with non-utc tz? I remember on daylight saving clock change some datetimes don’t actually exist and maybe there are time zones when these invalid datetimes would happen to be beginning or end of day?

[wojtekmach]

Could you add unit tests for precision? inputs like (…)09:00:00 and (…)09:00:00.123

[josevalim]

Ah, good point. This is timezone aware, which adds a reasonable amount of complexity here.

[NickNeck]

Here are two examples for daylight saving changes on a start of a day.

~N[2011-04-03 12:00:00]
|> DateTime.from_naive!("Africa/El_Aaiun")
|> Tox.DateTime.beginning_of_day()
=> #DateTime<2011-04-03 01:00:00+01:00 +01 Africa/El_Aaiun>

datetime = DateTime.from_naive!(~N[2020-10-25 12:00:00], "America/Scoresbysund")
=> #DateTime<2020-10-25 12:00:00-01:00 -01 America/Scoresbysund>
Tox.DateTime.beginning_of_day(datetime)
=> #DateTime<2020-10-25 00:00:00+00:00 +00 America/Scoresbysund>

And one example for end of the day.

~N[1916-06-14 12:00:00]
|> DateTime.from_naive!("Africa/Algiers")
|> Tox.DateTime.end_of_day()
=> #DateTime<1916-06-14 22:59:59.999999+00:00 WET Africa/Algiers>

[kipcole9]

Not all calendars start the day at 00:00:00. Julian calendar starts the day at 12:00:00. Islamic and Jewish calendars start of day is sunset. So there is definitely complexity for some calendars and it can't be assumed that the difference between start of day and end of day is 24 hours (DST already breaks that assumption anyway).

Nevertheless, by definition each day has a beginning and and end so its better that calendars resolve that complexity than developers and I don't see any insurmountable issues with the PR.

The main consideration in my mind, for calendar writers, is that the date components are calendar relative. But time components are always understood to be midnight to midnight.

For example, in an Islamic calendar the calendar day today is 1444-08-10. But all time references in Elixir are still midnight to midnight.

Even though in Istanbul today, where sunset is 18:57pm, I believe the practise would not be to say Islamic 1444-08-10 00:00:00 (Gregorian 2023-03-03 18:57+03) rather it would be to say 1444-08-10 18:57:00+03.

That calendar dates are calendar relative but times are always relative to midnight would be worth documenting on the Calendar module.

[josevalim]

Thank you @kipcole9! Your expectations pretty much match my mine. My only concern is that I don't know how to tackle this timezone wise. Especially in the sunset cases, where the calendar can only return the sunset if it knows the geographical area it is in.

[thiamsantos]

If we only introduce the functions on NaiveDateTime, and avoid dealing with timezones. We could avoid the complexity to make the calendar aware of the timezone?

[josevalim]

@thiamsantos it may be the opposite, we can't implement this without timezone information. Or do we have multiple calendars for each timezone in said cases? Thoughts @kipcole9?

[kipcole9]

I think the only issue with timezones would be if the timezone change occurs at midnight in which case the situation is the same as any tz conversion - the result can be ambiguous (two occurrences of the same time). In which case beginning_of_day and end_of_day would need to return an ambiguous result in such cases.

[josevalim]

So my suggestion is to add it only to NaiveDateTime. If someone wants to convert a DateTime to its beginning of day, then they do:

datetime
|> NaiveDateTime.beginning_of_day()
|> DateTime.from_naive(datetime.timezone)

and the docs for NaiveDateTime can say as much. WDYT?

[kipcole9]

I think that’s nice and simple and straight forward for calendar developers too.Sent from my iPhoneOn 4 Mar 2023, at 10:47, José Valim ***@***.***> wrote: So my suggestion is to add it only to NaiveDateTime. If someone wants to convert a DateTime to its beginning of day, then they do: datetime |> NaiveDateTime.beginning_of_day() |> DateTime.from_naive(datetime.timezone) and the docs for NaiveDateTime can say as much. WDYT? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[josevalim]

@thiamsantos can you please do these changes:

1. Add precision tests (tests, not doctests :))
2. Keep it only on NaiveDateTime
3. Add the docs I mentioned above for DateTime usage

Thank you!

[thiamsantos]

@josevalim changes pushed.

Thank you all for the feedback and thoughts :)

[josevalim]

Btw, can you push similar docs to beginning_of_week and end_of_week?

[wojtekmach]

the fact it's {999_999, 0} might lead to surprising behaviour:

import ExUnit.Assertions
start = ~N[2023-01-01 00:00:00]
assert NaiveDateTime.end_of_day(start) == 
       start |> NaiveDateTime.add(1, :day) |> NaiveDateTime.add(-1, :second)

** (ExUnit.AssertionError)

Assertion with == failed
code:  assert NaiveDateTime.end_of_day(start) ==
              start |> NaiveDateTime.add(1, :day) |> NaiveDateTime.add(-1, :second)
left:  ~N[2023-01-01 23:59:59]
right: ~N[2023-01-01 23:59:59]

the difference is:

iex> NaiveDateTime.end_of_day(start).microsecond
{999999, 0}

iex> start |> NaiveDateTime.add(1, :day) |> NaiveDateTime.add(-1, :second) |> Map.fetch!(:microsecond)
{0, 0}

We'd see similar behaviour for different precisions e.g. milliseconds.

I'm honestly not sure what should be the behaviour here.

[wojtekmach]

one way to look at it is we should be able to write the assertion under question using just sigils:

assert NaiveDateTime.end_of_day(~N[2000-01-01 23:00:07]) == ~N[2000-01-01 23:59:59]

and the fact we can't is a reason to revisit this?

[josevalim]

I created a new issue. We should probably truncate it based on the precision: {(10 ** precision) - 1, precision}

[thiamsantos]

I just opened a PR to fix this #12450.

[thiamsantos]

Btw, can you push similar docs to beginning_of_week and end_of_week?

@josevalim What you have in mind for this? As far as I understood the we can incur in problems when dealing with the time that a day starts/ends. As we only have Date.beginning_of_week and Date.end_of_week we don't need to worry about this.

[josevalim]

You are right, we can skip it for now!