classification
Title: calendar.timegm() belongs in time module, next to time.gmtime()
Type: enhancement Stage: commit review
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: belopolsky Nosy List: aht, amaury.forgeotdarc, belopolsky, djc, hodgestar, jcea, mark.dickinson, pr0gg3d, zooko
Priority: low Keywords: needs review, patch

Created on 2009-06-13 16:48 by zooko, last changed 2012-11-06 00:09 by jcea. This issue is now closed.

Files
File name Uploaded Description Edit
add-timegm-to-time.diff hodgestar, 2009-10-09 14:40 Adds calendar.timegm to the time module.
timemodule-gmtime-r265.diff pr0gg3d, 2010-04-26 08:53 Patch for r265
timemodule-gmtime-r27b1.diff pr0gg3d, 2010-04-26 08:53 patch for r27b1
timemodule-gmtime-r312.diff pr0gg3d, 2010-04-26 08:53 patch for r312
timemodule-gmtime-3-trunk.diff pr0gg3d, 2010-04-26 08:54 patch for trunk
issue6280-calendar.diff belopolsky, 2010-06-06 01:15
issue6280-calendar-2.diff belopolsky, 2010-06-14 19:06
Messages (20)
msg89334 - (view) Author: Zooko O'Whielacronx (zooko) Date: 2009-06-13 16:48
I've been struggling to write a function that takes UTC timestamps in
ISO-8601 strings and returns UTC timestamps in unix-seconds-since-epoch.
 The first implementation used time.mktime() minus time.timezone
(http://allmydata.org/trac/tahoe/browser/src/allmydata/util/time_format.py?rev=2424
), but that doesn't work in London if the argument date is in a certain
period during the 1970's.  Then there was force-the-tz-to-UTC
(http://allmydata.org/trac/tahoe/changeset/3911/src/allmydata/util/time_format.py
), but that doesn't work on Windows.  Then there was
force-the-tz-to-UTC-except-on-Windows
(http://allmydata.org/trac/tahoe/changeset/3913/src/allmydata/util/time_format.py
), but that still doesn't work on Windows.  Then there was this horrible
hack of converting from string-iso-8601 to localseconds with
time.mktime(), then converting from the resulting localseconds to an
iso-8601 string, then converting the resulting string back to seconds
with time.mktime(), then subtracting the two seconds values to find out
the real offset between UTC-seconds and local-seconds for the current tz
and for the time indicated by the argument
(http://allmydata.org/trac/tahoe/changeset/3914/src/allmydata/util/time_format.py
).  This actually works everywhere, but it is horrible.  Finally,
yesterday, someone pointed out to me that the inverse of time.gmtime()
is located in the "calendar" module and is named calendar.timegm().  Now
the implementation of our function is simple
(http://allmydata.org/trac/tahoe/changeset/3915/src/allmydata/util/time_format.py
)  I suggest that timegm() be moved to the time module next to its
sibling gmtime().
msg89335 - (view) Author: Zooko O'Whielacronx (zooko) Date: 2009-06-13 16:52
Here is the ticket that tracked this issue within the Tahoe-LAFS
project: http://allmydata.org/trac/tahoe/ticket/733
msg89379 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2009-06-14 23:31
Do you want to write a patch?
(Note: the time module is written in C)
msg91350 - (view) Author: Francesco Del Degan (pr0gg3d) Date: 2009-08-06 07:47
Hi, i started to produce a patch for timemodule.c.

Working into it, i found that we have almost 3 way to do that:

1. Use timegm(3) function where HAVE_TIMEGM is defined (i have a working patch for it)

2. Implement a more portable timegm function with tzset and mktime where HAVE_MKTIME and 
HAVE_WORKING_TZSET is defined (i have a working patch for it)

3. Doing some calculation taking calendar.timegm as example.


What do you think about it?

Thanks,
Francesco "pr0gg3d" Del Degan
msg93797 - (view) Author: Simon Cross (hodgestar) Date: 2009-10-09 14:40
The attached patch adds a simple implementation of time.timegm that
calls calendar.timegm. It includes a short test to show that 
time.timegm(time.gmtime(ts)) == ts for various timestamps.

I implemented a pure C version by pulling in the various functions
needed from datetime, but it was a bit messy and a lot more invasive.

Documentation updates are still needed -- I will do those if there is
support for the patch.
msg99633 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-02-20 21:22
Francesco,

You have my +1 for implementing both 1 and 2 below.
"""
1. Use timegm(3) function where HAVE_TIMEGM is defined (i have a working patch for it)

2. Implement a more portable timegm function with tzset and mktime where HAVE_MKTIME and 
HAVE_WORKING_TZSET is defined (i have a working patch for it)
"""

I don't think "3. Doing some calculation taking calendar.timegm as example" is a good idea.  IMHO, its is more important that timegm is a proper inverse of gmtime on any given platform than to have the same values produced by timegm across platforms.  If system timegm (or mktime) thinks that 1900 is a leap year, for example, python should not attempt to correct that.  Maybe doing "some calculation" on systems that don't have mktime is a reasonable last resort, but I am not sure it is worth the effort.
msg99905 - (view) Author: Francesco Del Degan (pr0gg3d) Date: 2010-02-23 09:23
I attached a patch that implements timegm according to two constraints:

1. If HAVE_TIMEGM is defined, use it

or

2. If HAVE_MKTIME and HAVE_WORKING_TZSET use a portable way, using mktime (taken from timegm(3) man)

Attached patches are for:

r264, r273a1, r311, trunk

What i'm missing just now are the tests (test_time.py) and docs update, if you like the patch, i can continue on that.
msg99938 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-02-23 17:28
The patch (I reviewed timemodule-gmtime-trunk.diff) looks mostly correct.  One problem that I see is that it will likely produce compiler warnings on the systems without timegm and mktime.  The warnings will be due to unused static function time_timegm and use of uninitialized variable tt.  I suggest that you wrap time_timegm in appropriate #ifdefs.

I trust that you tested that it works, but

#ifdef HAVE_TIMEGM || (defined(HAVE_MKTIME) && defined(HAVE_WORKING_TZSET))

looks like a non-standard construct to me.  I would do

#if defined(HAVE_TIMEGM) || (defined(HAVE_MKTIME) && defined(HAVE_WORKING_TZSET))

instead.

Finally, tests and documentation are needed.
msg100012 - (view) Author: Francesco Del Degan (pr0gg3d) Date: 2010-02-24 09:03
Those are the new updated patches with ifdef wrapped timegm function, docs, and a conversion test.
msg100052 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-02-24 17:21
Looks good. 

A documentation typo:

 gmtime() -- convert seconds since Epoch to UTC tuple\n\
+timegm() - Convert a UTC tuple to seconds since the Epoch.\n\

Note the use of -- in the existing items.

I understand that the choice of float for return value is dictated by consistency with mktime, but I question the wisdom of returning float instead of int from mktime.
msg104201 - (view) Author: Francesco Del Degan (pr0gg3d) Date: 2010-04-26 08:54
Fixed typos, new patches added
msg104262 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-04-26 17:43
It is too late to get new features in 2.x.

Francesco,

Please double-check timemodule-gmtime-r312.diff, it does not seem to reflect your reported changes. ('-' vs '--' typo is still there)

There is no need to submit multiple patches.  A patch for py3k branch should be enough.
msg104266 - (view) Author: Alexander Belopolsky (Alexander.Belopolsky) Date: 2010-04-26 18:00
Some more comments:

- Documentation needs a "versionadded" entry.

- A fate of calendar.timegm() needs to be decided.  If the decision is to deprecate it, deprecation warning need to be added to calendar module and docs updated accordingly.   Given that as implemented, time.timegm() is not a drop-in replacement for calendar.timegm(), deprecation may not be appropriate, it which case the difference between the two functions should probably be explained in the docs.
msg104269 - (view) Author: Francesco Del Degan (pr0gg3d) Date: 2010-04-26 19:56
I thinks that isn't a so easy decision to take.

And there are some other issues, imho:

1. timegm function is not specified by any standard (POSIX). The portable way (setting TZ, calling mktime, restore TZ) is a pure hack (could not work in future multithreaded environments).
2. if we want to strictly follow the time.h definition from POSIX standards, the timegm function should be kept away from time module (as now).
3. timegm seems to have some issues on mingw32. 
4. Solaris doesn't come with the timegm function out-of-the-box.

We could give up at this point.
msg107169 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-06 01:15
With recent enhancements to datetime module, timegm has become a 1-liner:

EPOCH = 1970
_EPOCH_DATETIME = datetime.datetime(EPOCH, 1, 1)
_SECOND = datetime.timedelta(seconds=1)

def timegm(tuple):
    """Unrelated but handy function to calculate Unix timestamp from GMT."""
    return (datetime.datetime(*tuple[:6]) - _EPOCH_DATETIME) // _SECOND

I suggest committing modernized implementation to serve as a reference and encourage people to use datetime module and datetime objects instead of time module and time tuples.
msg107631 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-12 06:37
Mark, reassigning this to you for commit review.
msg107802 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2010-06-14 17:18
The issue6280-calendar.diff patch looks good to me.
msg107804 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-14 17:43
Committed issue6280-calendar.diff in r81988.  I believe tests should be merged in 2.7.  Any objections?

As for the original RFE, I think it should be rejected.  I believe users should be encouraged to use datetime objects instead of timetuples and converting a UTC datetime to any kind of "since epoch" timestamp is very simple using datetime module arithmetics.
msg107808 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-06-14 19:06
I reverted r81988 in r81989. Some code may rely on timegm() accepting float in tm_sec.  See http2time in Lib/http/cookiejar.py.

It is very easy to modify implementation to accept float seconds, but old implementation accidentally work for float hours and minutes as well.
msg122917 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-11-30 17:05
I am going to close this as "rejected" unless someone objects.  The benefit is too small to make users suffer through the deprecation process.
History
Date User Action Args
2012-11-06 00:09:37jceasetnosy: + jcea
2010-12-24 00:10:12belopolskysetstatus: pending -> closed
nosy: zooko, amaury.forgeotdarc, mark.dickinson, belopolsky, djc, hodgestar, pr0gg3d, aht
2010-11-30 17:05:40belopolskysetstatus: open -> pending
resolution: rejected
messages: + msg122917
2010-06-14 19:06:08belopolskysetfiles: + issue6280-calendar-2.diff

messages: + msg107808
versions: - Python 2.7
2010-06-14 17:43:49belopolskysetmessages: + msg107804
versions: + Python 2.7
2010-06-14 17:18:07mark.dickinsonsetassignee: mark.dickinson -> belopolsky
messages: + msg107802
2010-06-12 06:37:12belopolskysetassignee: belopolsky -> mark.dickinson
messages: + msg107631
2010-06-06 01:15:09belopolskysetfiles: + issue6280-calendar.diff
priority: normal -> low

assignee: belopolsky

nosy: + belopolsky, mark.dickinson, - Alexander.Belopolsky
messages: + msg107169
stage: needs patch -> commit review
2010-04-26 19:56:16pr0gg3dsetmessages: + msg104269
2010-04-26 18:00:09Alexander.Belopolskysetmessages: + msg104266
2010-04-26 17:43:42Alexander.Belopolskysetmessages: + msg104262
versions: - Python 2.7
2010-04-26 08:54:57pr0gg3dsetmessages: + msg104201
2010-04-26 08:54:42pr0gg3dsetfiles: - timemodule-gmtime-2-r264.diff
2010-04-26 08:54:37pr0gg3dsetfiles: - timemodule-gmtime-2-r311.diff
2010-04-26 08:54:34pr0gg3dsetfiles: - timemodule-gmtime-2-r27a3.diff
2010-04-26 08:54:29pr0gg3dsetfiles: - timemodule-gmtime-2-trunk.diff
2010-04-26 08:54:11pr0gg3dsetfiles: + timemodule-gmtime-3-trunk.diff
2010-04-26 08:53:53pr0gg3dsetfiles: + timemodule-gmtime-r312.diff
2010-04-26 08:53:38pr0gg3dsetfiles: + timemodule-gmtime-r27b1.diff
2010-04-26 08:53:18pr0gg3dsetfiles: + timemodule-gmtime-r265.diff
2010-02-24 17:21:10Alexander.Belopolskysetmessages: + msg100052
2010-02-24 15:25:08brian.curtinsetpriority: normal
keywords: + needs review
type: enhancement
versions: + Python 2.7, Python 3.2, - Python 2.6, Python 2.5
2010-02-24 09:08:13pr0gg3dsetfiles: + timemodule-gmtime-2-r264.diff
2010-02-24 09:07:52pr0gg3dsetfiles: + timemodule-gmtime-2-r311.diff
2010-02-24 09:06:24pr0gg3dsetfiles: + timemodule-gmtime-2-r27a3.diff
2010-02-24 09:03:09pr0gg3dsetfiles: + timemodule-gmtime-2-trunk.diff

messages: + msg100012
2010-02-24 08:57:27pr0gg3dsetfiles: - timemodule-gmtime-trunk.diff
2010-02-24 08:57:18pr0gg3dsetfiles: - timemodule-gmtime-r311.diff
2010-02-24 08:57:15pr0gg3dsetfiles: - timemodule-gmtime-r27a3.diff
2010-02-24 08:57:12pr0gg3dsetfiles: - timemodule-gmtime-r264.diff
2010-02-23 17:28:45Alexander.Belopolskysetmessages: + msg99938
2010-02-23 09:24:28pr0gg3dsetfiles: + timemodule-gmtime-trunk.diff
2010-02-23 09:24:14pr0gg3dsetfiles: + timemodule-gmtime-r311.diff
2010-02-23 09:24:04pr0gg3dsetfiles: + timemodule-gmtime-r27a3.diff
2010-02-23 09:23:34pr0gg3dsetfiles: + timemodule-gmtime-r264.diff

messages: + msg99905
2010-02-20 21:22:19Alexander.Belopolskysetnosy: + Alexander.Belopolsky
messages: + msg99633
2010-02-16 12:35:09djcsetnosy: + djc
2009-10-09 14:40:23hodgestarsetfiles: + add-timegm-to-time.diff
keywords: + patch
messages: + msg93797
2009-10-08 11:18:17ahtsetnosy: + aht

versions: + Python 2.6
2009-08-06 08:22:37hodgestarsetnosy: + hodgestar
2009-08-06 07:47:23pr0gg3dsetnosy: + pr0gg3d
messages: + msg91350
2009-06-14 23:31:24amaury.forgeotdarcsetnosy: + amaury.forgeotdarc

messages: + msg89379
stage: needs patch
2009-06-13 16:52:51zookosetmessages: + msg89335
2009-06-13 16:48:01zookocreate