classification
Title: Add timezone support to datetime C API
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: belopolsky Nosy List: belopolsky, ncoghlan, p-ganssle, tim.peters, vstinner
Priority: normal Keywords: patch

Created on 2010-11-10 18:49 by belopolsky, last changed 2018-07-05 16:52 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 5032 merged p-ganssle, 2017-12-28 20:20
PR 5317 merged p-ganssle, 2018-01-25 13:35
PR 5318 merged p-ganssle, 2018-01-25 14:09
PR 5814 merged p-ganssle, 2018-02-22 16:33
PR 5819 merged miss-islington, 2018-02-22 20:28
PR 5929 merged miss-islington, 2018-02-27 19:41
Messages (14)
msg120928 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2010-11-10 18:49
With timezone class added to datetime module, C API should be extended to at the minimum support efficient creation of timezone instances and access to the singleton UTC instance.

I am not sure whether PyDateTime_TimeZone details should be exposed in datetime.h because presumably programmers should access it through the abstract tzinfo interface.
msg251873 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2015-09-29 18:13
Since issue 24773 (PEP 495 implementation) will touch on CAPI, maybe it is time to implement this as well.
msg309214 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-12-30 03:44
Would it be possible to deprecate (or at least stop enhancing) the current datetime C API and add a new capsule based one instead?

We're trying to get extension module authors to *stop* relying on C level globals, since it causes problems with:

- memory usage analysis
- interpreter reinitialisation
- module reloading
- subinterpreters

Unfortunately, the current design of the datetime C API *requires* the use of C level global state.
msg309234 - (view) Author: Paul Ganssle (p-ganssle) * Date: 2017-12-30 15:34
@Nick

I'm certainly fine with re-configuring my PR to center around a new capsule module with deprecation of the old C API (though if you have any examples of what you have in mind that would be helpful to me). Certainly the "C global needs to be initiated" problem has bitten me in the past and I'm certain will continue to bite people in the future.

That said, I think it would be really good if we could get a fast path for timezone creation and access to the UTC singleton into the Python 3.7 release. I think it's kind of a big disparity between the Python and C APIs that's existed for too long already.
msg309264 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-12-31 04:30
On 31 December 2017 at 01:34, Paul Ganssle <report@bugs.python.org> wrote:
> That said, I think it would be really good if we could get a fast path for timezone creation and access to the UTC singleton into the Python 3.7 release. I think it's kind of a big disparity between the Python and C APIs that's existed for too long already.

It turns out the API already *is* exported as a capsule:
https://github.com/python/cpython/blob/master/Include/datetime.h#L149

Our general guidance for capsule usage is just outdated, and the
datetime API follows that dated advice:
https://docs.python.org/3/extending/extending.html#providing-a-c-api-for-an-extension-module

I'll file a separate issue for that docs problem later (the tracker is
reporting 502 Proxy Error for me right now)
msg309267 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-12-31 05:28
https://bugs.python.org/issue32459 covers defining a module-reloading-friendly way of using capsules.

For this issue, I now think it makes sense to just ignore that problem, and add whatever you need to the existing API.
msg310639 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2018-01-24 22:29
New changeset 04af5b1ba9eb546a29735fac6cb5298159069b53 by Alexander Belopolsky (Paul Ganssle) in branch 'master':
bpo-10381: Add timezone to datetime C API (#5032)
https://github.com/python/cpython/commit/04af5b1ba9eb546a29735fac6cb5298159069b53
msg310662 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-01-25 08:08
Hunting reference leaks with "./python -m test -R 3:3 test_datetime" crashs since the commit 04af5b1ba9eb546a29735fac6cb5298159069b53.

***
vstinner@apu$ ./python -m test -R 3:3 test_datetime 
Run tests sequentially
0:00:00 load avg: 1.05 [1/1] test_datetime
beginning 6 repetitions
123456
Fatal Python error: Segmentation fault

Current thread 0x00007ff71fe33040 (most recent call first):
  File "/home/vstinner/prog/python/master/Lib/test/datetimetester.py", line 5464 in test_utc_capi
  (...)
Segmentation fault (core dumped)
***

Shorter example without -R, just run the same test twice:
***
vstinner@apu$ ./python -m test -v test_datetime test_datetime -m test_utc_capi
== CPython 3.7.0a4+ (heads/master:cab0b2b053, Jan 25 2018, 09:06:01) [GCC 7.2.1 20170915 (Red Hat 7.2.1-2)]
== Linux-4.14.13-300.fc27.x86_64-x86_64-with-fedora-27-Twenty_Seven little-endian
== cwd: /home/vstinner/prog/python/master/build/test_python_26308
== CPU count: 8
== encodings: locale=UTF-8, FS=utf-8
Run tests sequentially
0:00:00 load avg: 1.26 [1/2] test_datetime
test_utc_capi (test.datetimetester.CapiTest_Pure) ... skipped 'Not relevant in pure Python'
test_utc_capi (test.datetimetester.CapiTest_Fast) ... ok

----------------------------------------------------------------------
Ran 2 tests in 0.001s

OK (skipped=1)
0:00:00 load avg: 1.26 [2/2] test_datetime
test_utc_capi (test.datetimetester.CapiTest_Pure) ... skipped 'Not relevant in pure Python'
test_utc_capi (test.datetimetester.CapiTest_Fast) ... Fatal Python error: Segmentation fault

Current thread 0x00007f9836b31040 (most recent call first):
  File "/home/vstinner/prog/python/master/Lib/test/datetimetester.py", line 5464 in test_utc_capi
  (...)
Segmentation fault (core dumped)
***
msg310676 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-01-25 13:58
Ok, this time it should be fixed for real ;-)
msg310688 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-01-25 17:17
Oh, PR 5317 title mentionned this bpo number, but the merged change omits the bpo number and so wasn't mentioned on this issue.

PR 5317 added:

commit 58dc03c737a71de93edef7723c9f6186116288ef (upstream/master, master)
Author: Paul Ganssle <pganssle@users.noreply.github.com>
Date:   Thu Jan 25 08:58:07 2018 -0500

    Cleanup dangling reference in get_timezone_utc_capi (#5317)
msg313020 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2018-02-27 19:41
New changeset 5bd04f964b4f1bcdbd0fa36de04f087c2db07cfe by Alexander Belopolsky (Paul Ganssle) in branch 'master':
bpo-10381, bpo-32403: What's new entries for changes to datetime (gh-5814)
https://github.com/python/cpython/commit/5bd04f964b4f1bcdbd0fa36de04f087c2db07cfe
msg313022 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2018-02-27 19:58
New changeset fff596f792a0752b0e571fa57809e5752aba6353 by Alexander Belopolsky (Miss Islington (bot)) in branch '3.7':
bpo-10381, bpo-32403: What's new entries for changes to datetime (gh-5814) (gh-5929)
https://github.com/python/cpython/commit/fff596f792a0752b0e571fa57809e5752aba6353
msg321105 - (view) Author: Paul Ganssle (p-ganssle) * Date: 2018-07-05 15:03
This can be closed now, I think.
msg321117 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-07-05 16:52
I tested manually "./python -m test test_datetime test_datetime" in 3.6, 3.7 and master branches: I confirm that the test no longer crash. It has been fixed by the commit 58dc03c737a71de93edef7723c9f6186116288ef.

Moreover, I don't recall a recent crash on Windows Refleak or Gentoo Refleak buildbots.

Thanks again Paul Ganssle for this nice enhancement!
History
Date User Action Args
2018-07-05 16:52:42vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg321117

stage: patch review -> resolved
2018-07-05 15:03:09p-gansslesetmessages: + msg321105
2018-02-27 19:58:30belopolskysetmessages: + msg313022
2018-02-27 19:41:42miss-islingtonsetpull_requests: + pull_request5700
2018-02-27 19:41:33belopolskysetmessages: + msg313020
2018-02-22 20:28:20miss-islingtonsetpull_requests: + pull_request5596
2018-02-22 16:33:18p-gansslesetpull_requests: + pull_request5588
2018-01-25 17:17:10vstinnersetmessages: + msg310688
2018-01-25 14:09:02p-gansslesetpull_requests: + pull_request5164
2018-01-25 13:58:40vstinnersetmessages: + msg310676
2018-01-25 13:35:07p-gansslesetstage: resolved -> patch review
pull_requests: + pull_request5163
2018-01-25 08:08:39vstinnersetstatus: closed -> open

nosy: + vstinner
messages: + msg310662

resolution: fixed -> (no value)
2018-01-24 22:30:42belopolskysetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-01-24 22:29:37belopolskysetmessages: + msg310639
2017-12-31 05:28:49ncoghlansetmessages: + msg309267
2017-12-31 04:30:12ncoghlansetmessages: + msg309264
2017-12-30 15:34:04p-gansslesetmessages: + msg309234
2017-12-30 03:44:58ncoghlansetnosy: + ncoghlan
messages: + msg309214
2017-12-28 20:20:23p-gansslesetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request4919
2017-12-28 18:48:56p-gansslesetnosy: + p-ganssle
2015-09-29 18:13:31belopolskysetnosy: + tim.peters

messages: + msg251873
versions: + Python 3.6, - Python 3.5
2014-06-17 20:56:29BreamoreBoysetversions: + Python 3.5, - Python 3.2
2010-11-10 18:49:05belopolskycreate