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.

Sign up for GitHub

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

Jump to bottom

Use system provided tzdata as a priority #769

Open
2 tasks done
tuukkamustonen opened this issue Nov 13, 2023 · 3 comments · May be fixed by #801
Open
2 tasks done

Use system provided tzdata as a priority #769

tuukkamustonen opened this issue Nov 13, 2023 · 3 comments · May be fixed by #801

Comments

@tuukkamustonen
Copy link

tuukkamustonen commented Nov 13, 2023 •
edited

  • I am on the latest Pendulum version.

  • I have searched the issues of this repo and believe that this is not a duplicate.

  • OS version and name: Ubuntu 22.04

  • Pendulum version: 2.1.2

Issue

Pendulum uses tzdata package and imports the tzdata in

pendulum/src/pendulum/tz/__init__.py

Line 28 in 75a87a4

with cast(Path, resources.files("tzdata").joinpath("zones")).open() as f:

However, tzdata package's docs state:

It is generally recommended that any time zone libraries should attempt to use the system data before using the tzdata package

Should Pendulum rather look into system provided files first, and only resort to tzdata if the data is not found?

It's of course easier to just import from Python's tzdata package. Simiarly, if system provided data was used as a priority, then there would be need for a mechanism to override the system-provided data with Python's tzdata package (if OS provided tzdata is too old / incomplete).

So, might not be actionable.
@mgorny
Copy link

mgorny commented Dec 17, 2023

In fact, it's a violation of PEP 615 and as such, the package can't work on PEP 615-conformant Linux distributions that do not provide vendored tzdata, such as Gentoo.

@mgorny
Copy link

mgorny commented Dec 17, 2023

Also, I think the bug report is about 3.0.0.

@mgorny
Copy link

mgorny commented Dec 17, 2023

To be honest, I can't figure out what this is supposed to do. It seems to grab the list of timezones from tzdata, except that tzdata is not used anywhere else and system zoneinfo package is used instead which is going to prefer using system tzdata, so the list is effectively meaningless (in the best case it will match zoneinfo.available_timezones(), in the worst it will be mismatched to timezones actually supported). What's even more confusing is that pendulum.utils._zoneinfo seems to wrap available_timezones but it isn't used anywhere else.

mgorny added a commit to mgorny/pendulum that referenced this issue Feb 2, 2024
@mgorny
Fix pendulum.tz.timezones() to use system tzdata
0143f10 
Fix the `pendulum.tz.available_timezones()` to use
`available_timezones()` function instead of iterating over the files
in `tzdata` package.  This is more in line with PEP 615, as the system
timezone functions will operate on system-provided tzdata when
available, and use the `tzdata` package only if it's not available.
Therefore, the previous code would yield a potentially different list
of timezones than the system actually provides.

Furthermore, Gentoo provides a dummy `tzdata` package that does not
provide any data, since Python always uses system tzdata.  This change
is necessary to make pendulum work again on Gentoo.

Fixes sdispater#769
@mgorny mgorny linked a pull request Feb 2, 2024 that will close this issue
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix pendulum.tz.timezones() to use system tzdata mgorny/pendulum
2 participants
@tuukkamustonen @mgorny