ariebovenberg / whenever

⏰ Modern datetime library for Python, available in Rust or pure Python
https://whenever.rtfd.io
MIT License
831 stars 13 forks source link

rationale for 5 different DateTime subclasses? #59

Closed bxparks closed 7 months ago

bxparks commented 7 months ago

I read the Overview document (https://whenever.readthedocs.io/en/latest/overview.html). Maybe I missed something, but I don't understand the need for 5 different DateTime classes:

The following 3 should be enough: NaiveDateTime, OffsetDateTime and ZonedDateTime. No?

ariebovenberg commented 7 months ago

This is definitely a question that we can expect to be asked frequently.

Here is my quick-n-dirty rationale:

Like you mention, these three are the clearest:

here's my case for the others:

bxparks commented 7 months ago
* `UTCDateTime` — it's indeed its a single case of `OffsetDateTime`, but IMHO deserves its own class because:

  1. UTC is so frequently used, a specially named class communicates the developers intention to normalize to UTC
  2. UTC-only classes are faster to operate on and more efficient
  3. A UTC-only class can exist in fewer states. Only the exact same fields can represent the same moment in time

  The main critique of `UTCDateTime` is that it could be replaced with `Instant` as we see in Temporal and NodaTime, with the same advantages.

Ah, so you are using UTCDateTime as a substitute for Instant. Maybe the documentation can make that more clear. A point of confusion is that there are now 3 other ways to represent UTCDateTime in whenever:

1) OffsetDateTime(offset=0) 2) ZonedDateTime(tz="Etc/UTC") 3) epoch_seconds

It's not clear to me that increasing the API surface area (hence complexity) is worth it.

* `LocalDateTime`—it's _not_ just `ZonedDateTime` with the current timezone, for two reasons:

  1. it's not (always) possible to get the IANA timezone name of the current system (see `tzlocal` package). It _is_ possible to call the system APIs to account for DST transitions
  2. The system timezone can change.

My initial thought is that LocalDateTime (which I would rename SystemDateTime) belongs in a layer above the whenever library. The library itself ought to be focused on just the fundamentals: the "epoch seconds", "UTC offsets", and "Time Zones", and various conversions and operations.

It seems like what you need is a SystemClock class that has the now() method that returns "epoch seconds" and a getSystemDateTime() that returns a SystemDateTime object that contains whatever system TimeZone class is required (it doesn't have to be an IANA TimeZone). If the system timezone changes, then the user must call getSystemDateTime() again to get the new timezone. But if you do that, you don't need a SystemDateTime. You just need a ZonedDateTime.

ariebovenberg commented 7 months ago

Ah, so you are using UTCDateTime as a substitute for Instant

The whole discussion on whether to have Instant or not probably belongs in its own issue. The fact that most modern libraries have this, is a strong signal for its usefulness. Having both UTCDateTime and Instant would probably be unnecessary

there are now 3 other ways to represent UTCDateTime in whenever:

all things equal, having multiple ways to do things isn't good. however, having a dedicated class for a very common case can allow for a really clean API—even if it could be represented in another more general way. An analogy: you can use int to express boolean logic (0 and 1), but there is a specific class bool that is more precise. Both int and bool have a right to exist, just like UTCDateTime and OffsetDateTime.


Regarding LocalDateTime, perhaps we can continue this discussion in #30.

ariebovenberg commented 7 months ago

Regarding the need for a separate UTC class, there is an interesting analysis from Rust's Chrono (https://github.com/chronotope/chrono-rfcs/issues/3#issue-388096479)

ariebovenberg commented 7 months ago

Closed this issue since further discussion will take place in the issues linked above ☝️

bxparks commented 7 months ago

(Meta Comment: One of the problems of using GitHub Issues for discussion topics is that after the ticket is closed, it basically disappears from future readers. The default search option on GitHub is is:issue is:open. Almost no one removes the is:open to search for closed Issues. (A recent example that illustrates this is stub42/pytz#119, about GMT offsets, which had at least 4 duplicates). On my projects, I use the GitHub Discussions tab, so that people can more easily find and read previous discussions. I use Issues strictly for bugs and tracking tasks. Anyway, it's your project, totally up to you.)

ariebovenberg commented 7 months ago

I'm indeed aware. GitHub discussions are theoretically the place where you should move discussions. However:

Of course, I'm totally willing to reconsider if discussions in issues become unmanageable