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

Fix pluralization in TypeError messages in getargs.c #78374

Closed
tirkarthi opened this issue Jul 23, 2018 · 4 comments
Closed

Fix pluralization in TypeError messages in getargs.c #78374

tirkarthi opened this issue Jul 23, 2018 · 4 comments
Labels
3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@tirkarthi
Copy link
Member

BPO 34193
Nosy @rhettinger, @terryjreedy, @serhiy-storchaka, @pppery, @tirkarthi
PRs
  • bpo-34193: Fix pluralization in getargs.c and test cases #8438
  • 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 2018-12-24.13:18:25.682>
    created_at = <Date 2018-07-23.05:54:16.102>
    labels = ['interpreter-core', 'type-feature', '3.8']
    title = 'Fix pluralization in TypeError messages in getargs.c'
    updated_at = <Date 2018-12-24.13:18:25.682>
    user = 'https://github.com/tirkarthi'

    bugs.python.org fields:

    activity = <Date 2018-12-24.13:18:25.682>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-12-24.13:18:25.682>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2018-07-23.05:54:16.102>
    creator = 'xtreak'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 34193
    keywords = ['patch']
    message_count = 4.0
    messages = ['322176', '322296', '322298', '332424']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'terry.reedy', 'serhiy.storchaka', 'ppperry', 'xtreak']
    pr_nums = ['8438']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue34193'
    versions = ['Python 3.8']

    @tirkarthi
    Copy link
    Member Author

    This ticket is to address comments by @terry.reedy on GitHub for PR #8395.

    Ref : #8395 (comment)

    Tests: In the 3 tests before this, the suffix 0, 2, or 3 is the number of arguments given. This would suggest 0a, 0b, 2a, and 3a for the suffixes here. I like 1min, 2min, 1max, and 2max even better. You can wait a bit for other reviews before changing.

    Blurb:

    Let us be more specific. For the patch as is,
    Fix pluralization of 'argument' in TypeError message with 'expected at least/most 1 argument', instead of '1 arguments'.

    But if we add all the remaining fixes needed in this file (see below), we could write.
    Fix pluralization in TypeError messages in getargs.c: '1 argument' instead of '1 arguments' and '1 element' instead of '1 elements'.

    Fixes: This patch follows existing expressions formatted into argument%s:
    line 380: (nargs < min ? min : max) == 1 ? "" : "s",
    lines 1667 and 2093: (len == 1) ? "" : "s", (parens not needed)

    Scope: I think we should finish the job in the file. The next line in both else clauses,
    "unpacked tuple should have %s%zd elements,"
    should get the same treatment. This may break existing tests. (These are the only two formatted 'elements'.

    Fixes for the following will break some test, which will then need fixing also.
    542: "expected %d arguments, not %" PY_FORMAT_SIZE_T "d",
    1720, "%.200s%s takes %s %d positional arguments"
    1799, "%.200s%s takes %s %d positional arguments"
    2106, "%200s%s takes %s %d positional arguments (%d given)",
    2154: "%.200s%s takes %s %d positional arguments"

    I checked all PyErr_Format entries and did not see anything other than %d arguments and %d elements.

    @tirkarthi tirkarthi added 3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Jul 23, 2018
    @serhiy-storchaka
    Copy link
    Member

    See also "expected %d arguments, got %zd" in Objects/typeobject.c.

    @tirkarthi
    Copy link
    Member Author

    Thanks, this needs to be fixed only in check_num_args and can help in all the other functions like lt, delattr etc. in the file that use this.

    Before suggestion :

    ➜  cpython git:(bpo34193) ✗ ./python -c "a = {}; a.__delattr__()"
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    TypeError: expected 1 arguments, got 0

    After suggested change :

    ➜  cpython git:(bpo34193) ✗ ./python -c "a = {}; a.__delattr__()"
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    TypeError: expected 1 argument, got 0 

    Is there some doc on clarification of the difference between 'required' argument and positional argument errors. I was trying to write to tests for the changes and I couldn't find functions that hit the exact branch of code. Some of them generate required argument error and some of them generate positional argument expected error. I looked up in the argument clinic docs (https://docs.python.org/3/howto/clinic.html) but it's little hard for me to grasp as a beginner. Any pointers will be helpful.

    Thanks

    @serhiy-storchaka
    Copy link
    Member

    New changeset 6326278 by Serhiy Storchaka (Xtreak) in branch 'master':
    bpo-34193: Fix pluralization in getargs.c and test cases. (GH-8438)
    6326278

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants