Patches item #1667546, was opened at 2007-02-24 00:25 Message generated for change (Comment added) made by pboddie You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1667546&group_id=5470
Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Core (C code) Group: Python 2.4 Status: Open Resolution: None Priority: 5 Private: No Submitted By: Paul Boddie (pboddie) Assigned to: Georg Brandl (gbrandl) Summary: Time zone-capable variant of time.localtime Initial Comment: Patch related to #1493676: "time.strftime() %z error" This provides a localtime_tz function whose return value is the usual localtime time tuple with an additional field reflecting the underlying tm_gmtoff data. Various internal function signatures are modified to support the flow of time zone information, with the gettmarg most noticably changed (probably quite inelegantly - I don't do Python core development). This patch is against the Python 2.4.4 release sources. ---------------------------------------------------------------------- >Comment By: Paul Boddie (pboddie) Date: 2007-08-04 02:18 Message: Logged In: YES user_id=226443 Originator: YES I've updated the patch to work around redefinition of the tm_isdst field by mktime, which appeared to defeat the measures put in place to support mktimetz. File Added: time-2.diff ---------------------------------------------------------------------- Comment By: Georg Brandl (gbrandl) Date: 2007-07-12 11:51 Message: Logged In: YES user_id=849994 Originator: NO I still have a failing test with the latest patch: ====================================================================== FAIL: test_mktimetz (test.test_time.TimeTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/gbr/devel/python/Lib/test/test_time.py", line 272, in test_mktimetz self.assert_(time.mktimetz(lt) == time.mktimetz(gt)) AssertionError ====================================================================== FAIL: test_timegm (test.test_time.TimeTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/gbr/devel/python/Lib/test/test_time.py", line 253, in test_timegm self.assert_(0 <= dt < 1) AssertionError ---------------------------------------------------------------------- Ran 17 tests in 1.249s FAILED (failures=2) test test_time failed -- errors occurred; run in verbose mode for details 1 test failed: test_time ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-23 01:21 Message: Logged In: YES user_id=226443 Originator: YES I've enhanced your version of the patch to work with the following defines: #define HAVE_TZNAME 1 #undef HAVE_TM_ZONE #undef HAVE_STRUCT_TM_TM_ZONE #undef HAVE_ALTZONE It now uses struct_time in such an environment to provide the extra offset information used in the unixtime function rather than accessing tm_gmtoff. I suppose one could do that for all cases, in fact, since this is done independently of any mktime invocation. The above defines probably represent the next most "sane" of environments after those which have tm_gmtoff. If one starts to remove other things, other more established tests seem to fail, too. File Added: time-1-improved.diff ---------------------------------------------------------------------- Comment By: Georg Brandl (gbrandl) Date: 2007-03-21 11:25 Message: Logged In: YES user_id=849994 Originator: NO I'm attaching a patch with some corrections of mine. It looks very good now. However, your tests fail if HAVE_STRUCT_TM_TM_ZONE is not defined. Can that be repaired? If not, the tests must be skipped in this case. File Added: time-1.diff ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-20 01:20 Message: Logged In: YES user_id=226443 Originator: YES File Added: tm_gmtoff_zone_timegm_mktimetz_26.diff ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-20 01:19 Message: Logged In: YES user_id=226443 Originator: YES Yet another round. Fixed timegm, hopefully. Added mktimetz which uses time zone information to convert local and UMT times to UNIX times. Added tests and updated the docs. The usual caveats apply: I'm new to all this, so some things may need sanity checking. File Added: tm_gmtoff_zone_timegm_mktimetz.diff ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-14 23:43 Message: Logged In: YES user_id=226443 Originator: YES Looking at this a bit more, it seems like timegm (which is a pretty desirable function to have) really is as simple as the calendar.timegm function, as far as I can tell: it just throws time zone information away. So the timegm implementation in the patches so far is actually wrong (and broken in the way it attempts to use tm_gmtoff, anyway). However, it might be nice to have a function which actually interprets times properly in order to produce a UNIX time. In other words, something which returns zero for both time.localtime(0) and time.gmtime(0), along with other times which happen to refer to the epoch but in other time zones. I'll upload a fixed patch in the next day or so, hopefully. Sorry for the noise! ---------------------------------------------------------------------- Comment By: Georg Brandl (gbrandl) Date: 2007-03-12 15:13 Message: Logged In: YES user_id=849994 Originator: NO The patch looks good so far -- but I'd be comfortable with a few more tests, for example for strptime and strftime. Also, the documentation must be updated for every user-visible change (the addition of timegm(), the additional timetuple fields, the now-working (?) %z and %Z in str[fp]time and behavior changes). ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-11 22:02 Message: Logged In: YES user_id=226443 Originator: YES SourceForge "replay" added new attachment - now deleted. ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-11 21:15 Message: Logged In: YES user_id=226443 Originator: YES File Added: tm_gmtoff_zone_timegm_26.diff ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-11 18:00 Message: Logged In: YES user_id=226443 Originator: YES File Added: tm_gmtoff_zone_timegm_26.diff ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-11 18:00 Message: Logged In: YES user_id=226443 Originator: YES New patches against 2.4.4 and the trunk, providing tm_gmtoff, tm_zone, strptime enhancements, a timegm function, plus tests. Where a platform supports the timezone and altzone module attributes but not tm_gmtoff and tm_zone struct tm fields, the code attempts to populate the struct_time fields appropriately, setting None values only if no timezone information exists whatsoever. Limitations include the inability to parse/obtain timezone information beyond that provided by system calls, which is an existing limitation that affects the strptime implementation amongst other things. Again, testing for "deficient" platforms with limited struct tm definitions has been limited. File Added: tm_gmtoff_zone_timegm.diff ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-10 23:59 Message: Logged In: YES user_id=226443 Originator: YES After some thought, perhaps the population of timezone fields is more difficult than described below. Will need to look at other parts of the module code to see what the complications are. What a headache! ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-09 00:57 Message: Logged In: YES user_id=226443 Originator: YES File Added: tm_gmtoff_zone_26.diff ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-09 00:57 Message: Logged In: YES user_id=226443 Originator: YES Attached are two patches (for 2.4.4 and against the trunk). Apart from the addition of tm_zone, there's one big change: if no timezone fields/members exist on struct tm, the code attempts to read that data from the module's timezone and tzname attributes in order to populate tm_gmtoff and tm_zone; if that fails then both tm_gmtoff and tm_zone are set to None. The logic for all this is tested in test_time.py, but it really needs checking for suitability and testing on something like HP-UX. Details for the logic here: http://devrsrc1.external.hp.com/STKT/impacts/i117.html File Added: tm_gmtoff_zone.diff ---------------------------------------------------------------------- Comment By: Guido van Rossum (gvanrossum) Date: 2007-03-08 01:52 Message: Logged In: YES user_id=6380 Originator: NO No immediate time for review, but this sounds encouraging. Would you mind adding tm_zone (a string) as well? And how about working relative to the 2.6 trunk (since that's the earliest version where this new feature can be introduced)? ---------------------------------------------------------------------- Comment By: Paul Boddie (pboddie) Date: 2007-03-08 01:28 Message: Logged In: YES user_id=226443 Originator: YES One learns new things about time and stat tuples every day! Made a much cleaner patch which provides an extra named attribute (tm_gmtoff) whilst preserving the 9-tuple layout. Where no time zone support exists, tm_gmtoff is None; otherwise it's the GMT/UTC offset. I had weird test issues with range(7) giving "TypeError: an integer is required" in the code employed (deep down) in test_strptime at some point during development, probably due to memory issues, so it might be worth checking that I've dealt properly with such things. File Added: tm_gmtoff.diff ---------------------------------------------------------------------- Comment By: Guido van Rossum (gvanrossum) Date: 2007-03-02 16:52 Message: Logged In: YES user_id=6380 Originator: NO Without even looking at the patch, IMO it would be much better to add tm_gmtoff and tm_zone (and any other fields) to the record returned by localtime(), but in such a way that when accessed as a tuple it still has only 9 fields. There is infrastructure for doing so somewhere for the stat structure that I'm sure could be borrowed or generalized (if it isn't already general). That's much better than adding a new function. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1667546&group_id=5470 _______________________________________________ Patches mailing list Patches@python.org http://mail.python.org/mailman/listinfo/patches