Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restrict timezone names for tests #775

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

niccokunzmann
Copy link
Member

This is intended to be a fix for #763.

@niccokunzmann
Copy link
Member Author

niccokunzmann commented Feb 6, 2025

icalendar.timezone.tzid.tinfo2tzids (which I added) would provide us with a timezone name for a timezone even if this timezone is not easily recognized as having one, so we can generate timezones if needed.

Now, the code assumes that timezones in the past would not change.
But they do! Even between 1970 and 2000.

Here is an example of how the timezone definitions are different between each other, using EET which is +2 hours away from UTC.

>>> 60*60*2  # 2 hours offset
7200
>>> from dateutil.tz import gettz
>>> from pytz import timezone
>>> from zoneinfo import ZoneInfo
>>> import datetime
>>> d = datetime.datetime(1983, 3, 27, 3, 0)
>>> tz1 = gettz("EET")
>>> tz1.utcoffset(d)
datetime.timedelta(seconds=10800)  # wrong offset
>>> tz2 = ZoneInfo("EET")
>>> tz2.utcoffset(d)   # correct offset
datetime.timedelta(seconds=7200)
>>> tz3 = timezone("EET")
>>> tz3.utcoffset(d)  # no offset
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/nicco/icalendar/.tox/py39/lib/python3.9/site-packages/pytz/tzinfo.py", line 425, in utcoffset
    dt = self.localize(dt, is_dst)
  File "/home/nicco/icalendar/.tox/py39/lib/python3.9/site-packages/pytz/tzinfo.py", line 344, in localize
    raise NonExistentTimeError(dt)
pytz.exceptions.NonExistentTimeError: 1983-03-27 03:00:00

So, we can determine pytz and zoneinfo easily because they have a name in them. Dateutil timezones do not have this.

I wonder how to really go about this and will open an issue for more guidance.

@niccokunzmann
Copy link
Member Author

@jacadzaca @geier @mauritsvanrees, the main branch fails because I added tests based on the assumption that timezone definitions will not change between 1970 and 2000. This is wrong but a good learning. This PR will enable us to have a green main branch again until there is an update. Could you please have a look at this and see what it takes to merge a fix? Otherwise all other PRs from main from not on will fail.

Options chosen here:

  • remove tests for failing (changed) timezone definitions
  • remove all tests for guessing timezone names
  • adjust assumptions for the current release of timezone definitions in pytz, zoneinfo and dateutil

@niccokunzmann
Copy link
Member Author

niccokunzmann commented Feb 6, 2025

10bd46d addresses the problem that around a change of a timezone we get different results for different implementations.

What happens after merge is that we can rest until the next update of the timezone database.

@niccokunzmann
Copy link
Member Author

See also #780

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant