-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
Implement PEP 495 (Local Time Disambiguation) #68961
Comments
Adds a boolean member to the instances of See Datetime-SIG "Local time disambiguation proposal" discussion 1 for more details. |
Something went wrong in my hg clone and Rietveld did not like my patch. I moved my development branch to github: https://github.com/abalkin/cpython/tree/ltdf Please feel free to leave your comments there. |
Changing the title to reference PEP-495. |
Submitting the latest Github snapshot as a patch against master for review. See bpo-24773-s3-2.diff. |
Naoki, thanks for the review. I addressed your comments in https://github.com/abalkin/cpython/commit/a61a25d2dd04336361b2ea676c80afdb2639f579 Attached patch, bpo-24773-s3-3.diff, incorporates your comments and a few other fixes. See github for the full history. |
The new patch passes test_datetime on Windows. This was the last stumbling block for me to commit this patch. Unless anyone would ask for more time to review, I plan to commit later this week. |
I am posting the final version of the patch complete with the NEWS entry. Compared with the previous patch, bpo-24773-final.diff contains a fix for a reference leak. I would like to use this opportunity to thank Zachary Ware for providing a Windows VM with a complete Python development setup, as well as everyone who participated in PEP-495 discussions and code reviews. |
New changeset 7c0917670ab8 by Alexander Belopolsky in branch 'default': |
This upset the buildbots: http://buildbot.python.org/all/builders/PPC64%20Fedora%203.x/builds/1259/steps/test/logs/stdio Traceback (most recent call last):
File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/test/datetimetester.py", line 4784, in test_all
self.assertTrue(result.wasSuccessful(), name + ' ' + suffix)
AssertionError: False is not true : Africa/El_Aaiun system_transitions |
FYI, other failures that may be different problems: http://buildbot.python.org/all/builders/AMD64%20Snow%20Leop%203.x/builds/4930/steps/test/logs/stdio http://buildbot.python.org/all/builders/AMD64%20OpenIndiana%203.x/builds/11008/steps/test/logs/stdio Traceback (most recent call last):
File "/export/home/buildbot/64bits/3.x.cea-indiana-amd64/build/Lib/test/datetimetester.py", line 4680, in setUp
self.tz = ZoneInfo.fromname(self.zonename)
File "/export/home/buildbot/64bits/3.x.cea-indiana-amd64/build/Lib/test/datetimetester.py", line 4514, in fromname
with open(path, 'rb') as f:
FileNotFoundError: [Errno 2] No such file or directory: '/usr/share/zoneinfo/Iran' http://buildbot.python.org/all/builders/x86%20Ubuntu%20Shared%203.x/builds/13492/steps/test/logs/stdio Traceback (most recent call last):
File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/support/__init__.py", line 1563, in inner
return func(*args, **kwds)
File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/datetimetester.py", line 1931, in test_timestamp_naive
self.assertEqual(self.theclass.fromtimestamp(s), t)
OverflowError: timestamp out of range for platform time_t |
Also failing on all freebsd buildbots, all tests failing with: FileNotFoundError: [Errno 2] No such file or directory: '/usr/share/zoneinfo/Iran' |
Does anyone know whether zoneinfo is installed on FreeBSD buildbot? If it is, at which path?
|
New changeset e72aab080165 by Alexander Belopolsky in branch 'default': |
This may be an indication of misconfigured zoneinfo on the buildbot. What system_transitions test does is compare the results obtained from the system timezone computations to the same computations using an explicitly loaded timezone. This may fail if /usr/share/zoneinfo exists, but the system is using a different database. Is there a way to troubleshoot the buildbot short of trying a series of hg commits? |
I can help providing information on the koobs-* freebsd buildbots (I run them). In a default installation, the timezone entries are available in /usr/share/zoneinfo (see attachment for contents) Iran is not in the root directory, 'Tehran' is in Asia/ subdirectory tzsetup man page: https://www.freebsd.org/cgi/man.cgi?query=tzsetup&sektion=8 The misc/zoneinfo port/package [1] can be installed which overwrites entries in the above location. The files this port/package installs are in the pkg-plist file: https://svnweb.freebsd.org/ports/head/misc/zoneinfo/pkg-plist?view=log There doesn't appear to be an 'Iran' entry in the root of this port/package either, so my guess is its a distribution specific location [1] https://svnweb.freebsd.org/ports/head/misc/zoneinfo/ Beyond the above, tests should not fail (but skip) if the resources it requires are not available, so the change in e72aab080165 is welcome |
See Also: non standard (standard?) timezones. I don't know to what extent these links are considered standard, but I'll talk to Julian about whether we can get these links installed in FreeBSD base (and the zoneinfo port/package). having said that, even if they can be/are added, the tests should still not expect them (blindly) to be available. |
FWIW I can produce two of these failures locally: the time_t OverflowError, and the test_all() system_transitions one. Let me know if you want any more info. In my case the message for the test_all() failure is “posix/Africa/Casablanca system_transitions”. I am on 64-bit Arch Linux, but Python is currently compiled for the 32-bit ABI. $ pacman -Qo /usr/share/zoneinfo/posix/Africa/Casablanca
/usr/share/zoneinfo/posix/Africa/Casablanca is owned by tzdata 2013h-1 |
The issue here is that for a large date, dt.timestamp() returns a float large enough to cause overflow in fromtimestamp.
Can you figure out what date causes this (0002-01-01 or 9998-12-12 or both)? What value is returned by dt.timestamp()? Does pure python implementation behave the same as C? (SEt sys.modules['_datetime'] to None before importing datetime to get a pure python implementation.)
Do you get this failure only on a 32-bit interpreter? Please add class CasablancaTest(ZoneInfoTest):
zonename = 'Africa/Casablanca' to datetimetester.py and run python -mtest -v test_datetime. |
Looking at the stable buildbots. http://buildbot.python.org/all/waterfall?category=3.x.stable AMD64 OpenIndiana 3.x - unrelated failures |
New changeset ae19ea7c36e6 by Alexander Belopolsky in branch 'default': |
New changeset dca143512f6e by Alexander Belopolsky in branch 'default': |
Without blocking the C implementation _datetime, I get both extremes causing OverflowError: >>> import datetime, time, os
>>> os.environ["TZ"] = "EST+05EDT,M3.2.0,M11.1.0"
>>> time.tzset()
>>> t = datetime.datetime(2,1,1)
>>> s = t.timestamp()
>>> s
-122233584944.0
>>> datetime.datetime.fromtimestamp(s)
OverflowError: timestamp out of range for platform time_t
>>> t = datetime.datetime(9998,12,12)
>>> s = t.timestamp()
>>> s
760175195728.0
>>> datetime.datetime.fromtimestamp(s)
OverflowError: timestamp out of range for platform time_t When I repeated the test with sys["_datetime"] = None, both t.timestamp() calls raised OverflowError, which I assume is expected. |
I _think_ the system_transitions failure only happens for 32 bit (have to test more to be sure). (My 32-bit environment is lacking many libraries compared to main 64-bit environment, but still uses the same filesystem etc.) First system_transitions failure with today’s new code: FAIL: test_system_transitions (test.datetimetester.ZoneInfoTest[posix/Africa/Casablanca]) Traceback (most recent call last):
File "/media/disk/home/proj/python/cpython/Lib/test/datetimetester.py", line 4760, in test_system_transitions
self.assertEquivDatetimes(sdt, tzdt)
File "/media/disk/home/proj/python/cpython/Lib/test/datetimetester.py", line 4687, in assertEquivDatetimes
(b.replace(tzinfo=None), b.fold, id(b.tzinfo)))
AssertionError: Tuples differ: (datetime.datetime(2037, 10, 11, 3, 0), 0, 140273296) != (datetime.datetime(2037, 10, 11, 2, 0, fold=1), 1, 140273296) First differing element 0:
+ (datetime.datetime(2037, 10, 11, 2, 0, fold=1), 1, 140273296) Your CasablancaTest gave the same failure: Traceback (most recent call last):
File "/media/disk/home/proj/python/cpython/Lib/test/datetimetester.py", line 4760, in test_system_transitions
self.assertEquivDatetimes(sdt, tzdt)
File "/media/disk/home/proj/python/cpython/Lib/test/datetimetester.py", line 4687, in assertEquivDatetimes
(b.replace(tzinfo=None), b.fold, id(b.tzinfo)))
AssertionError: Tuples differ: (datetime.datetime(2037, 10, 11, 3, 0), 0, 140273296) != (datetime.datetime(2037, 10, 11, 2, 0, fold=1), 1, 140273296) First differing element 0:
+ (datetime.datetime(2037, 10, 11, 2, 0, fold=1), 1, 140273296) |
That's very helpful. It looks like on a Mac 32-bit build has 64-bit time_t. I'll build a 32-bit Python on Linux tomorrow and try to get to the bottom of this. |
koobs-freebsd-* 3.x builds are now passing, but I'd like to make an additional (trivial) proposal, and that is: If the tests that were previously failling, aren't *specifically* testing for 'backward compatible' timezone definitions, that the tests that currently use $ROOT/Iran timezone file, instead use a timezone file that is *not* contained within the 'backwards' (backwards compatible) timezone file in the zoneinfo distribution, which may or may not be installed in various environments. For example, the tests could instead use Asia/Tehran |
New changeset 8cc06070e98b by Alexander Belopolsky in branch 'default': |
The remaining failures all seems to be related to the Morocco rules: both Africa/El_Aaiun 1 and Africa/Casablanca 2 use those rules. The affected date is October 4, 2037, for which Morocco has a special rule. 3 It looks like the problem is with the system date/time library. The 2037 transition is specified in the tzfile as POSIX time 2138234400 and system date utility produced a value different from that of IANA's date 4: $ TZ=/usr/share/zoneinfo/Africa/El_Aaiun date -d @2138234400
Sun Oct 4 03:00:00 WEST 2037
$ TZ=/usr/share/zoneinfo/Africa/El_Aaiun ./date -r 2138234400
Sun Oct 4 02:00:00 WET 2037 |
Could our Morocco issue be similar to Fiji issue? |
Just confirming that my Casablanca failure is restricted to the 32-bit build, though I think you already figured this out. |
New changeset 95df96aa2f5a by Alexander Belopolsky in branch 'default': |
It looks like PPC64 Fedora 3.x builder 1 also has a problem with a transition in 2037. |
It looks like th Morocco issue has been reported to CentOS recently but they kicked it upstream. |
Also reported for Ubuntu: https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/1587128 |
It looks like Ruby folks encountered the Morocco issue 1 before us. They closed the issue on their bug tracker blaming glibc. This tells us, I guess, that we should skip this transition on the affected systems. Unfortunately, it is not just 32-bit time_t systems - I was able to reproduce the problem on 64-bit Fedora 22. If no one suggests a better way, I will just skip year 2037 transitions unconditionally. |
It looks like the tzdata folks have agreed 1 that there is a problem with the Morocco rules in the Africa file and will likely fix it in the next release. This is an interesting situation where a bug in tzcode masks a bug in tzdata while glibc implements the documented behavior faithfully but as a result suffers from the data bug. I will wait for the conclusion of the discussion on the TZ list because there is a chance that we should fix the ZoneInfo logic to match glibc. |
The links like Iran are non-standard. they are specified in the "backward" file in the IANA tzdata distribution which has the following preamble: # This file provides links between current names for time zones I'll change "Iran" to "Asia/Tehran" in the test. $ grep Iran Work/tz/backward
Link Asia/Tehran Iran |
New changeset eed3a5b9239f by Alexander Belopolsky in branch 'default': |
Thank you Alexander, consider me satisfied :) |
Any news on the remaining failures for year 2037? What about the buildbots that time out? Can the size of the tests be reduced, or perhaps should the buildbots be updated to extend the timeout? |
Yes, the problem was tracked to a bug 1 in zic. If the buildbots get regular updates, the problem will go away with the next tzdata release. Meanwhile, I'll try to figure out a way to suppress the error. |
Yes, the long test walks the zoneinfo tree and runs on every tzfile including the aliases. I am going to change that to parsing the zone.tab file for the list of zone names. This should shorten the time at least 2x. |
New changeset 05120447f2c6 by Alexander Belopolsky in branch 'default': |
Both parts of your commit seem to have helped. However I found two failures still happening, and one new failure:
Traceback (most recent call last):
File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/test/datetimetester.py", line 4781, in test_system_transitions
self.assertEquivDatetimes(sdt, tzdt)
File "/home/shager/cpython-buildarea/3.x.edelsohn-fedora-ppc64/build/Lib/test/datetimetester.py", line 4706, in assertEquivDatetimes
(b.replace(tzinfo=None), b.fold, id(b.tzinfo)))
AssertionError: Tuples differ: (datetime.datetime(2037, 10, 10, 3, 0), 0, 271733936) != (datetime.datetime(2037, 10, 10, 2, 0, fold=1), 1, 271733936) First differing element 0:
+ (datetime.datetime(2037, 10, 10, 2, 0, fold=1), 1, 271733936)
Traceback (most recent call last):
File "/buildbot/buildarea/3.x.ware-gentoo-x86/build/Lib/test/datetimetester.py", line 4694, in setUp
self.tz = ZoneInfo.fromname(self.zonename)
File "/buildbot/buildarea/3.x.ware-gentoo-x86/build/Lib/test/datetimetester.py", line 4527, in fromname
return cls.fromfile(f)
File "/buildbot/buildarea/3.x.ware-gentoo-x86/build/Lib/test/datetimetester.py", line 4519, in fromfile
self = cls(ut, ti)
File "/buildbot/buildarea/3.x.ware-gentoo-x86/build/Lib/test/datetimetester.py", line 4472, in __init__
self.lt = self.invert(ut, ti)
File "/buildbot/buildarea/3.x.ware-gentoo-x86/build/Lib/test/datetimetester.py", line 4482, in invert
lt[0][i] += ti[i-1][0] // SEC
OverflowError: Python int too large to convert to C long
Traceback (most recent call last):
File "/buildbot/buildarea/3.x.ware-gentoo-x86/build/Lib/test/datetimetester.py", line 4781, in test_system_transitions
self.assertEquivDatetimes(sdt, tzdt)
File "/buildbot/buildarea/3.x.ware-gentoo-x86/build/Lib/test/datetimetester.py", line 4706, in assertEquivDatetimes
(b.replace(tzinfo=None), b.fold, id(b.tzinfo)))
AssertionError: Tuples differ: (datetime.datetime(2037, 10, 9, 0, 0), 0, 137328448) != (datetime.datetime(2037, 10, 8, 23, 0, fold=1), 1, 137328448) First differing element 0:
+ (datetime.datetime(2037, 10, 8, 23, 0, fold=1), 1, 137328448) |
New changeset 71a7db7ceabc by Alexander Belopolsky in branch 'default': |
New changeset 617104a6b759 by Alexander Belopolsky in branch 'default': |
As far as I can tell, the buildbots are happy now. Closing. Please open new issues if missed anything. |
New changeset 5c02f689c62c by Victor Stinner in branch '3.6': |
I don't like the Perhaps Now about the values. Having Compare these two lines of code and tell me which is more apparant at first glance:
|
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: