Repository navigation
Add pure Python implementation of datetime module to CPython #52237
Description
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:
assignee = 'https://github.com/abalkin'
closed_at = <Date 2010-07-23.19:32:38.815>
created_at = <Date 2010-02-22.16:22:07.251>
labels = ['type-feature', 'library']
title = 'Add pure Python implementation of datetime module to CPython'
updated_at = <Date 2010-07-23.19:32:38.814>
user = 'https://github.com/briancurtin'bugs.python.org fields:
activity = <Date 2010-07-23.19:32:38.814>
actor = 'belopolsky'
assignee = 'belopolsky'
closed = True
closed_date = <Date 2010-07-23.19:32:38.815>
closer = 'belopolsky'
components = ['Library (Lib)']
creation = <Date 2010-02-22.16:22:07.251>
creator = 'brian.curtin'
dependencies = []
files = ['17698', '17701', '17718', '17842', '17845', '17848', '17978', '17992', '18130', '18148', '18155']
hgrepos = []
issue_num = 7989
keywords = ['patch']
message_count = 91.0
messages = ['99774', '99801', '99804', '107171', '107258', '107272', '107275', '107292', '107293', '107295', '107298', '107302', '107303', '107308', '107309', '107312', '107317', '107333', '107451', '107453', '107454', '108019', '108020', '108021', '108023', '108024', '108025', '108026', '108027', '108028', '108029', '108030', '108035', '108037', '108047', '108048', '108051', '108053', '108055', '108056', '108059', '108089', '108090', '108122', '108124', '108126', '108129', '108158', '108179', '108553', '108624', '108643', '108645', '108648', '108649', '108666', '108682', '108687', '108697', '108743', '108747', '109058', '109061', '109063', '109067', '109069', '109128', '109129', '109131', '109134', '109135', '109136', '109140', '109142', '109144', '109146', '109147', '109148', '109149', '109152', '109286', '109662', '110155', '110248', '111097', '111131', '111210', '111211', '111338', '111356', '111375']
nosy_count = 16.0
nosy_names = ['lemburg', 'tim.peters', 'brett.cannon', 'rhettinger', 'amaury.forgeotdarc', 'mark.dickinson', 'davidfraser', 'belopolsky', 'pitrou', 'vstinner', 'techtonik', 'giampaolo.rodola', 'eric.araujo', 'r.david.murray', 'brian.curtin', 'daniel.urban']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue7989'
versions = ['Python 3.2']
Activity
briancurtin commentedon Feb 22, 2010
After discussion on numerous issues, python-dev, and here at the PyCon sprints, it seems to be a good idea to move timemodule.c to _timemodule.c and convert as much as possible into pure Python. The same change seems good for datetime.c as well.
bitdancer commentedon Feb 22, 2010
By 'convert', I believe you mean 'create a python implementation of'. That is, there's no reason to drop any c code, just to create parallel python versions when possible.
Also note that one of the alternate implementations (IronPython I think) has an implementation of DateTime in Python that they plan to contribute.
briancurtin commentedon Feb 22, 2010
Correct, your wording is better than mine.
I'll ask around and see where that datetime module may be and what it's state is.
abalkin commentedon Jun 6, 2010
As far as I remember, the datetime module started as a pure python module and was reimplemented in C around year 2003 or so. One of the important additions at that time was the C API to datetime functionality. I am afraid that with the _timemodule.c/timemodule.py split there will be more an more functionality that is awkward to access from C API.
malemburg commentedon Jun 7, 2010
Alexander Belopolsky wrote:
That's correct, though the main reason for rewriting the module in
C was to gain performance - this is essential for basic types like
date/time types.
-1 on undoing the C rewrite.
It would be much better to spell out the problems you mention and
provide patches to implement solutions for them.
abalkin commentedon Jun 7, 2010
For the datetime module there are also a few more subtle issues that would make it difficult to make a hybrid C/Python implementation. The problem is that unlike the time module where most of the functionality is implemented in module level functions, datetime functionality is entirely in class methods.
One may suggest to use inheritance and derive datetime classes from those implemented in _datetime.c, but this will not work because for example datetime + timedelta will return a _datetime class unless __add__ is overridden in Python. There is another even less obvious issue with inheriting from datetime classes: see issue bpo-5516.
Therefore, it looks like there are only two choices:
Replicate all functionality in both _datetime.c and datetime.py and thus double the cost of implementing new features in CPython. (OK, maybe not double because Python development is easier than C, but increase instead of decrease.)
Incur a performance penalty in every method because every C call wil have to be wrapped in Python.
Another -1 from me.
briancurtin commentedon Jun 7, 2010
It seems like this might not be worth it or a good idea, and I have no strong feeling for this being done. Feel free to close/reject this one.
abalkin commentedon Jun 8, 2010
Brett Cannon wrote in a comment (msg106498) on another issue:
"""
The stated long-term goal of the stdlib is to minimize the C extension modules to only those that have to be written in C (modules can still have performance enhancing extension back-ends). Since datetime does not meet that requirement it's not a matter of "if" but "when" datetime will get a pure Python version and use the extension code only for performance.
"""
I'll keep this open for a while to give Brett and others a chance to respond to opposition.
abalkin commentedon Jun 8, 2010
Also, my opposition is only to splitting datetime. While I am not against splitting the time module, I believe it should be phased out eventually and posix compatibility portion folded into posix module.
brettcannon commentedon Jun 8, 2010
So I see a couple of objections here to the idea that I will try to address.
First is MAL's thinking that this will undo any C code, which it won't. The idea is that stdlib modules that do not inherently rely on other C code (e.g. sqlite3 does not fall underneath this) would have a pure Python implementation with possible C enhancements. In the case of datetime that code is done, so it won't go anywhere. In this case it would be bringing in a pure Python implementation like the one PyPy maintains. You can look at heapq if you want an existing example of what it looks like to maintain a pure Python and C version of a module.
Alexander is worried about performance because of Python shims and duplication of work. For the performance issue, there is none. If you look at something like heapq you will see that there is an
import *in there to pull in all extension code; there is no Python shim to pass through if you don't engineer the extension that way. So in datetime's case all of the extension code would just get pulled into datetime.py directly and not have any indirection cost.As for duplication of work, we already have that with datetime in the other Python VMs. IronPython, Jython, and PyPy have to do their own version of datetime because the stdlib doesn't provide a pure Python version for them to use. So while CPython might not be duplicating work, other people are. Besides, people typically prototype in Python anyway (including you, Alexander, for the UTC patch) and then write the C code, so there really isn't any wasted development cycles by having the Python and C version already.
The key thing to remember is that this is so the VMs other than CPython are not treated as second-class. They are legit implementations just like CPython, but because we have this legacy stuff written in C in the stdlib they are put at a disadvantage. It would be better to pool resources and make sure everyone gets to use an equivalent standard library when possible.
And I should also mention I expect the PyPy folks to be the ones to move their code over; I would never expect anyone to re-implement datetime from scratch.
75 remaining items
[-]Add pure Python implementation of datetime module to CPython[/-][+]Add pure Python implementation of datetime module to CPython[/+]abalkin commentedon Jul 2, 2010
I am attaching a patch from bpo-5288 as an example of a change that I would favor more than bpo-7989-cmp.diff. This patch eliminates _utcoffset and _dst methods that duplicate utcoffset and dst, but return integer minutes rather than a timedelta.
I am not checking these changes in sandbox because these are examples of how I plan to improve C implementation when datetime.py makes it into the main tree. I envision such changes to be discussed within context of datetime.py and if approved, implemented in C and committed simultaneously.
Improving datetime.py implementation and making it diverge from C implementation defeats the purpose I see in having datetime.py in the first place.
Antoine?
tim-one commentedon Jul 5, 2010
Sure you can - but you have to write your code to do time arithmetic. The time implementation does so under the covers when two "aware" time objects are compared:
The docs _suggest_ that tzinfo.utcoffset(x, None) return the "standard" UTC offset, but in the end it's up to the tzinfo subclass designer to decide what they want an aware time object to return as an offset.
History has nothing to do with it ;-) There are several uses spelled out in the docs. In addition to "aware" time comparison (mentioned above):
Maintaining the datetime invariant
d == datetime.combine(d.date(), d.timetz())
requires that an "aware" datetime's tzinfo member be attached to at least one of the date and time projections, and since raw dates are always "naive", that leaves the time projection as the only choice.
That said, would have been fine by me if the "naive" versus "aware" distinction had been limited to datetime objects (i.e., if plain date and plain time objects had no notion of time zone). If you need to know why time objects "really" support being aware, you'll have to ask Guido ;-)
abalkin commentedon Jul 9, 2010
issue7989a.diff is a partial success implementing Nick Coghlan's testing idea. Unfortunately, datetime implementation with it's circular dependency on _strftime is not very robust with respect to import trickery.
I am calling this a partial success because although running Lib/test/test_datetime.py does not report any errors, it only works with pure python version of pickle. (I had to add sys.modules['_pickle'] = None at the top of the module to make it work.)
Also, the resulting test_datetime is quite an abomination!
abalkin commentedon Jul 13, 2010
issue9206b.diff fixes test_datetime in issue9206a.diff by restoring sys.modules in-place in tearDown method.
Based on python-dev discussion, I am marking this as accepted and uploading the patch to Rietveld for commit review.
Please comment on the code at http://codereview.appspot.com/1824041 . Unfortunately upload tool does not recognize copied/moved files.
abalkin commentedon Jul 14, 2010
In issue7989c.diff, I reverted to lazy import of _strptime, added cleanup of _xyz helper functions, and made test_datetime more robust by restoring sys.modules more thoroughly.
Unfortunately I've encountered an issue with Rietveld that prevents me from uploading the latest patch for review. See http://code.google.com/p/rietveld/issues/detail?id=208
abalkin commentedon Jul 21, 2010
Is anyone interested in giving this a final review? I would like to check this in unless someone objects or needs time to review.
amauryfa commentedon Jul 21, 2010
Technically, since the C module is now named _datetime, it needs to be renamed in Modules/Setup.dist, and most importantly in PC/config.c (because on Windows datetime is built in the main interpreter)
abalkin commentedon Jul 22, 2010
Fixed in issue7989d.diff, thanks.
In order to commit this patch I need an SVN advise. I would like to copy datetime.py from sandbox to py3k in a way that will preserve the history. (I know, this strictly necessary, but I don't want my name on every line in svn blame datetime.py given how little I had to change there.)
I tried both svn copy oldpath newpath and svn copy oldurl newpath and it did not work (most likely because sandbox and py3k are independent checkouts.) I don't want to use svn copy oldurl newurl because that would require a separate commit for datetime.py.
abalkin commentedon Jul 22, 2010
s/strictly necessary/not strictly necessary/
briancurtin commentedon Jul 23, 2010
One additional change was needed to compile on Windows:
Index: PC/config.c
===================================================================
abalkin commentedon Jul 23, 2010
Brian, thanks for the fix and for testing. I am attaching a commit-ready patch issue7989e.diff, that includes Brian's fix and a few white-space changes.
I hope I've resolved the SVN issue: I was working in a read-only checkout while sandbox checkout was read/write.
Here is the svn status now:
M PCbuild/pythoncore.vcproj
M setup.py
M Misc/NEWS
M PC/config.c
A + Lib/datetime.py
A + Lib/test/datetimetester.py
M Lib/test/test_datetime.py
A + Modules/_datetimemodule.c
D Modules/datetimemodule.c
M Modules/Setup.dist
Note that unlike previous patches, issue7989e.diff, contains only datetime.py differences compared to sandbox. You should do
svn cp svn+ssh://pythondev@svn.python.org/sandbox/branches/py3k-datetime/datetime.py Lib
before it can be applied. Depending on your patch utility, you may also need to do
svn cp Lib/test/test_datetime.py Lib/test/datetimetester.py
I am running final tests and will commit this patch shortly.
abalkin commentedon Jul 23, 2010
Committed in r83112.