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

Some timezones breaks after migrating from pytz to zoneinfo #333

Closed
tobixen opened this issue Nov 16, 2021 · 7 comments
Closed

Some timezones breaks after migrating from pytz to zoneinfo #333

tobixen opened this issue Nov 16, 2021 · 7 comments

Comments

@tobixen
Copy link
Contributor

tobixen commented Nov 16, 2021

I just encountered a strange issue when running tests in my calendar-cli project. In particular, it broke with the DeNoronha timestamp (according to a contributor of test code to the caldav library, this place is well-known for being a "remote place", so it's used as a "remote time zone" in the tests). After migrating from pytz to zoneinfo the test code started breaking.

I wrote up the following code to highlight the breakage:

import datetime
import pytz
import zoneinfo
import icalendar

utc1 = pytz.utc
utc2 = zoneinfo.ZoneInfo('UTC')
oslo1 = pytz.timezone('Europe/Oslo')
oslo2 = zoneinfo.ZoneInfo('Europe/Oslo')
remote1 = pytz.timezone('Brazil/DeNoronha')
remote2 = zoneinfo.ZoneInfo('Brazil/DeNoronha')

for tz in (utc1, utc2, oslo1, oslo2, remote1, remote2):
    myevent = icalendar.Event()
    myevent.add('dtstart', datetime.datetime.now().astimezone(tz))
    print(myevent.to_ical().decode('ASCII'))

When running it, I get the following output:

BEGIN:VEVENT
DTSTART;VALUE=DATE-TIME:20211116T232629Z
END:VEVENT

BEGIN:VEVENT
DTSTART;TZID=UTC;VALUE=DATE-TIME:20211116T232629Z
END:VEVENT

BEGIN:VEVENT
DTSTART;TZID=Europe/Oslo;VALUE=DATE-TIME:20211117T002629
END:VEVENT

BEGIN:VEVENT
DTSTART;TZID=CET;VALUE=DATE-TIME:20211117T002629
END:VEVENT

BEGIN:VEVENT
DTSTART;TZID=Brazil/DeNoronha;VALUE=DATE-TIME:20211116T212629
END:VEVENT

BEGIN:VEVENT
DTSTART;TZID=-02;VALUE=DATE-TIME:20211116T212629
END:VEVENT

Note that for every time zone above, the output has changed simply from moving from pytz to zoneinfo. I would expect there to be no changes in the output, and I think the pytz behaviour is correct:

  • for timestamp suffixed with Zulu, no point to specify UTC.
  • The full name of the timezone should be given, and not the short name
  • In particular, "-02" is simply not an acceptable TZID.
tobixen added a commit to tobixen/icalendar that referenced this issue Nov 17, 2021
@tobixen
Copy link
Contributor Author

tobixen commented Nov 17, 2021

I entered debug mode and found a function tzid_from_dt that returns the wrong thing

(Pdb) tzid_from_dt(pytz.timezone('Europe/Oslo').localize(datetime.now()))
'Europe/Oslo'
(Pdb) tzid_from_dt(datetime.now().astimezone(zoneinfo.ZoneInfo('Europe/Oslo')))
'CET'

tobixen added a commit to tobixen/icalendar that referenced this issue Nov 17, 2021
@tobixen
Copy link
Contributor Author

tobixen commented Nov 17, 2021

I found the tzid_from_dt and managed to mend it so the new test code passes.

However, it will still be broken for time zones created through dateutil.tz. There seems to be no trivial way to find the full name of the time zone from a given timezone object produced by dateutil.tz.

Timestamps and time zones in Python is such a mess ...

tobixen added a commit to tobixen/icalendar that referenced this issue Nov 17, 2021
…le. Still quite much copied code - this could be refactored to reduce the number of code lines, but possibly on the cost of readability. Updates collective#333
tobixen added a commit to tobixen/icalendar that referenced this issue Nov 17, 2021
See collective#333 for details

Includes test code and changelog entry
@tobixen
Copy link
Contributor Author

tobixen commented Nov 17, 2021

New test run of the code above:

$ python ./test-ical.py
BEGIN:VEVENT
DTSTART;VALUE=DATE-TIME:20211117T150915Z
END:VEVENT

BEGIN:VEVENT
DTSTART;TZID=UTC;VALUE=DATE-TIME:20211117T150915Z
END:VEVENT

BEGIN:VEVENT
DTSTART;TZID=Europe/Oslo;VALUE=DATE-TIME:20211117T160915
END:VEVENT

BEGIN:VEVENT
DTSTART;TZID=Europe/Oslo;VALUE=DATE-TIME:20211117T160915
END:VEVENT

BEGIN:VEVENT
DTSTART;TZID=Brazil/DeNoronha;VALUE=DATE-TIME:20211117T130915
END:VEVENT

BEGIN:VEVENT
DTSTART;TZID=Brazil/DeNoronha;VALUE=DATE-TIME:20211117T130915
END:VEVENT

Still one issue here, the UTC time zone should not be displayed. I'll create a separate issue for that.

Replacing zoneinfo.ZoneInfo with dateutil.tz.gettz, the original issues still remain. I'll create a separate issue for that, too.

@NicoHood
Copy link
Contributor

NicoHood commented Jan 5, 2022

Does this library use pytz or zoneinfo? Because in the readme it states pytz and I was about to ask, if we can replace it with zoneinfo nowadays?

@tobixen
Copy link
Contributor Author

tobixen commented May 30, 2022

Zoneinfo is the way ahead, pytz is legacy.

geier pushed a commit to tobixen/icalendar that referenced this issue Aug 8, 2022
See collective#333 for details

Includes test code and changelog entry
@tobixen
Copy link
Contributor Author

tobixen commented Aug 21, 2022

I believe this bug can be closed, as #339 has been merged. Or should the issue stay open until a proper release has been done?

@niccokunzmann
Copy link
Member

niccokunzmann commented Aug 21, 2022

Hi, thanks again! I will close this when PRs get merged because issues are for development and the changelog for the releases. Thanks for asking!

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

No branches or pull requests

3 participants