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

Weird keyword parameter names in builtins #73881

Closed
serhiy-storchaka opened this issue Mar 2, 2017 · 19 comments
Closed

Weird keyword parameter names in builtins #73881

serhiy-storchaka opened this issue Mar 2, 2017 · 19 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 29695
Nosy @brettcannon, @rhettinger, @vstinner, @bitdancer, @serhiy-storchaka
PRs
  • bpo-29695: Deprecated using bad named keyword arguments in builtings: #486
  • bpo-29695: Remove bad keyword parameters in int(), bool(), float(), l… #518
  • bpo-29695: Fixed tests after removing keyword args support in some ba… #520
  • [Do Not Merge] Sample of CPython life with blurb. #703
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • deprecate-keyword-x.patch
  • deprecate-keyword-sequence.patch
  • deprecate-bad-keywords.patch
  • remove-bad-keywords.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 = <Date 2017-03-07.19:47:54.772>
    created_at = <Date 2017-03-02.14:54:04.612>
    labels = ['interpreter-core', 'type-feature', '3.7']
    title = 'Weird keyword parameter names in builtins'
    updated_at = <Date 2017-03-31.16:36:09.226>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-03-31.16:36:09.226>
    actor = 'dstufft'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-03-07.19:47:54.772>
    closer = 'serhiy.storchaka'
    components = ['Interpreter Core']
    creation = <Date 2017-03-02.14:54:04.612>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = ['46684', '46685', '46686', '46702']
    hgrepos = []
    issue_num = 29695
    keywords = ['patch']
    message_count = 19.0
    messages = ['288802', '288803', '288804', '288805', '288806', '288808', '288809', '288811', '289052', '289067', '289068', '289105', '289107', '289110', '289114', '289115', '290285', '290289', '290294']
    nosy_count = 5.0
    nosy_names = ['brett.cannon', 'rhettinger', 'vstinner', 'r.david.murray', 'serhiy.storchaka']
    pr_nums = ['486', '518', '520', '703', '552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue29695'
    versions = ['Python 3.7']

    @serhiy-storchaka
    Copy link
    Member Author

    Proposed patches deprecate the "x" keyword parameter in int(), bool() and float() and the "sequence" keyword parameter in list() and tuple(). Name "x" is meaningless, and name "sequence" is misleading (any iterable is accepted, not just sequence). The documentation uses name "iterable" for list() and tuple().

    It is never documented that any of these parameters are accepted by keywords. There was only a test for int(), but it was added just for increasing coverity, not to test intended behavior. Does this mean that the support of keyword arguments can be removed without deprecation?

    The general idea got preliminary approval from Guido (https://mail.python.org/pipermail/python-ideas/2017-March/044959.html).

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Mar 2, 2017
    @bitdancer
    Copy link
    Member

    I don't think we should do it without deprecation, since it could break working code. But it certainly sounds like a marginal case: I doubt there is *much* code that uses the existing keyword names.

    @vstinner
    Copy link
    Member

    vstinner commented Mar 2, 2017

    R. David Murray added the comment:

    I don't think we should do it without deprecation, since it could break working code. But it certainly sounds like a marginal case: I doubt there is *much* code that uses the existing keyword names.

    I'm not sure that it's worth it to add a deprecation warning in Python
    3.7. It's trivial to fix code passing an argument by keyword rather
    than by position.

    @serhiy-storchaka
    Copy link
    Member Author

    Well, I was feeling the same, but asked just for the case.

    Could you then look at deprecation messages? I'm sure that 'The "x" keyword argument of int() is deprecated' is not the best wording, and may be even ugly wording. Could you please suggest better wording David?

    Patches don't document new deprecations because it is not documented that keyword arguments are supported either. Is mentioning in What's New enough?

    @vstinner
    Copy link
    Member

    vstinner commented Mar 2, 2017

    "What's New in Python 3.7" has a section for such changes: "Porting to
    Python 3.7: Changes to the Python API".
    https://docs.python.org/dev/whatsnew/3.7.html#porting-to-python-3-7

    Oh, I see that you already made a similar change but this one had a
    deprecation period of 2 releases (3.5 and 3.6): "A format string
    argument for string.Formatter.format() is now positional-only."

    See also the positional-only arguments discussion on python-ideas:

    @bitdancer
    Copy link
    Member

    So these will become positional only? In that case I'd say "Using 'x' as a keyword argument is deprecated; specify the value as a positional argument instead"

    @serhiy-storchaka
    Copy link
    Member Author

    See also bpo-20283. Obviously wrong keyword parameter names in regex methods were deprecated in 2.7, 3.3 and 3.4 and removed in 3.6.

    @serhiy-storchaka
    Copy link
    Member Author

    Thank you David! Updated warning messages for your suggestion and added entries in Misc/NEWS and What's New.

    @rhettinger
    Copy link
    Contributor

    I concur with Victor and Serhiy that we can bypass a deprecation phase here and just get it done. In the rare case of int(x=3.14), the fix is trivial.

    @serhiy-storchaka
    Copy link
    Member Author

    I already merged the patch as 58d23e6.

    If you think that it would be better to remove without deprecation, the following patch does this.

    @rhettinger
    Copy link
    Contributor

    I do think it would be better to not have the deprecation.

    @vstinner
    Copy link
    Member

    vstinner commented Mar 6, 2017

    If you think that it would be better to remove without deprecation, the following patch does this.

    @serhiy: Can you please create a PR for it?

    @serhiy-storchaka
    Copy link
    Member Author

    If David agrees with this.

    @bitdancer
    Copy link
    Member

    If Raymond is on the side of skipping the deprecation than I'm good with it. Like I said, this is a marginal case.

    @serhiy-storchaka
    Copy link
    Member Author

    Thanks all!

    @vstinner
    Copy link
    Member

    vstinner commented Mar 6, 2017

    Unhappy buildbot.

    http://buildbot.python.org/all/builders/AMD64%20Debian%20root%203.x/builds/456/steps/test/logs/stdio

    ======================================================================
    ERROR: test_keyword_arguments (test.test_descr.ClassPropertiesAndMethods)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_descr.py", line 3451, in test_keyword_arguments
        list.__init__(a, sequence=[0, 1, 2])
    TypeError: list() does not take keyword arguments

    ======================================================================
    ERROR: test_keywords (test.test_descr.ClassPropertiesAndMethods)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_descr.py", line 2888, in test_keywords
        self.assertEqual(int(x=1), 1)
    TypeError: 'x' is an invalid keyword argument for this function

    @brettcannon
    Copy link
    Member

    New changeset d908fd9 by Brett Cannon (Serhiy Storchaka) in branch 'master':
    bpo-29695: Fixed tests after removing keyword args support in some basic type constructors. (GH-520)
    d908fd9

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 2e56424 by Serhiy Storchaka in branch 'master':
    bpo-29695: Remove bad keyword parameters in int(), bool(), float(), list() and tuple(). (#518)
    2e56424

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 58d23e6 by Serhiy Storchaka in branch 'master':
    bpo-29695: Deprecated using bad named keyword arguments in builtings: (#486)
    58d23e6

    @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.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants