Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Derby #5: Convert 50 sites to Argument Clinic across 3 files #64373

Closed
larryhastings opened this issue Jan 7, 2014 · 13 comments
Closed

Derby #5: Convert 50 sites to Argument Clinic across 3 files #64373

larryhastings opened this issue Jan 7, 2014 · 13 comments
Labels
extension-modules C modules in the Modules dir topic-argument-clinic type-feature A feature request or enhancement

Comments

@larryhastings
Copy link
Contributor

BPO 20174
Nosy @loewis, @larryhastings, @pdmccormick
Files
  • argument_clinic_functools.patch: docstrings for _functools.partial pickle protocol methods
  • argument_clinic_socketmodule.patch
  • argument_clinic_socketmodule_v2.patch: v2 socketmodule Argument Clinic conversion
  • argument_clinic_socketmodule_v3.patch: v3 socketmodule Argument Clinic conversion
  • argument_clinic_socketmodule_v4.patch: v4 socketmodule Argument Clinic conversion
  • 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 = None
    created_at = <Date 2014-01-07.23:47:41.904>
    labels = ['extension-modules', 'type-feature', 'expert-argument-clinic']
    title = 'Derby #5: Convert 50 sites to Argument Clinic across 3 files'
    updated_at = <Date 2015-08-02.20:43:00.310>
    user = 'https://github.com/larryhastings'

    bugs.python.org fields:

    activity = <Date 2015-08-02.20:43:00.310>
    actor = 'baikie'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Extension Modules', 'Argument Clinic']
    creation = <Date 2014-01-07.23:47:41.904>
    creator = 'larry'
    dependencies = []
    files = ['33372', '33472', '33489', '33490', '33496']
    hgrepos = []
    issue_num = 20174
    keywords = ['patch']
    message_count = 12.0
    messages = ['207619', '207722', '207819', '207984', '207985', '208134', '208227', '208239', '208264', '224127', '224756', '240722']
    nosy_count = 5.0
    nosy_names = ['loewis', 'baikie', 'larry', 'rmsr', 'pdmccormick']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'needs patch'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue20174'
    versions = ['Python 3.5']

    @larryhastings
    Copy link
    Contributor Author

    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/socketmodule.c: 47 sites
    Modules/socketmodule.h: 1 sites
    Modules/_functoolsmodule.c: 2 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

    @larryhastings larryhastings added type-bug An unexpected behavior, bug, or error extension-modules C modules in the Modules dir type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Jan 7, 2014
    @rmsr
    Copy link
    Mannequin

    rmsr mannequin commented Jan 9, 2014

    Taking a crack at this.

    socketmodule.h: just a comment, skipping.

    _functools.c: kind-of skipping, because the relevant functions are not normally directly called (pickle protocol __reduce__ and __setstate__ on partial, __call__ on cmp wrapper). Will add simple docstrings for the pickle protocol handlers though.

    @larryhastings
    Copy link
    Contributor Author

    While it's dandy to add docstrings, your patch doesn't have any Argument Clinic stuff in it. I don't mind if you add docstrings as part of the process of converting to Argument Clinic, but I'm not interested in this patch as it stands.

    @rmsr
    Copy link
    Mannequin

    rmsr mannequin commented Jan 12, 2014

    Just discovered that the bugtracker mail was all going into my spam filter, yay. Didn't notice your reply until just now.

    The functools patch was just a quickie to get it out of my mental queue, and I'm still working on socketmodule. This is my first time using the Python bugtracker, I don't know if multipart patches are acceptable?

    @larryhastings
    Copy link
    Contributor Author

    I don't know what you mean by a "multipart patch", but the bug tracker has handled every patch I've thrown at it so far. (Assuming that the patch is based on a reasonably fresh checkout of trunk.)

    @rmsr
    Copy link
    Mannequin

    rmsr mannequin commented Jan 15, 2014

    Here's the socketmodule patch. I aggressively imported text from the docs for the docstrings, along with matching parameter names, given how far the old docstrings have drifted over time. The Windows-specific code is untested, but otherwise the tests pass. I tagged functions which can't be converted or whose argument handling is esoteric. These comments can be removed prior to commit.

    I am not sure if one should convert a C-level class's init method.

    @rmsr
    Copy link
    Mannequin

    rmsr mannequin commented Jan 16, 2014

    Forgot to linewrap a paragraph.

    @rmsr
    Copy link
    Mannequin

    rmsr mannequin commented Jan 16, 2014

    Tweaked the argument list for functions using a NULL default. Kludgy but doesn't lie to the user.

    @rmsr
    Copy link
    Mannequin

    rmsr mannequin commented Jan 16, 2014

    Here's sendmsg with only nested bracket optional args. If Rietveld doesn't like this patch I may cry.

    @loewis
    Copy link
    Mannequin

    loewis mannequin commented Jul 27, 2014

    Ryan, 4 hunks of your patch fail to apply now. Can you please update the patch?

    @larryhastings
    Copy link
    Contributor Author

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

    @pdmccormick
    Copy link
    Mannequin

    pdmccormick mannequin commented Apr 13, 2015

    I am working on revising the Argument Clinic definitions for socketmodule.c.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    sobolevn added a commit to sobolevn/cpython that referenced this issue Sep 7, 2022
    sobolevn added a commit to sobolevn/cpython that referenced this issue Sep 7, 2022
    @sobolevn
    Copy link
    Member

    sobolevn commented Sep 7, 2022

    I've submitted a patch for _functools: #96640
    There are some uncoverted items like partial() which needs both *args and **kwargs support from AC.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    extension-modules C modules in the Modules dir topic-argument-clinic type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants