ariebovenberg / whenever

⏰ Modern datetime library for Python
https://whenever.rtfd.io
MIT License
898 stars 15 forks source link

Probably a misleading warning #169

Closed ArneBachmannDLR closed 2 months ago

ArneBachmannDLR commented 2 months ago

The error message mentions LocalDateTime, but there is none involved, or is it? OffsetDateTime has no information on DST anyway, so I don't understand the error message.

> OffsetDateTime(2020,1,1,12,offset=0).add(DateTimeDelta(days=1))
*** whenever.ImplicitlyIgnoringDST: Adding time units to a LocalDateTime implicitly ignores Daylight Saving Time. Instead, convert to a ZonedDateTime first using assume_tz(). Or, if you're sure you want to ignore DST, explicitly pass ignore_dst=True.
ariebovenberg commented 2 months ago

Indeed there is a typo (Local should be replaced with Offset), but otherwise the warning is correct. With an OffsetDateTime (even with offset 0), you don't strictly "know" what the offset will be a day later. For example, if "+0" meant London, one day later it may be "+1" instead. This is such a common oversight that it warrants an error.

I am curious though: what's your use case? Is the offset always 0? If so, Instant might be a better fit since it is explicit about not caring about any "local" representation of time.

>>> Instant(...).add(hours=24)
>>> OffsetDateTime(...).add(days=1, ignore_dst=True)

edit: clarification

ArneBachmannDLR commented 2 months ago

I think the reason was that at another place I tried to call Instant.time() which didn't work, so I changed to Offset(0).

Also I get TypeError: add() takes no positional arguments

Oh got it, I cannot add(days=...), only hours!

Background: I compute time slots as (days-since-epoch, quarter hour slot of day), ignoring any time shifts and timezones (all in UTC). There will never be 23 or 25 hour days in that model, so I can ignore most of the complex handling, but need to convert to and from timezones when displaying and inputting data from the user session (web browser)

ariebovenberg commented 2 months ago

Oh got it, I cannot add(days=...), only hours!

Yes this is intentional—but I agree it's a bit awkward. The reason for this is to make it explicit that Instant doesn't deal in "local" time units such as years, months, days, etc. However, you could also make the point that "day" is always 24 hours in Instant, and is thus allowed.

I compute time slots as (days-since-epoch, quarter hour slot of day), ignoring any time shifts and timezones (all in UTC). There will never be 23 or 25 hour days in that model, so I can ignore most of the complex handling, but need to convert to and from timezones when displaying and inputting data from the user session (web browser)

In that case, I'd recommend working with Instant in your code to compute the time slots, and using to_tz() for displaying in the browser. This assumes the calculation of time slots is independent of any location or local time.

ariebovenberg commented 2 months ago

call Instant.time() which didn't work, so I changed to Offset(0).

Not sure I understand entirely. Getting the time() component from Instant is indeed impossible, as time-of-day is locally defined and Instant explicitly ignores local considerations. That said, if you do want the UTC time-of-day (this should only be required at the 'edges' of your program), you can do my_instant.to_fixed_offset(0).time()

ariebovenberg commented 2 months ago

Also relevant: I try to explain this here. If you have any ideas to make the docs clearer, they are also welcome

ArneBachmannDLR commented 2 months ago

Thanks! I think I got it working again with reduced number of conversions and also found the to_fixed_offset(0) trick.

FWIW, I was surprised by incompatible changes to the API twice already, how can I stay informed on changes?

ariebovenberg commented 2 months ago

You needn't be surprised 🙂 , the README and documentation front page have a section called "versioning and compatibility policy" which should tell you everything you need to know. Of course, if you have a suggestion to clarify this, let me know.

ariebovenberg commented 2 months ago

In the latest release, the typo was fixed and a link was added to the docs