calendar.timegm() belongs in time module, next to time.gmtime()

[182980c4-e4f4-4e83-8e3e-662d412bbf71]

BPO	6280
Nosy	@jcea, @amauryfa, @mdickinson, @abalkin, @djc, @pganssle
Files	add-timegm-to-time.diff: Adds calendar.timegm to the time module. timemodule-gmtime-r265.diff: Patch for r265 timemodule-gmtime-r27b1.diff: patch for r27b1 timemodule-gmtime-r312.diff: patch for r312 timemodule-gmtime-3-trunk.diff: patch for trunk issue6280-calendar.diff issue6280-calendar-2.diff

^{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: ```python assignee = 'https://github.com/abalkin' closed_at = None created_at = labels = ['3.7', 'type-feature', 'library'] title = 'calendar.timegm() belongs in time module, next to time.gmtime()' updated_at = user = 'https://bugs.python.org/zooko' ``` bugs.python.org fields: ```python activity = actor = 'p-ganssle' assignee = 'belopolsky' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'zooko' dependencies = [] files = ['15092', '17085', '17086', '17087', '17088', '17566', '17669'] hgrepos = [] issue_num = 6280 keywords = [] message_count = 22.0 messages = ['89334', '89335', '89379', '91350', '93797', '99633', '99905', '99938', '100012', '100052', '104201', '104262', '104266', '104269', '107169', '107631', '107802', '107804', '107808', '122917', '277965', '319965'] nosy_count = 11.0 nosy_names = ['jcea', 'zooko', 'amaury.forgeotdarc', 'mark.dickinson', 'belopolsky', 'djc', 'hodgestar', 'pr0gg3d', 'aht', 'p-ganssle', 'oric'] pr_nums = [] priority = 'normal' resolution = None stage = 'needs patch' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue6280' versions = ['Python 3.7'] ```

[182980c4-e4f4-4e83-8e3e-662d412bbf71]

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().

[182980c4-e4f4-4e83-8e3e-662d412bbf71]

Here is the ticket that tracked this issue within the Tahoe-LAFS project: http://allmydata.org/trac/tahoe/ticket/733

[amauryfa]

Do you want to write a patch? (Note: the time module is written in C)

[d929153d-db26-456f-8a25-0c309f220ac1]

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

[2eadb908-4a42-43e8-b4f5-d43cea0e9806]

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.

[5579dc13-9f48-42d1-bb17-9c003ef6fa70]

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.

[d929153d-db26-456f-8a25-0c309f220ac1]

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.

[5579dc13-9f48-42d1-bb17-9c003ef6fa70]

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.

[d929153d-db26-456f-8a25-0c309f220ac1]

Those are the new updated patches with ifdef wrapped timegm function, docs, and a conversion test.

[5579dc13-9f48-42d1-bb17-9c003ef6fa70]

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.

[d929153d-db26-456f-8a25-0c309f220ac1]

Fixed typos, new patches added

[5579dc13-9f48-42d1-bb17-9c003ef6fa70]

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.

[5579dc13-9f48-42d1-bb17-9c003ef6fa70]

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.

[d929153d-db26-456f-8a25-0c309f220ac1]

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.

[abalkin]

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.

[abalkin]

Mark, reassigning this to you for commit review.

[mdickinson]

The bpo-6280-calendar.diff patch looks good to me.

[abalkin]

Committed bpo-6280-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.

[abalkin]

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.

[abalkin]

I am going to close this as "rejected" unless someone objects. The benefit is too small to make users suffer through the deprecation process.

[abalkin]

I would like to revisit this for 3.7.

[b4331dc2-4085-46ea-9502-ca45e8ad6782]

Please don't name it timegm which is such a stupid name (why not mgemit!).

I knows it comes from history but history had timelocal which has been replaced by mktime (and python has mktime and not timelocal) so please, call it mkgmtime

time.mkgmtime()

I think it would make things more clear.

[kulikjak]

4. Solaris doesn't come with the timegm function out-of-the-box.

I just wanted to note that both Solaris and Illumos should now have usable gmtime ;). https://docs.oracle.com/cd/E88353_01/html/E37843/gmtime-3c.html https://illumos.org/man/3C/gmtime