Skip to content

Navigation Menu

Sign in
Sign up

Add a PyICU timezone provider (#692) - #1755

Draft
h4syy wants to merge 1 commit into
collective:main from
h4syy:feat/692-pyicu-timezone-provider
Draft

Add a PyICU timezone provider (#692) #1755
h4syy wants to merge 1 commit into
collective:main from
h4syy:feat/692-pyicu-timezone-provider

Conversation

@h4syy

@h4syy h4syy commented Sep 2, 2026

Copy link
Copy Markdown

Linked issue

Description

Adds a PyICU timezone provider — a PYICU implementation of TZProvider, selectable via use_pyicu() and wired through TZP and the icalendar.timezone package. This is part 1 of #692 and is opened as a draft / work in progress.

Working so far (verified locally against the repo's VTIMEZONE fixtures — 37/38 create_timezone succeed):

  • timezone lookup via icu.ICUtzinfo, UTC handling, and knows_timezone_id
  • unknown ids return None, so the Windows→Olson and globally-unique-TZID fallbacks in TZP still work
  • create_timezone parses the VTIMEZONE via icu.VTimeZone.createVTimeZone (DST transitions correct); a VTIMEZONE that ICU cannot parse raises ValueError instead of segfaulting the interpreter

Still to do (follow-up commits): parametrize conftest.py across the pyicu backend, add PyICU-specific tests, declare the optional PyICU test dependency (gated so it skips when ICU is absent), resolve uses_pyicu(), and lint/type-hint cleanup.

Questions I'd love input on, @niccokunzmann (I'll also add inline comments on the lines):

  1. create_timezone uses icu.VTimeZone.createVTimeZone as you suggested. One fixture — issue_321_assert_dst_offset_is_not_false.ics (Europe/Berlin) — is rejected by ICU (empty id) where zoneinfo/pytz succeed. Prefer a fallback for those, or an xfail for pyicu?
  2. uses_pyicu(): add it to the TZProvider ABC + TZP proxy for symmetry with uses_pytz/uses_zoneinfo, or keep providers reporting only pytz/zoneinfo?
  3. PyICU is hard to install (especially in CI). How would you like the optional dependency and test-skipping handled?

Checklist

  • I added a change log entry, following the instructions in all subsections under Change log requirements.
  • I followed icalendar's Artificial intelligence policy and disclosed my Responsible AI use in my commit messages, if applicable.
  • I added or updated tests, if applicable.
  • I ran and ensured all tests pass locally by following Run tests.
  • I added or edited documentation as necessary, both as docstrings to be rendered in the API documentation and narrative documentation, following the Style guide.

Additional information

Draft / WIP — tests, docs, and the conftest parametrization are coming in follow-up commits.

AI disclosure: I used Claude (Opus 4.8, via Claude Code) to explain the timezone-provider architecture and to verify the provider's behaviour by running it; the code is my own. This is also noted in the commit message and the news/692.feature entry.

Add a PYICU implementation of TZProvider backed by PyICU/ICU, plus
use_pyicu() wiring in TZP and the icalendar.timezone package.
Working so far: timezone lookup via icu.ICUtzinfo, UTC handling,
knows_timezone_id, unknown id -> None, and create_timezone via
icu.VTimeZone.createVTimeZone. A VTIMEZONE that ICU cannot parse raises
a ValueError instead of crashing the interpreter.
Still TODO (follow-up commits): parametrize conftest.py over the pyicu
backend, add PyICU-specific tests, declare the optional test dependency,
and lint/type-hint cleanup.
AI use disclosure: I used Claude Opus 4.8 (via Claude Code) to explain the
icalendar timezone provider architecture and to verify the provider's
behaviour by running it. I wrote the code myself.
Refs collective#692 
@github-actions github-actions Bot added the ai-suspicion This contribution is possibly created with lots of AI help without enough human understanding. label Sep 2, 2026

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

This pull request did not pass quality checks and AI use is suspected. Please review Contribute and make any necessary amendments.

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Profile summary:

GitHub user: h4syy
🟢 No concerns found with user's profile.
🟢 No concerns found with recent PR activity.
🟢 No concerns found with recent issue activity.
For a more detailed report, run `gh-profiler h4syy`.
Full profile
GitHub user: h4syy
🟢 No concerns found with user's profile.
 🟢 Account age: 5 years
 🟢 Profile information:
 name: Yash Paudel
 company: Khalti
 linkedin: https://www.linkedin.com/in/yashpaudel/
 Empty fields: blog, location, email, bio
🟢 No concerns found with recent PR activity.
 2 PRs opened in the last 21 days.
 0 opened against repos the user owns.
 0 opened against repos in publicly associated orgs.
 2 opened against external repos.
 🟢 1 of 2 external PRs merged in the last 21 days.
 🟢 0 of 2 external PRs closed without merging in the last 21 days.
🟢 No concerns found with recent issue activity.
 6 new issues opened in the last 21 days.
 6 opened in repos the user owns.
 0 opened in repos in publicly associated orgs.
 0 opened in external repos.
 🟢 0 external issues closed as NOT_PLANNED.
 🟢 0 external issues opened with the same title.

niccokunzmann commented Sep 2, 2026
edited
Loading

Copy link
Copy Markdown
Member

Hi, thanks for the PR!

Some answers:

uses_pyicu() - yes, they all should have the same interface.

issue_321_assert_dst_offset_is_not_false.ics - if a fix cannot be done because it is too much for this PR, then skipping is a good way to work this this (and preferably adding a TODO in the code or an issue if it is important). Best would be to somehow fix this - either in icalendar or in PyICU itself - report this upstream. Generally all files should work with all timezones. The reason is that we collect the different quirks and wild (mis-)interpretations of the RFC standard and want to take the burden off developers to deal with these. So, that special case is best handeled in a consistent way with the others by calendar, so there is no deviation. If you think, this should be handled by PyICU instead of icalendar, create an issue in PyICU to fix it and add a skip with a reference to that issue so it can be removed when the issue is closed.

If it is hard to install, then, I would say the tests should check - maybe like with pytz - wether pyicu is installed. It is also a question of documentation but some kind of switch is required, so they can be switched on. Is it hard to install PyICU on linux? What is the easiest way to install it?

Let me know when you would like me to review this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@niccokunzmann niccokunzmann Awaiting requested review from niccokunzmann niccokunzmann will be requested when the pull request is marked ready for review niccokunzmann is a code owner
@stevepiercy stevepiercy Awaiting requested review from stevepiercy stevepiercy will be requested when the pull request is marked ready for review stevepiercy is a code owner
@angatha angatha Awaiting requested review from angatha angatha will be requested when the pull request is marked ready for review angatha is a code owner

At least 1 approving review is required to merge this pull request.

Labels

ai-suspicion This contribution is possibly created with lots of AI help without enough human understanding.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /