Questions about timezone/ZoneInfo and fold attribute

[ariebovenberg]

👋 Hi there, I recently came across this nice project. As far as I know, it's the only one looking to make the aware/naive distinction at both runtime and type-checking 👏

Looking at the code though I had some questions:

1. The local timezone is cast to ZoneInfo here, but .tzinfo here always returns datetime.timezone instances.

heliclockter/src/heliclockter/__init__.py

Line 45 in 9d9443c

tz_local = cast(ZoneInfo, _datetime.datetime.now().astimezone().tzinfo)

I know because I've tried as well ;). timezone (e.g. CET) sadly doesn't account for variable UTC offsets (i.e. no DST), while ZoneInfo (e.g. Europe/Paris) does.
The solution could be to use the tzlocal library which helps determine the actual IANA timezone (i.e. ZoneInfo object) for the system. Pendulum uses this approach AFAIK.

2. The fold attribute is omitted from the constructor. Is this intentional? It's the only way to disambiguate times that occur twice is a timezone. It would be interesting if it could be omitted for the UTC datetime (which is never ambiguous), but required for IANA zoned datetimes.

😉 PS: I'm working on a datetime library of my own. Feel free to drop feedback if you like 🙂

[blubberdiblub]

1. The local timezone is cast to ZoneInfo here, but .tzinfo here always returns datetime.timezone instances.

heliclockter/src/heliclockter/__init__.py

Line 45 in 9d9443c

tz_local = cast(ZoneInfo, _datetime.datetime.now().astimezone().tzinfo)

I know because I've tried as well ;). timezone (e.g. CET) sadly doesn't account for variable UTC offsets (i.e. no DST), while ZoneInfo (e.g. Europe/Paris) does. The solution could be to use the tzlocal library which helps determine the actual IANA timezone (i.e. ZoneInfo object) for the system. Pendulum uses this approach AFAIK.

Not sure whether the casting to ZoneInfo is without problems, but aside from that as far as I'm aware .astimezone(tz=None) (where tz=None is the default when specifying no argument) is a special case and means to convert to the so-called "system" timezone (called "local" by pendulum), which is the thing handled by time.tzset(), time.timezone, time.altzone and time.daylight, respectively, and therefore does account for DST and does have a variable UTC offset (time.timezone vs time.altzone seconds) if the "system" timezone has it.

Or in other words, what heliclockter is doing here is getting the "system" (or "local") timezone and casting it to a ZoneInfo.

[ariebovenberg]

I didn't explain very well. Indeed tz=None results in the system timezone, but it gives a fixed offset instead of the actual IANA timezone (which may have variable offset).

example for me in Europe:

>>> from datetime import datetime, timezone, timedelta
>>> d = datetime(2023, 1, 1).astimezone(None)
datetime.datetime(2023, 1, 1, 0, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=3600), 'CET'))
# it's correct that this is the offset _at this time_, but it's fixed
>>> timezone.__doc__
'Fixed offset from UTC implementation of tzinfo.'
# if we move to summer the offset in europe should be +2hr instead of +1hr.
# however, because `timezone` is fixed offset, this isn't correctly reflected.
>>> in_summer = d + timedelta(days=150)
>>> in_summer
datetime.datetime(2023, 5, 31, 0, 0, tzinfo=datetime.timezone(datetime.timedelta(seconds=3600), 'CET'))
>>> in_summer.utcoffset()
datetime.timedelta(seconds=3600)  # incorrect. Should be +2hrs (CEST)

correct using IANA timezones

>>> d = datetime(2023, 1, 1, tzinfo=ZoneInfo("Europe/Paris"))
>>> d.utcoffset()
datetime.timedelta(seconds=3600)  # correct -- wintertime!
>>> in_summer_correct = d + timedelta(days=150)
>>> in_summer_correct.utcoffset()
datetime.timedelta(seconds=7200)  # correct -- summertime!

The difference is that CET and CEST (both fixed offset) are observed by "Europe/Paris". In the winter, it's CET and in the summer CEST. timezone object in Python cannot handle this variable offset, that is what differentiates it from ZoneInfo

[blubberdiblub]

@ariebovenberg
Yeah, I see what you mean now. However, I believe this is due to the broken dt + timedelta handling (or call it idiosyncrasy if you like - it's mentioned in the docs) of datetime.

Here's the example showing that the "system" timezone does in principle have varying UTC offsets (btw, I'm in Berlin, Germany, so CET/CEST here, too):

In [18]: dt = datetime(2023, 1, 1).astimezone(None); dt.utcoffset(), dt.tzinfo
Out[18]: 
(datetime.timedelta(seconds=3600),
 datetime.timezone(datetime.timedelta(seconds=3600), 'CET'))

In [19]: dt = datetime(2023, 7, 1).astimezone(None); dt.utcoffset(), dt.tzinfo
Out[19]: 
(datetime.timedelta(seconds=7200),
 datetime.timezone(datetime.timedelta(seconds=7200), 'CEST'))

Of course, for this to work at all (even in this limited form), the system itself needs to be properly configured to handle DST correctly. The above should work if the following works in the shell:

$ date --date='2023-01-01T12:00:00Z'                  
2023-01-01T13:00:00 CET
$ date --date='2023-07-01T12:00:00Z'
2023-07-01T14:00:00 CEST

[blubberdiblub]

Thinking a little more about it, I see now that I was wrong and I was misunderstanding what you were trying to say.
Mea culpa.

Basically, what you're saying is that once you attach datetime's "system" timezone to a datetime, it resolves to a static timezone (i.e. either one with DST or one without) at that point, its offset gets fixed at that time and can never change again. It's static now.

Whereas a real ZoneInfo has a dynamic offset.

Sorry again. I guess I got confused by my own implementation of a "system" timezone class, which datetime doesn't do. (mine is actually dynamic)

[ariebovenberg]

Indeed. It's a shame that astimezone() doesn't get us the dynamic IANA timezone. It'll probably stay like this for backwards compatibility, or the fact that it's nontrivial to determine (as evidenced by the tzlocal package). Although, it could be worth posting a ticket to CPython 🤔

what you mention about datetime + timedelta not taking DST into account is a whole separate pitfall 😬.

[ariebovenberg]

Here is an example that would probably need to be patched:

from heliclockter import datetime_local
from datetime import datetime

f1 = datetime_local.from_datetime(datetime(2024, 7, 1).astimezone())
f2 = datetime_local.from_datetime(datetime(2024, 1, 1).astimezone())
print(f1)
print(f2)

outputs (running the code in europe in winter):

2024-06-30 23:00:00+01:00
2024-01-01 00:00:00+01:00

And If we'd be running the code in summer, we'd get:

2024-07-01 00:00:00+02:00
2020-01-01 01:00:00+02:00

expected (year round):

2024-07-01 00:00:00+02:00
2024-01-01 00:00:00+01:00

Probably the datetime_local would need special logic to allow summertime/wintertime being different timezone values. Or: use the tzlocal package to bypass this entirely

[p-nilsson]

Hi @ariebovenberg thanks for your interesting questions! Let me try to elaborate on our choices.

1. The local timezone is cast to ZoneInfo here, but .tzinfo here always returns datetime.timezone instances.

Good point! This can definitely be seen as "wrong" since we are telling mypy the type is different than what it actually is.

The solution could be to use the tzlocal library which helps determine the actual IANA timezone (i.e. ZoneInfo object) for the system.

Very true. Before open sourcing this library we did use tzinfo internally at Channable, but one of the design decisions was to make this library work with no 3rd party dependencies to make it as easy to install and use for as many people as possible. This is also the reasoning for the optional pydantic integration. We need that at Channable, but didn't want to force other developers to install pydantic just to use this library.

2. The fold attribute is omitted from the constructor. Is this intentional?

Good question. Very true that this might cause slight hiccups around DST changes. We have been running this code in production for upwards of 3 years now without any noticeable issues, but that doesn't mean it shouldn't or couldn't change to make the library more robust. I don't recall any deliberate decision to omit fold from the constructor. Maybe @jochemb remembers as we originally co-wrote this logic.

All that being said, PRs to improve the library are always welcome, of course, but please do add tests for any new functionalities added.

[ariebovenberg]

Thanks for the elaboration. The decision to avoid third-party libraries makes sense, especially pydantic of course which is hugely complex. Note that pendulum seems to have completely vendored the tzlocal codebase, so that could also be an option for you to avoid 3rd party dependencies.

Regarding fold: it's not that omitting it from the constructor would lead to bugs (the fold attribute is always present behind the scenes after all). The issue is that you won't be able to express certain times using the constructor.

i.e. imagine we want to construct the datetime 2023-10-29 2:30 in Berlin, after the DST transition.

in the standard library:

d = datetime(2023, 10, 29, 2, 30, fold=1, tzinfo=ZoneInfo("Europe/Berlin"))

This issue is that by removing fold from the constructor, you've effectively removed functionality (seemingly) without reason.

[p-nilsson]

This issue is that by removing fold from the constructor, you've effectively removed functionality (seemingly) without reason.

I believe you are right. I have opened a PR to add fold: #13

Note that pendulum seems to have completely vendored the tzlocal codebase, so that could also be an option for you to avoid 3rd party dependencies

This does seem like a possible improvement and definitely a workaround to avoid 3rd party dependencies. Personally I don't have the time at the moment to do this though

[ariebovenberg]

Nice! 😁

@p-nilsson what are your thought on this though?

effectively this line will behave differently depending on whether you import it in summer or wintertime.

tz_local = cast(ZoneInfo, _datetime.datetime.now().astimezone().tzinfo) 

Imagine servers being automatically scaled up and down after a DST transition to summertime. newly deployed servers would get tz_local equal to CEST, while already deployed servers (for which the code is already imported), tz_local would be CET.

[p-nilsson]

Ah, sorry, indeed, I can agree that that can be an issue / lead to unexpected /wrong timestamps around DST. This is mainly a problem for long running processes though. Short lived processes won't really be affected by this, although the possibility is there of course.

I agree that it is something that can and should be improved, but preferrably not by introducing a 3rd party dependency like tzlocal. The fastest and easiest way might be to just copy the relevant code from tzlocal, as you suggest pendulum also does

[ariebovenberg]

Understandable. Although I do want to add that it's not only about long-running processes: Imagine a script used to generate a logistical planning for the upcoming week that you run each Friday. That script would generate timestamps 1 hour off for two weeks in the year 🤔

[p-nilsson]

Yes, I agree 👍 assuming you schedule that script to run around the time change

Again, I definitely agree it can be a problem, and it is worthy of an improvement, just that with our use case it never really has been in the 3 years we have been running this code in production

[ariebovenberg]

Ok 👍. Thanks for indulging me with these questions 😁 . Feel free to close the issue when you like.