Bug 471014 - pytz should use system-wide zoneinfo data
pytz should use system-wide zoneinfo data
Product: Fedora
Classification: Fedora
Component: pytz (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jef Spaleta
Fedora Extras Quality Assurance
: 471010 (view as bug list)
Depends On:
  Show dependency treegraph
Reported: 2008-11-11 07:43 EST by Petr Machata
Modified: 2015-05-04 21:34 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2009-01-07 23:18:45 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)
A couple more adjustments (22.11 KB, patch)
2008-11-11 19:45 EST, Petr Machata
no flags Details | Diff
Patch that I've used for testing. (21.97 KB, patch)
2008-11-13 08:33 EST, Petr Machata
no flags Details | Diff

  None (edit)
Description Petr Machata 2008-11-11 07:43:05 EST
Description of problem:
It's necessary to update pytz to more recent version, current 2006p is ancient by tzdata standards.  To avoid having to rebase every time Olson database changes, pytz should use tzdata files instead of its own.  It might be possible to even not ship pytz own data, and simply use tzdata always.

Version-Release number of selected component (if applicable):

How reproducible:

Steps to Reproduce:
$ rpm -ql tzdata | grep Salta
$ python -c 'import pytz; pytz.timezone("America/Argentina/Salta")'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib/python2.5/site-packages/pytz/__init__.py", line 71, in timezone
    raise KeyError, zone
KeyError: 'America/Argentina/Salta'
Actual results:
New zones are not available, the existing ones are presumably not current.

Expected results:
pytz is kept in sync with tzdata.

Additional info:
I've had a chat with pytz maintainer, and he told me that new versions of pytz already have a support in place that makes it possible to not use pytz own zoneinfo data, and supply system-wide instead.  Debian are reportedly doing just that.  It would be a very desirable feature to have in Fedora, too.
Comment 1 Jef Spaleta 2008-11-11 12:06:51 EST
I'm not seeing any documentation in the tarball on how make use of local zoneinfo data.

Is this a build time change?
Do I need patch the setup.py to fill the resource list in a different manner?

Or is this a runtime selection?

Comment 2 Petr Machata 2008-11-11 12:24:47 EST
I'm sorry, I should have checked first, and write more of what the maintainer told me.  I'm eyeing the tarball, and the zoneinfo files in new pytz are simply the same file that tzdata distributes.  Function open_resource then opens appropriate zoneinfo file and returns that.  Right now the function looks into the subdirectory of the module, but it can be simply changed to look at /usr/share/zoneinfo.  The maintainer has sent me actual code snippet:

def open_resource(name):
    """Open a resource from the zoneinfo subdir for reading.

    Uses the pkg_resources module if available.
    # Patched in Debian, use the system zoninfo from the tzdata package
    name_parts = name.lstrip('/').split('/')
    for part in name_parts:
        if part == os.path.pardir or os.path.sep in part:
            raise ValueError('Bad path segment: %r' % part)
    filename = os.path.join('/usr/share/zoneinfo', *name_parts)
    return open(filename, 'rb')

If we have this, I think we can simply stop shipping pytz zoneinfo files, and make tzdata a prerequisite of pytz (not exactly necessary, libc dependency already pulls tzdata in, but for correctness sake it makes sense).  Also it is then possible to change versioning scheme so that users don't get confused (2008i would imply obsolete data, which would not be true), but that's for you to decide I guess.
Comment 3 Jef Spaleta 2008-11-11 12:31:56 EST
I picked this up as an orphan... I think we'd have to discuss a change in the naming scheme with a larger group. Changing the naming scheme willynilly runs you into upgrade cornercases.  A discussion for the development tree after F10 release.

In the meantime...i'll replace on the hardcoded 'zoneinfo' strings with '/usr/share/zoneinfo'

This should really be reworked so you can set this in setup.py and have the setup process replace the strings. Or possibly hardcode the directory  just once so it can be replaced. 

Are you going to be able to test this out from koji? I dont want to do a build into a release tree without some testing.  I take it this is not a release blocker and this can wait to go into F10 updates.

Comment 4 Petr Machata 2008-11-11 13:13:42 EST
About versioning scheme, right, agreed.  I was thinking that epoch bump solves that, but on second thought it's probably not very wise anyway, for it would be less clear what upstream was used for given package.  It was an immediate idea, not anything that I've thought through.

Re 'zoneinfo' strings... I can't claim I understand python install process, but it looks to me like erasing /usr/lib/python2.5/site-packages/pytz/zoneinfo/ after setup.py is done would simply get the thing done.  Anyway, there's no point in distributing second copy of zoneinfo files.  Hardcoding /usr/share/zoneinfo seems acceptable to me, libc looks there too (if you change the TZ environment variable).

I can do the testing on my F-9 and on rawhide that I grab somewhere, I'll just have to figure out how to push python-matplotlib through the right code paths.  

It's not a blocker, it has been waiting since 2006p, and can wait a bit more for F-10 :)
Comment 5 Jef Spaleta 2008-11-11 14:08:42 EST
builds up

please test out of koji

-jef"someone else other than me uses matplotlib..amazing"spaleta
Comment 6 Petr Machata 2008-11-11 19:45:41 EST
Created attachment 323282 [details]
A couple more adjustments

The attached patch does two things:

* Unconditionally loads files from /usr/share/zoneinfo (previous version could find module-local files), or from the directory defined by the environment variable TZDIR, if it's set.

* It builds lists all_timezones and common_timezones dynamically.  The former by iterating files installed in zoneinfo directory, the latter from zone.tab.  The latter currently diverges from what upstream considers to be common timezone: they reject all links, including what I think are some valid ones.  I am in communication with upstream regarding that.

The filesystem traversal may be considered expensive, and my thinking was to do it lazily.  Building lazy list of all zones is reasonably easy, however I got stuck doing lazy all_timezones_set.  It looks like you simply have to reimplement all set methods in lazy set class, and there is no way to do it simply using e.g. __getattribute__ overload (len(x) et.al. don't go through that).  I decided to drop the idea unless it's deemed necessary.

That's just fixes that I found by simple "gimme America/Argentina/Salta" snippet, I'll do more proper testing tomorrow.
Comment 7 Petr Machata 2008-11-13 08:30:21 EST
I've taken a look at how does matplotlib use pytz.  From the /look/ of it, I would expect the changes in pytz to make no difference.  I also checked that the test snippet in matplotlib.dates gives the same results before and after I upgrade pytz.  I tried a couple demos that involve dates, and got no nasty surprises.  As far as I can tell, the update should be OK.
Comment 8 Petr Machata 2008-11-13 08:33:09 EST
Created attachment 323447 [details]
Patch that I've used for testing.

This patch should be used instead of your current patch.
Comment 9 Jef Spaleta 2008-11-13 12:13:41 EST
I'll push this to rawhide today.  I'll probably hold back on this as a test update for F9 and F10 until i get some feedback from a tinyerp-server person.

repoquery --whatrequires pytz

Comment 10 Jef Spaleta 2008-11-14 19:54:32 EST
hmm the build failed with the patch applied.....
Comment 11 Petr Machata 2008-11-18 06:38:29 EST
The patch needs to be applied with -p1.  I forgot to mention that.
Comment 12 Jef Spaleta 2008-11-18 18:29:41 EST
built against devel tree. I'd appreciate some testing of that build before F10 release.  If you are happy with how its functioning. We can que it up for F10 and F9 updates after F10 is released.

Comment 13 Petr Machata 2008-11-20 17:48:30 EST
I picked 2008i-3 from koji, and did the same testing as the last time (on F-9), and again found nothing suspicious.  Upgrade from 2006p worked with no problems.
Comment 14 Jef Spaleta 2008-12-11 19:19:15 EST
okay i'm picking this back up right now.  I'll see if I can get the patch applied for F-10 and F-9 updates-testing in the next 2 or 3 hours.

Comment 15 Fedora Update System 2008-12-11 19:29:12 EST
pytz-2008i-1.fc10 has been submitted as an update for Fedora 10.
Comment 16 Fedora Update System 2008-12-11 20:40:38 EST
pytz-2008i-2.fc10 has been submitted as an update for Fedora 10.
Comment 17 Fedora Update System 2008-12-13 10:02:45 EST
pytz-2008i-2.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update pytz'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2008-11228
Comment 18 Fedora Update System 2008-12-13 10:03:46 EST
pytz-2008i-2.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing-newkey update pytz'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-11232
Comment 19 Fedora Update System 2009-01-07 23:18:42 EST
pytz-2008i-2.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 20 Fedora Update System 2009-01-07 23:19:35 EST
pytz-2008i-2.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 21 Felix Schwarz 2009-04-18 07:50:59 EDT
*** Bug 471010 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.