Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Making icalendar.from_ical independent of pytz and usable with all timezone implementations #441

Closed
niccokunzmann opened this issue Oct 7, 2022 · 3 comments
Labels

Comments

@niccokunzmann
Copy link
Member

niccokunzmann commented Oct 7, 2022

At the present moment v5.0.0a2, I understand that icalendar components can be constructed as objects with any timezone implementation (pytz, zoneinfo, tz).

As pytz is more and more outdated,
I would like to be able to choose the timezone implementation that we use when we parse an ICS file.
At the moment, I believe that this is the step forward to make the icalendar step in up to the advances Python developers have made with the latest timezone implementations.

Proposal 1

Proposal for the interface:

Calendar/Event/Component
    .from_ical(..., timezone=pytz.tzinfo)
    .from_ical(..., timezone=tz.gettz)
    .from_ical(..., timezone=zoneinfo.ZoneInfo)

Any object returned from these calls should not use pytz but the timezone specified.

Proposal 2

As icalendar has a global state to parse timezones set to pytz, we could set an other global default:

icalendar.set_timezone_implementation(pytz.tzinfo)
icalendar.set_timezone_implementation(tz.gettz)
icalendar.set_timezone_implementation(zoneinfo.ZoneInfo)

The global default will be something like a lazy pytz (as proposed by #294) that imports pytz only if no other implementation is set.
Calling icalendar.set_timezone_implementation will set the default parameter from Proposal 1.

Implementation

As we use pytest to run the tests and @jacadzaca is already heavily refactoring the tests, see #400, we can use a parameter for parsing to pass the desired timezone implementation.
Parametrizing timezone implementations shows that pytz will stand out and always look a bit different.
The recurring-ical-events library shows that test parametrization can be used to run all tests for pytz and zoneinfo.

Datetime/Timezone implementations

These timezone implementations should be supported:

Related


Comments and ideas are very much appreciated on this as this is a deep change - feels like it to me. Also, I wrote this now without reading through all the related issues. Please comment.

@jacadzaca
Copy link
Collaborator

I like the first option better - if people would like to use pytz in one section (e.g. legacy code) and ZoneInfo in other (e.g. new code) they easily can.

@jacadzaca
Copy link
Collaborator

running grep 'import pytz' on the tests folder shows these tests still use it explicitly (in order of # of pytz usages):
test_unit_cal.py
test_recurrence.py
test_timezoned.py
test_unit_prop.py

@jacadzaca
Copy link
Collaborator

While refactoring test_timezone.py, I noticed that inside cal.py icalendar uses the pytz specific field _utc_transition_times, which seem to have an equivalent in ZoneInfo , but it's unaccesible e.g:

from zoneinfo import ZoneInfo

ZoneInfo('Europe/Warsaw')._trans_utc

throws an AttributeError: 'zoneinfo.ZoneInfo' object has no attribute '_trans_utc'

@collective collective locked and limited conversation to collaborators Aug 30, 2023
@niccokunzmann niccokunzmann converted this issue into discussion #532 Aug 30, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Projects
None yet
Development

No branches or pull requests

2 participants