Skip to content

PEP 3121 Refactoring applied to _csv module #58937

Closed

Description

@RobinSchreiber
BPO 14732
Nosy @loewis, @smontanaro, @pitrou, @merwok
Files
  • csv_pep3121.patch: Patches csv extension module for PEP3121 compliance.
  • refactoring_test.py
  • csv_pep3121_fix1.patch: Adds the missing Py_INCREF statements mentioned by Antoine
  • 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 = None
    closed_at = <Date 2012-05-16.09:36:11.869>
    created_at = <Date 2012-05-05.19:47:18.413>
    labels = ['extension-modules', 'type-feature']
    title = 'PEP 3121 Refactoring applied to _csv module'
    updated_at = <Date 2012-05-16.10:04:09.328>
    user = 'https://bugs.python.org/RobinSchreiber'

    bugs.python.org fields:

    activity = <Date 2012-05-16.10:04:09.328>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2012-05-16.09:36:11.869>
    closer = 'pitrou'
    components = ['Extension Modules']
    creation = <Date 2012-05-05.19:47:18.413>
    creator = 'Robin.Schreiber'
    dependencies = []
    files = ['25473', '25474', '25559']
    hgrepos = []
    issue_num = 14732
    keywords = ['patch']
    message_count = 10.0
    messages = ['160032', '160033', '160192', '160200', '160423', '160425', '160516', '160818', '160819', '160823']
    nosy_count = 6.0
    nosy_names = ['loewis', 'skip.montanaro', 'pitrou', 'eric.araujo', 'python-dev', 'Robin.Schreiber']
    pr_nums = []
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue14732'
    versions = ['Python 3.3']

    Activity

    1. RobinSchreiber commented on May 5, 2012

      RobinSchreibermannequin
      MannequinAuthor

      This patch presents my first try to apply the proposed Refactoring of PEP-3121 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.

    2. RobinSchreiber commented on May 5, 2012

      RobinSchreibermannequin
      MannequinAuthor

      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.

    3. smontanaro commented on May 8, 2012

      @smontanaro
      Contributor

      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

    4. pitrou commented on May 8, 2012

      @pitrou
      Member

      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.

    5. merwok commented on May 11, 2012

      @merwok
      Member

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

    6. smontanaro commented on May 11, 2012

      @smontanaro
      Contributor

      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

    7. python-dev commented on May 13, 2012

      python-devmannequin
      Mannequin

      New changeset 90cf321615e5 by Antoine Pitrou in branch 'default':
      Remove Skip from the csv experts (see issue bpo-14732).
      http://hg.python.org/devguide/rev/90cf321615e5

    8. python-dev commented on May 16, 2012

      python-devmannequin
      Mannequin

      New changeset 2496602a56e5 by Antoine Pitrou in branch 'default':
      Issue bpo-14732: The _csv module now uses PEP-3121 module initialization.
      http://hg.python.org/cpython/rev/2496602a56e5

    9. pitrou commented on May 16, 2012

      @pitrou
      Member

      Thanks for the updated patch, Robin. I have now committed it to the default branch.

    10. pitrou commented on May 16, 2012

      @pitrou
      Member

      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.

    11. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      extension-modulesC modules in the Modules dirtype-featureA feature request or enhancement

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions