Skip to content

Derby #15: Convert 50 sites to Argument Clinic across 9 files #64351

Closed
Closed
Derby #15: Convert 50 sites to Argument Clinic across 9 files#64351
Assignees
brettcannon
Labels
extension-modulesC modules in the Modules dirtype-featureA feature request or enhancement

Description

@brettcannon
brettcannon
opened on Jan 6, 2014
BPO 20152
Nosy @loewis, @brettcannon, @larryhastings, @serhiy-storchaka
Files
  • import_ac.diff
  • grp_derby.diff: grp module converted to Argument Clinic
  • spwd_derby.diff: spwd converted to Argument Clinic
  • pwd_derby.diff: pwd converted to Argument Clinic
  • fcntl_derby.diff: fcntl converted to Argument Clinic (except for ioctl)
  • pyexpat_derby.diff: pyexpat converted, relying on default self converter
  • multibytecodec_derby.diff: multibytecodec with default self converter
  • array_derby.diff: array module converted to AC
  • cmath_derby.diff: all of cmath converted
  • fcntl_derby.diff
    		•

    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/brettcannon'
closed_at = <Date 2014-11-10.01:26:34.392>
created_at = <Date 2014-01-06.20:18:13.317>
labels = ['extension-modules', 'type-feature']
title = 'Derby #15: Convert 50 sites to Argument Clinic across 9 files'
updated_at = <Date 2014-11-10.01:26:34.391>
user = 'https://github.com/brettcannon'

    bugs.python.org fields:

    activity = <Date 2014-11-10.01:26:34.391>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2014-11-10.01:26:34.392>
closer = 'brett.cannon'
components = ['Extension Modules']
creation = <Date 2014-01-06.20:18:13.317>
creator = 'brett.cannon'
dependencies = []
files = ['33351', '33511', '33513', '33515', '33591', '33828', '33829', '33831', '33839', '37144']
hgrepos = []
issue_num = 20152
keywords = ['patch']
message_count = 35.0
messages = ['207474', '207598', '207625', '207641', '207808', '207809', '208347', '208351', '208353', '208357', '208653', '208668', '209788', '209789', '209826', '224141', '224142', '225678', '225679', '225680', '225681', '225691', '225692', '225694', '225695', '229035', '229036', '229362', '230819', '230822', '230857', '230858', '230859', '230927', '230930']
nosy_count = 5.0
nosy_names = ['loewis', 'brett.cannon', 'larry', 'python-dev', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue20152'
versions = ['Python 3.5']

    Activity

    1. brettcannon commented on Jan 6, 2014

      @brettcannon
      brettcannon
      on Jan 6, 2014
      MemberAuthor

      Since _imp isn't directly exposed it isn't a priority to do the conversion, but for maintenance and such it should still occur.

    2. brettcannon
      self-assigned this
      on Jan 6, 2014
    3. brettcannon
      added
      extension-modulesC modules in the Modules dir
      on Jan 6, 2014

    4. larryhastings commented on Jan 7, 2014

      @larryhastings
      larryhastings
      on Jan 7, 2014
      Contributor

      I'm normalizing the names of these issues so they're easier to read and to search for.

    5. larryhastings
      changed the title [-]Use Argument Clinic with _imp[/-] [+]Derby: Convert the _imp module to use Argument Clinic[/+] on Jan 7, 2014

    6. brettcannon commented on Jan 7, 2014

      @brettcannon
      brettcannon
      on Jan 7, 2014
      MemberAuthor

      I have attached my conversion of import.c. Larry, can you look it over to make sure I did it as expected? I tried to not have to write my own converter for O& as the docs seem to suggest object(converter='...') should work, but I got an error instead.

    7. brettcannon
      assigned
      larryhastings
      and unassigned
      brettcannon
      on Jan 7, 2014

    8. larryhastings commented on Jan 8, 2014

      @larryhastings
      larryhastings
      on Jan 8, 2014
      Contributor

      (Brett: in order to avoid creating 129 differet issues on the tracker, I'm bundling files together into groups of 50 sites. You don't have to tackle the whole list if you don't want to.)

      This issue is part of the Great Argument Clinic Conversion Derby,
      where we're trying to convert as much of Python 3.4 to use
      Argument Clinic as we can before Release Candidate 1 on January 19.

      This issue asks you to change the following bundle of files:
      Python/import.c: 8 sites
      Modules/cmathmodule.c: 8 sites
      Modules/cjkcodecs/multibytecodec.c: 8 sites
      Modules/arraymodule.c: 8 sites
      Modules/pyexpat.c: 7 sites
      Modules/fcntlmodule.c: 7 sites
      Modules/pwdmodule.c: 2 sites
      Modules/spwdmodule.c: 1 sites
      Modules/grpmodule.c: 1 sites

      Talk to me (larry) if you only want to attack part of a bundle.

      For instructions on how to convert a function to work with Argument
      Clinic, read the "howto":
      http://docs.python.org/dev/howto/clinic.html

    9. larryhastings
      changed the title [-]Derby: Convert the _imp module to use Argument Clinic[/-] [+]Derby #15: Convert 50 sites to Argument Clinic across 9 files[/+] on Jan 8, 2014
    10. larryhastings
      added
      type-featureA feature request or enhancement
      on Jan 8, 2014

    11. python-dev commented on Jan 10, 2014

      @python-dev
      python-devmannequin
      on Jan 10, 2014
      Mannequin

      New changeset 1f3242fb0c9c by Brett Cannon in branch 'default':
      Issue bpo-20152: import.c now uses Argument Clinic.
      http://hg.python.org/cpython/rev/1f3242fb0c9c

    12. brettcannon commented on Jan 10, 2014

      @brettcannon
      brettcannon
      on Jan 10, 2014
      MemberAuthor

      Now it's:

      [Modules/cmathmodule.c](https://github.com/python/cpython/blob/main/Modules/cmathmodule.c): 8 sites
[Modules/cjkcodecs/multibytecodec.c](https://github.com/python/cpython/blob/main/Modules/cjkcodecs/multibytecodec.c): 8 sites
[Modules/arraymodule.c](https://github.com/python/cpython/blob/main/Modules/arraymodule.c): 8 sites
[Modules/pyexpat.c](https://github.com/python/cpython/blob/main/Modules/pyexpat.c): 7 sites
[Modules/fcntlmodule.c](https://github.com/python/cpython/blob/main/Modules/fcntlmodule.c): 7 sites
[Modules/pwdmodule.c](https://github.com/python/cpython/blob/main/Modules/pwdmodule.c): 2 sites
[Modules/spwdmodule.c](https://github.com/python/cpython/blob/main/Modules/spwdmodule.c): 1 sites
[Modules/grpmodule.c](https://github.com/python/cpython/blob/main/Modules/grpmodule.c): 1 sites
    13. brettcannon
      unassigned
      larryhastings
      on Jan 10, 2014

    14. brettcannon commented on Jan 17, 2014

      @brettcannon
      brettcannon
      on Jan 17, 2014
      MemberAuthor

      I'm going to start tackling some of these today and checking them in directly. If I don't finish I will update the bug with what I didn't get to.

    15. 24 remaining items

    16. python-dev commented on Aug 22, 2014

      @python-dev
      python-devmannequin
      on Aug 22, 2014
      Mannequin

      New changeset c48980af7df2 by Brett Cannon in branch 'default':
      Issue bpo-20152: Port the pwd module to Argument Clinic.
      http://hg.python.org/cpython/rev/c48980af7df2

    17. python-dev commented on Aug 22, 2014

      @python-dev
      python-devmannequin
      on Aug 22, 2014
      Mannequin

      New changeset 0c2792a8f9db by Brett Cannon in branch 'default':
      Issue bpo-20152: Port pyexpat to Argument Clinic.
      http://hg.python.org/cpython/rev/0c2792a8f9db

    18. brettcannon commented on Aug 22, 2014

      @brettcannon
      brettcannon
      on Aug 22, 2014
      MemberAuthor

      To make it easier to track what is left to commit:

      • fcntl [does not apply cleanly
      • array
      • cmath [Martin says this no longer applies cleanly]

    19. python-dev commented on Oct 10, 2014

      @python-dev
      python-devmannequin
      on Oct 10, 2014
      Mannequin

      New changeset f7ab0884467e by Brett Cannon in branch 'default':
      Issue bpo-20152: Port the array module to Argument Clinic.
      https://hg.python.org/cpython/rev/f7ab0884467e

    20. brettcannon commented on Oct 10, 2014

      @brettcannon
      brettcannon
      on Oct 10, 2014
      MemberAuthor

      Only ones left are:

      • fcntl
      • cmath

      Both no longer apply cleanly.

    21. python-dev commented on Oct 14, 2014

      @python-dev
      python-devmannequin
      on Oct 14, 2014
      Mannequin

      New changeset 5e8b94397f81 by Brett Cannon in branch 'default':
      Issue bpo-20152: Convert the cmath module to Argument Clinic.
      https://hg.python.org/cpython/rev/5e8b94397f81

    22. brettcannon commented on Nov 7, 2014

      @brettcannon
      brettcannon
      on Nov 7, 2014
      MemberAuthor

      Here is a new patch for fcntl. I would like a review since I had to get a bit tricky to handle the polymorphic arguments for fcntl and ioctl and I don't think the test suite is that thorough for this module.

    23. serhiy-storchaka commented on Nov 7, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Nov 7, 2014
      Member

      I found bugs or doubtful code in the fcntl module. They should be fixed before converting to Argument Clinic.

    24. brettcannon commented on Nov 8, 2014

      @brettcannon
      brettcannon
      on Nov 8, 2014
      MemberAuthor

      So I disagree that the code needs to be tweaked before converting to Argument Clinic. If the Clinic conversion is not adding to the problem then the code churn is just going to make applying this patch that much harder.

      Thanks for the code review regardless, though!

    25. serhiy-storchaka commented on Nov 8, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Nov 8, 2014
      Member

      If first convert to Argument Clinic then fixing bugs will be much harder.

    26. serhiy-storchaka commented on Nov 8, 2014

      @serhiy-storchaka
      serhiy-storchaka
      on Nov 8, 2014
      Member

      About argument names. You have changed argument names and docstrings in any case (e.g. was "op", now "code"). Why not conform with standard documentation? This wouldn't add additional code churn if change it now. But will add if change it later.

    27. python-dev commented on Nov 10, 2014

      @python-dev
      python-devmannequin
      on Nov 10, 2014
      Mannequin

      New changeset 6e6532d313a1 by Brett Cannon in branch 'default':
      bpo-20152, 22821: Port the fcntl module to Argument Clinic.
      https://hg.python.org/cpython/rev/6e6532d313a1

    28. brettcannon commented on Nov 10, 2014

      @brettcannon
      brettcannon
      on Nov 10, 2014
      MemberAuthor

      Finally finished!

      I am never trusting Larry to count anything ever again. ;)

    29. brettcannon
      closed this as completedon Nov 10, 2014
    30. ezio-melotti
      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

    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