Josh> OK, it was pretty straightforward. Thanks for the direction.
Glad to help.
Josh> To whom should I send the patch (attached)?
Patches should be posted to SourceForge using this form:
http://sourceforge.net/tracker/?func=add&group_id=5470&atid=305470
Note that you will have to be registered at SourceForge if you aren't
already. Full details about submitting patches are here:
http://www.python.org/patches/
(though I think we're just as happy to read unified diffs as context diffs
these days).
The patch you posted looks like a good start. I have a few quick comments:
* The references returned by PySequence_GetItem() are leaking. An easy
way to see is to run it in an infinite loop and watch your process's
memory size:
while True:
x = datetime.datetime.strptime("2004-12-01", "%Y-%m-%d")
If it's leaking, memory size should grow pretty quickly. A debug
build will also show the memory growth:
>>> x = datetime.datetime.strptime("2004-12-01", "%Y-%m-%d")
[29921 refs]
>>> x = datetime.datetime.strptime("2004-12-01", "%Y-%m-%d")
[29928 refs]
>>> x = datetime.datetime.strptime("2004-12-01", "%Y-%m-%d")
[29935 refs]
>>> x = datetime.datetime.strptime("2004-12-01", "%Y-%m-%d")
[29942 refs]
You can see the leaking references quite clearly.
* The seventh item returned from time.strptime() is the day of the week.
You're passing it into the microsecond arg of the datetime constructor
and ignoring the timezone info (ninth item returned from
time.strptime(), eighth arg to the datetime constructor).
* The return values of the Py* calls must always be checked.
* A documentation patch for Doc/lib/libdatetime.tex is needed.
* Lib/test/test_datetime.tex should be extended to add one or more test
cases for the new constructor.
* It would be nice if the import happened only once and was cached:
static PyObject *time_module;
...
if (time_module == NULL &&
(time_module = PyImport_ImportModule("time")) == NULL)
return NULL;
obj = PyObject_CallMethod(time_module, ...);
After awhile these sorts of things become second nature. They are part of
the boilerplate/details required for most changes though.
Skip
--
http://mail.python.org/mailman/listinfo/python-list
