Skip to content

Derby #9: Convert 52 sites to Argument Clinic across 11 files #64377

Description

@larryhastings
BPO 20178
Nosy @larryhastings, @meadori, @berkerpeksag, @serhiy-storchaka, @erlend-aasland
Files
  • issue20178-cyptes-01.patch
  • issue20178-sqlite-01.patch
  • issue20178-cyptes-02.patch
  • 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/meadori'
    closed_at = None
    created_at = <Date 2014-01-08.00:14:38.886>
    labels = ['extension-modules', 'type-feature', '3.8', 'expert-argument-clinic']
    title = 'Derby #9: Convert 52 sites to Argument Clinic across 11 files'
    updated_at = <Date 2021-06-22.22:57:35.751>
    user = 'https://github.com/larryhastings'

    bugs.python.org fields:

    activity = <Date 2021-06-22.22:57:35.751>
    actor = 'erlendaasland'
    assignee = 'meador.inge'
    closed = False
    closed_date = None
    closer = None
    components = ['Extension Modules', 'Argument Clinic']
    creation = <Date 2014-01-08.00:14:38.886>
    creator = 'larry'
    dependencies = []
    files = ['33438', '33470', '39296']
    hgrepos = []
    issue_num = 20178
    keywords = ['patch']
    message_count = 14.0
    messages = ['207634', '207740', '208000', '208001', '208130', '208186', '208187', '208381', '208414', '224759', '242607', '242649', '324851', '396373']
    nosy_count = 5.0
    nosy_names = ['larry', 'meador.inge', 'berker.peksag', 'serhiy.storchaka', 'erlendaasland']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue20178'
    versions = ['Python 3.8']

    Activity

    1. larryhastings commented on Jan 8, 2014

      @larryhastings
      ContributorAuthor

      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:
      Modules/_ctypes/_ctypes.c: 8 sites
      Modules/_ctypes/_ctypes_test.c: 1 sites
      Modules/_ctypes/callproc.c: 14 sites
      Modules/_ctypes/stgdict.c: 0 sites
      Modules/_curses_panel.c: 3 sites
      Modules/_sqlite/cache.c: 1 sites
      Modules/_sqlite/connection.c: 12 sites
      Modules/_sqlite/cursor.c: 5 sites
      Modules/_sqlite/microprotocols.c: 1 sites
      Modules/_sqlite/module.c: 6 sites
      Modules/_sqlite/row.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

    2. meadori commented on Jan 9, 2014

      @meadori
      Member

      I will pick this one up.

    3. self-assigned this
      on Jan 9, 2014
    4. meadori commented on Jan 13, 2014

      @meadori
      Member

      Larry, the attached patch converts what is convertible of
      Modules/_ctypes/_ctypes. Although, I ran into an odd case
      with this conversion: the converted functions are each used
      in *multiple* PyMethodDef tables. So while clinic can generate
      equivalent code, the builtins are not all pinned to one type.
      The type I pinned it to is the same type that the methods
      are documented against:
      http://docs.python.org/2/library/ctypes.html#ctypes.\_CData

      Since clinic makes you explicitly specify the fully qualified method
      name, I assume sharing like this is not allowed.

      Thoughts?

    5. meadori commented on Jan 13, 2014

      @meadori
      Member

      None of the sites in Modules/_curses_panel.c look convertible.

    6. meadori commented on Jan 15, 2014

      @meadori
      Member

      Attached is a first cut for sqlite3. It is generally OK, but I have the following
      nits:

      * As is probably expected, __init__ and __call__ procs can't be converted.
      
      * sqlite3.Connection.{execute, executemany, executescript} don't use
        PyArg_Parse*.
      
      * The clinic version of 'sqlite3.adapt' has an argument string of "O|OO".
        The first optional parameter defaults to 'pysqlite_PrepareProtocolType'
        in C and 'sqlite3.ProtocolType' in Python.  However, I don't know how to
        represent the Python value because Python default values are 'eval'd by
        clinic *and* the 'sqlite3' module is not imported for the eval.
      
      * The clinic version of 'sqlite3.Cursor.fetchmany' has an arguments string
        of "|i".  The first parameter defaults to
        '((pysqlite_Cursor *)self)->arraysize' in C and 'self.arraysize' in
        Python.  I don't know how to express the Python initialization.
      
      * 'sqlite3.complete' uses 'as module_complete' because 'sqlite3_complete'
        is a public SQLite API function: http://www.sqlite.org/c3ref/complete.html.
        I used 'as' on all the other functions in 'module.c' as well.  Mainly
        for local consistency.
      
      * '_pysqlite_query_execute' required a little refactoring due to the fact
        that it is a utility function used to implement 'sqlite3.Cursor.execute'
        and 'sqlite3.Cursor.executemany'.  'PyArg_ParseTuple' was being called in
        different way depending on a control parameter.
      
      * I didn't convert 'sqlite3.Cursor.setinputsizes' and
        'sqlite3.Cursor.setoutputsize' because they share a docstring and
        simple no-op method def.
      
      * I didn't convert 'sqlite.connect' because it would have required packaing
        the arguments back up to pass to 'PyObject_Call'.
      
    7. larryhastings commented on Jan 15, 2014

      @larryhastings
      ContributorAuthor

      In the past few days I added "cloning" functions, which lets you reuse the parameters and return converter from an existing function. That might help with Modules/_ctypes/_ctypes.

    8. larryhastings commented on Jan 15, 2014

      @larryhastings
      ContributorAuthor

      All the functions in curses_panel are convertable. The threeMETH_NOARGS functions simply get no arguments. And here's new_panel:

      new_panel
      window: object(subclass_of='&PyCursesWindow_Type')
      /

    9. larryhastings commented on Jan 18, 2014

      @larryhastings
      ContributorAuthor

      Meador: I'm going to change Argument Clinic so you can get the "args" and "kwargs" values passed in to the impl. That's issue bpo-20291; I already added you to it "nosy" list. With that change in you'll be able to convert "sqlite3.connect".

    10. larryhastings commented on Jan 18, 2014

      @larryhastings
      ContributorAuthor

      Larry Hastings added the comment:

      • As is probably expected, __init__ and __call__ procs
        can't be converted.

      I should have a fix in for __init__ and __call__ later today. (Issue bpo-20294.)
      For the record, you could convert them, you just had to wrap the parsing function
      to deal with the return type difference, and *args / **kwargs might be tricky too.

      * sqlite3.Connection.{execute, executemany, executescript} don't use
        PyArg_Parse*.
      

      In the next few days I'm going to add support for "*args" and "**kwargs", and
      then you'll be able to convert these functions. (Issue bpo-20291.) Again, even
      if Argument Clinic doesn't do any parsing, it's still helpful to have it
      generate signatures.

      * The clinic version of 'sqlite3.adapt' has an argument string of "O|OO".
        The first optional parameter defaults to 'pysqlite_PrepareProtocolType'
        in C and 'sqlite3.ProtocolType' in Python.  However, I don't know how to
        represent the Python value because Python default values are 'eval'd by
        clinic \*and* the 'sqlite3' module is not imported for the eval.
      

      You'll be able to do this soon:

      parameter: object(c_default='pysqlite_PrepareProtocolType') = ProtocolType

      This feature isn't checked in yet, I'm waiting for a review. It's part of
      bpo-20226, which I guess you already noticed.

      • The clinic version of 'sqlite3.Cursor.fetchmany' has an arguments string
        of "|i". The first parameter defaults to
        '((pysqlite_Cursor *)self)->arraysize' in C and 'self.arraysize' in
        Python. I don't know how to express the Python initialization.

      You can't. How about you use a default of -1 and then:

      if (maxrows == -1)
      maxrows = self->arraysize

      • I didn't convert 'sqlite3.Cursor.setinputsizes' and
        'sqlite3.Cursor.setoutputsize' because they share a docstring and
        simple no-op method def.

      I'd prefer it if you converted them anyway. Converting them means they'll
      have signature information which is an unconditional good.

      • I didn't convert 'sqlite.connect' because it would have required
        packaing the arguments back up to pass to 'PyObject_Call'.

      Once I add the ability to pass in *args and **kwargs, you'll be able to
      convert sqlite.connect.

      I think I responded to all your other comments when I reviewed the patch.

      Thanks!

    11. larryhastings commented on Aug 4, 2014

      @larryhastings
      ContributorAuthor

      All the Derby patches should only go into trunk at this point.

    12. serhiy-storchaka commented on May 5, 2015

      @serhiy-storchaka
      Member

      bpo-20178-cyptes-01.patch is outdated due to changes in Argument Clinic and ctypes. Here is updated and extended patch.

    13. serhiy-storchaka commented on May 6, 2015

      @serhiy-storchaka
      Member

      For Modules/_curses_panel.c there is special issue, bpo-20171, with the patch.

      bpo-20178-sqlite-01.patch is applied almost clearly, but due to changes to Argument Clinic it should be updated. Perhaps more functions can be converted (functions that don't use PyArg_Parse* are worth to be converted too).

    14. berkerpeksag commented on Sep 8, 2018

      @berkerpeksag
      Member

      I'm working on converting Modules/_sqlite/* to Argument Clinic.

    15. erlend-aasland commented on Jun 22, 2021

      @erlend-aasland
      Contributor

      FYI, sqlite3 was converted to Argument Clinic in bpo-40956.

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

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions