classification
Title: PEP 3121 Refactoring applied to _csv module
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Robin.Schreiber, eric.araujo, loewis, pitrou, python-dev, skip.montanaro
Priority: low Keywords: patch

Created on 2012-05-05 19:47 by Robin.Schreiber, last changed 2012-05-16 10:04 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
csv_pep3121.patch Robin.Schreiber, 2012-05-05 19:47 Patches csv extension module for PEP3121 compliance. review
refactoring_test.py Robin.Schreiber, 2012-05-05 19:49
csv_pep3121_fix1.patch Robin.Schreiber, 2012-05-13 10:11 Adds the missing Py_INCREF statements mentioned by Antoine review
Messages (10)
msg160032 - (view) Author: Robin Schreiber (Robin.Schreiber) * Date: 2012-05-05 19:47
This patch presents my first try to apply the proposed Refactoring of PEP3121 to the csv module. I have identified three mutable global variables inside the module, two of which are references to PyObjects. I have wrapped all of them inside a dedicated struct, which is traversed by the gc after "freeing" the module. I also defined some macros, to hide functions calls that are now needed because of the newly introduced indirections.
msg160033 - (view) Author: Robin Schreiber (Robin.Schreiber) * Date: 2012-05-05 19:49
The following script should fail before you have applied the bespoken patch: It basically checks wether one of the global PyObjects inside the csv module is being deleted after freeing the csv module.
msg160192 - (view) Author: Skip Montanaro (skip.montanaro) * Date: 2012-05-08 10:14
> Changes by Éric Araujo <merwok@netwok.org>:
>
>
> ----------
> nosy: +skip.montanaro

Thanks, but I'm out of the Python development business, except as it
pertains to my day job...

Skip
msg160200 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-05-08 12:33
PyModule_AddObject steals the value's reference, so you need to INCREF it before. Besides that, I don't see any obvious bug, but perhaps Martin wants to take a look.
msg160423 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-05-11 16:55
Skip: I used the nosy field autocomplete which is based on the experts file in the devguide; I can mark you "retired" in that file so that your name does not show up in autocomplete (but humans will still know that you might be contacted when all else fails, unless you prefer your name to be fully removed).
msg160425 - (view) Author: Skip Montanaro (skip.montanaro) * Date: 2012-05-11 17:03
> Skip: I used the nosy field autocomplete which is based on the experts file in the devguide; I can mark you "retired" in that file so that your name does not show up in autocomplete (but humans will still know that you might be contacted when all else fails, unless you prefer your name to be fully removed).

Thanks.  That would be great.  I don't mind the occasional question,
but don't want people thinking I am going to jump in feet first any
time I'm made nosy on a ticket.

S
msg160516 - (view) Author: Roundup Robot (python-dev) Date: 2012-05-13 12:29
New changeset 90cf321615e5 by Antoine Pitrou in branch 'default':
Remove Skip from the csv experts (see issue #14732).
http://hg.python.org/devguide/rev/90cf321615e5
msg160818 - (view) Author: Roundup Robot (python-dev) Date: 2012-05-16 09:35
New changeset 2496602a56e5 by Antoine Pitrou in branch 'default':
Issue #14732: The _csv module now uses PEP 3121 module initialization.
http://hg.python.org/cpython/rev/2496602a56e5
msg160819 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-05-16 09:36
Thanks for the updated patch, Robin. I have now committed it to the default branch.
msg160823 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-05-16 10:04
Robin, I forgot: could you please sign and send the contrib form at http://www.python.org/psf/contrib/ ?
It is not a copyright assignment, just a piece of paper that formally allows us to license your contribution for distribution with Python.
History
Date User Action Args
2013-08-07 09:05:59belopolskylinkissue15787 dependencies
2012-05-16 10:04:09pitrousetmessages: + msg160823
2012-05-16 09:36:11pitrousetstatus: open -> closed
resolution: fixed
messages: + msg160819

stage: patch review -> resolved
2012-05-16 09:35:30python-devsetmessages: + msg160818
2012-05-13 12:29:43python-devsetnosy: + python-dev
messages: + msg160516
2012-05-13 10:11:07Robin.Schreibersetfiles: + csv_pep3121_fix1.patch
2012-05-11 17:03:36skip.montanarosetmessages: + msg160425
2012-05-11 16:55:24eric.araujosetnosy: + eric.araujo
messages: + msg160423
2012-05-08 12:33:02pitrousetpriority: normal -> low

nosy: + pitrou
messages: + msg160200

stage: patch review
2012-05-08 10:14:28skip.montanarosetnosy: + skip.montanaro
messages: + msg160192
2012-05-08 10:13:30skip.montanarosetnosy: - skip.montanaro
2012-05-08 03:08:37eric.araujosetnosy: + skip.montanaro
2012-05-05 20:21:35loewissetnosy: + loewis
2012-05-05 19:49:43Robin.Schreibersetfiles: + refactoring_test.py

messages: + msg160033
2012-05-05 19:47:18Robin.Schreibercreate