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

Argument Clinic: Fix signature of optional positional-only arguments #73485

Closed
vstinner opened this issue Jan 17, 2017 · 9 comments
Closed

Argument Clinic: Fix signature of optional positional-only arguments #73485

vstinner opened this issue Jan 17, 2017 · 9 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes topic-argument-clinic

Comments

@vstinner
Copy link
Member

BPO 29299
Nosy @rhettinger, @vstinner, @larryhastings, @serhiy-storchaka, @1st1, @remilapeyre
Files
  • ac_optional_positional.patch
  • getattr_ac.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 = None
    closed_at = None
    created_at = <Date 2017-01-17.15:17:14.278>
    labels = ['3.7', '3.8', 'expert-argument-clinic']
    title = 'Argument Clinic: Fix signature of optional positional-only arguments'
    updated_at = <Date 2019-03-15.19:50:22.024>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2019-03-15.19:50:22.024>
    actor = 'remi.lapeyre'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Argument Clinic']
    creation = <Date 2017-01-17.15:17:14.278>
    creator = 'vstinner'
    dependencies = []
    files = ['46316', '46317']
    hgrepos = []
    issue_num = 29299
    keywords = ['patch']
    message_count = 8.0
    messages = ['285650', '285651', '285656', '285657', '285659', '285661', '338014', '338024']
    nosy_count = 6.0
    nosy_names = ['rhettinger', 'vstinner', 'larry', 'serhiy.storchaka', 'yselivanov', 'remi.lapeyre']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue29299'
    versions = ['Python 3.7', 'Python 3.8']

    @vstinner
    Copy link
    Member Author

    When a function has only positional arguments and at least one argument is optional, the expected signature is:

      func(mandatory_arg1, mandatory_arg2[, optional_arg3[, optinal_arg4]])

    For example, the signature of format() is inconsistent with its documentation.

    Signature:
    ---

    $ python3 -c 'help(format)'|cat
    Help on built-in function format in module builtins:
    format(value, format_spec='', /)
        Return value.__format__(format_spec)
        
        format_spec defaults to the empty string

    Documentation:
    ---
    .. function:: format(value[, format_spec])
    ---

    Attached patch is a first attempt to fix the issue. The problem is that my heuristic to check if an argument is "optional" doesn't seem to work as expected in all cases. I chose to check if the C default is NULL.

    The problem is that some functions defines a C default to NULL whereas the Python default is set to a different value and is correct.

    Example with _io.FileIO.truncate:

    /*[clinic input]
    _io.FileIO.truncate
        size as posobj: object = NULL
        /
    

    whereas the documentation says that the default is None:

    .. method:: truncate(size=None)

    It's easy to fix the default, but in this case my change doesn't fix the signature anymore since the C default is still NULL:

    /*[clinic input]
    _io.FileIO.truncate
        size as posobj: object(c_default="NULL") = None
        /
    

    We need a different heuristic than C default is NULL, or we should fix functions where the heuristic fails.

    @vstinner
    Copy link
    Member Author

    List of functions modified when Argument Clinic is run to regenerate .c.h files:

    builtin_format
    builtin_getattr
    builtin_input
    builtin_sum
    cmath_log
    _codecs_ascii_decode
    _codecs_ascii_encode
    _codecs_charmap_decode
    _codecs_charmap_encode
    _codecs_code_page_encode
    _codecs_escape_decode
    _codecs_escape_encode
    _codecs_latin_1_decode
    _codecs_latin_1_encode
    _codecs_mbcs_encode
    _codecs_oem_encode
    _codecs_raw_unicode_escape_decode
    _codecs_raw_unicode_escape_encode
    _codecs_readbuffer_encode
    _codecs_unicode_escape_decode
    _codecs_unicode_escape_encode
    _codecs_unicode_internal_decode
    _codecs_unicode_internal_encode
    _codecs_utf_16_be_encode
    _codecs_utf_16_le_encode
    _codecs_utf_32_be_encode
    _codecs_utf_32_le_encode
    _codecs_utf_7_encode
    _codecs_utf_8_encode
    _dbm_dbm_get
    _dbm_dbm_setdefault
    fcntl_fcntl
    _imp_create_dynamic
    _io_FileIO_truncate
    os_putenv
    os_unsetenv
    pyexpat_xmlparser_ExternalEntityParserCreate
    _sre_SRE_Match_end
    _sre_SRE_Match_span
    _sre_SRE_Match_start
    _tkinter_create
    unicodedata_UCD_decimal
    unicodedata_UCD_digit
    unicodedata_UCD_name
    unicodedata_UCD_numeric
    unicode_lstrip
    unicode_maketrans
    unicode_rstrip

    @vstinner
    Copy link
    Member Author

    See also issue bpo-20291: "Argument Clinic should understand *args and **kwargs parameters".

    @vstinner
    Copy link
    Member Author

    This issue is blocking me to convert more functions to Argument Clinic. See for example attached getattr_ac.patch which converts getattr() to AC. Without ac_optional_positional.patch, AC generates the signature:

    "getattr($module, object, name, default=None, /)\n"

    whereas the following signature is expected:

    "getattr($module, object, name[, default])\n"

    @serhiy-storchaka
    Copy link
    Member

    The problem is that

      func(mandatory_arg1, mandatory_arg2[, optional_arg3[, optinal_arg4]])

    is not compatible with the inspect module.

    In many case a meaningful default value was added if this is possible. For example the Python default shown in the signature can be set to '', 'utf-8' or 'strict' while the C default value is NULL for performance. If the parameter is upper index in the sequence it can be set to sys.maxsize (Py_SSIZE_T_MAX in C).

    This is not always possible. For example there is not default value for dict.pop().

    @serhiy-storchaka
    Copy link
    Member

    Please don't change this part of Argument Clinic without Larry. There were several attempts to solve this problem, I don't like current status, but this is Larry's decision.

    @larryhastings
    Copy link
    Contributor

    Argument Clinic can currently only generate signatures for functions whose signatures can be represented in Python. This is because the signatures are parsed by the inspect module, which in turn uses the ast module to generate a parse tree; it then examines the parse tree and uses that to generate the Signature object.

    (By the time you see a signature of a function using inspect, the signature has actually made a round-trip through the ast module, been turned into a Signature object, then str() has been run on it to re-generate the text representation from scratch. The fact that it's usually identical to the original text signature buried in the function's docstring is a happy accident.)

    Changing Argument Clinic to generate signatures with square brackets in them to signify optional parameters is insufficient. You'd also have to upgrade inspect to support this new syntax--otherwise there'd be no point. And *that* would be a lot of code.

    @remilapeyre
    Copy link
    Mannequin

    remilapeyre mannequin commented Mar 15, 2019

    Thanks for the explanation.

    And *that* would be a lot of code.

    Would an effort to make inspect support this be appreciated or do you think this is not important?

    @remilapeyre remilapeyre mannequin added the 3.8 only security fixes label Mar 15, 2019
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @vstinner
    Copy link
    Member Author

    vstinner commented Nov 3, 2022

    Sadly I don't have the bandwidth to work on this issue, it's inactive for 3 years, I just close the issue.

    @vstinner vstinner closed this as completed Nov 3, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes topic-argument-clinic
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants