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

Undocumented different between METH_KEYWORDS and **kws #62731

Closed
warsaw opened this issue Jul 22, 2013 · 16 comments
Closed

Undocumented different between METH_KEYWORDS and **kws #62731

warsaw opened this issue Jul 22, 2013 · 16 comments
Labels
docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error

Comments

@warsaw
Copy link
Member

warsaw commented Jul 22, 2013

BPO 18531
Nosy @warsaw, @ericvsmith, @alex, @serhiy-storchaka, @iritkatriel
Files
  • test_args_kwargs_types.patch
  • ceval_kwargs_exact_dict.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 2021-12-06.23:30:41.931>
    created_at = <Date 2013-07-22.18:51:43.696>
    labels = ['type-bug', 'docs']
    title = 'Undocumented different between METH_KEYWORDS and **kws'
    updated_at = <Date 2021-12-06.23:30:41.931>
    user = 'https://github.com/warsaw'

    bugs.python.org fields:

    activity = <Date 2021-12-06.23:30:41.931>
    actor = 'iritkatriel'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2021-12-06.23:30:41.931>
    closer = 'iritkatriel'
    components = ['Documentation']
    creation = <Date 2013-07-22.18:51:43.696>
    creator = 'barry'
    dependencies = []
    files = ['42743', '42746']
    hgrepos = []
    issue_num = 18531
    keywords = ['patch']
    message_count = 16.0
    messages = ['193559', '193560', '264943', '264947', '264979', '264982', '264985', '264994', '265160', '265166', '265171', '265181', '265182', '265190', '265674', '407307']
    nosy_count = 8.0
    nosy_names = ['barry', 'eric.smith', 'alex', 'abacabadabacaba', 'docs@python', 'python-dev', 'serhiy.storchaka', 'iritkatriel']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue18531'
    versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

    @warsaw
    Copy link
    Member Author

    warsaw commented Jul 22, 2013

    A colleague discovered an interesting implementation different between C-defined functions and Python-defined functions called with **kws arguments. He tried to do this:

    >>> from collections import defaultdict
    >>> '{foo}{bar}'.format(**defaultdict(str))
    ''

    He wondered how this could work, especially given:

    >>> def what(**kws):
    ...   print(type(kws), repr(kws))
    ... 
    >>> what(**defaultdict(str))
    <class 'dict'> {}

    The Language Reference clearly states that when what() is called, a new dictionary is create, which is populated by keyword arguments not consumed by positional args.

    http://docs.python.org/3/reference/compound_stmts.html#function-definitions
    http://docs.python.org/3/reference/expressions.html#calls

    What isn't described is the behavior when the function is defined in C using METH_KEYWORDS. In that case, the object passed through the ** argument is passed unscathed to the underlying methodobject defined to take METH_KEYWORDS. str.format() is of the latter type so it gets the actual defaultdict. what() of course gets the defined behavior.

    It would be nice if the implementation details of the function did not leak into this behavior, but this is admittedly a corner case of the CPython implementation. I haven't checked any of the alternative implementations to see what they do. My guess is that CPython won't change for practical and backward compatibility reasons, thus I've opened this issue as a documentation bug. At the very least, an implementation note in the Language Reference should be added.

    @warsaw warsaw added the docs Documentation in the Doc dir label Jul 22, 2013
    @alex
    Copy link
    Member

    alex commented Jul 22, 2013

    I'll confirm that PyPy raises a KeyError on the format() code.

    @serhiy-storchaka
    Copy link
    Member

    Yet one difference is that kwargs can be NULL in C function if no keyword arguments are supplied.

    Here is a patch that adds tests for exact types of args and kwargs arguments in C functions.

    I think that we should keep passing NULL as kwargs for performance reasons. But shouldn't we convert dict subclass to exact dict? I think there are good reasons to do this. Some functions can save kwargs for latter use. And this makes the implementation more consistent with PyPy.

    If defaultdict behavior is needed for formatting, there is format_map().

    @serhiy-storchaka serhiy-storchaka added the type-bug An unexpected behavior, bug, or error label May 6, 2016
    @serhiy-storchaka
    Copy link
    Member

    Proposed patch converts var-keyword arguments to exact dict. Since it changes the behavior of str.format() it can be pushed only in 3.6.

    @warsaw
    Copy link
    Member Author

    warsaw commented May 6, 2016

    Eric should chime in on the str.format() implications.

    @ericvsmith
    Copy link
    Member

    I think it's okay to change this as far as str.format() goes.

    Interestingly enough, the motivating case to add str.format_map() in bpo-6081 was exactly this kind of code. Although I notice that the example in that issue, which it shows as failing, works in both 2.7 and 3.4 (the only versions I have handy). So I'm not sure what changed after 2009 to cause that code to start working, but I'd be okay with it breaking again. But maybe we should track it down, in case it was deliberately changed and is important to someone.

    In any event, since str.format_map() is well-known (to me, at least!), and is the approved way of getting the behavior of passing a specific map in to the format machinery, I'm okay with changing **dict to create an exact new dict, at least as far as str.format() goes. I can't speak to the overall implications (such as other APIs and performance).

    I agree it would be a 3.6 only change (and I've change the versions to reflect that).

    @warsaw
    Copy link
    Member Author

    warsaw commented May 6, 2016

    On May 06, 2016, at 03:41 PM, Eric V. Smith wrote:

    I agree it would be a 3.6 only change (and I've change the versions to
    reflect that).

    The reason the other versions were included was because this was originally
    reported as a documentation bug. It should probably still be documented in
    those older releases.

    @ericvsmith
    Copy link
    Member

    Okay. Adding back 3.5 and 2.7, in case someone wants to document the behavior there. I'm not sure if we fix docs in 3.4 still.

    @serhiy-storchaka
    Copy link
    Member

    str.format() uses PyDict_GetItem() in 2.7, but PyObject_GetItem() in 3.x. A comment before:

        /* Use PyObject_GetItem instead of PyDict_GetItem because this
           code is no longer just used with kwargs. It might be passed
           a non-dict when called through format_map. */
    

    Thus the behavior of str.format() was changed by accident.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 8, 2016

    New changeset e4835b1ed7b1 by Serhiy Storchaka in branch 'default':
    Issue bpo-18531: Single var-keyword argument of dict subtype was passed
    https://hg.python.org/cpython/rev/e4835b1ed7b1

    @ericvsmith
    Copy link
    Member

    I'm not sure PyObject_GetItem instead of PyDict_GetItem in 3.x completely explains things, because the code in bpo-6081 also works in 2.7.

    But I don't think it's terribly important in any event, as long as we understand the 3.x difference. Thanks for researching!

    @serhiy-storchaka
    Copy link
    Member

    I'm not sure PyObject_GetItem instead of PyDict_GetItem in 3.x completely
    explains things, because the code in bpo-6081 also works in 2.7.

    It doesn't work to me in current develop 2.7.11+. What version did you test?

    @serhiy-storchaka
    Copy link
    Member

    Now this is just the documentation issue. I think the change of the behavior in 3.6 should be documented too, but I even don't know where.

    @ericvsmith
    Copy link
    Member

    Oops, sorry. I'm an idiot. I was running in a venv where python was python3. Checking with my installed 2.7.10, I see the expected error.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 16, 2016

    New changeset e3283b4ae61d by Serhiy Storchaka in branch '3.5':
    Backported tests for issue bpo-18531.
    https://hg.python.org/cpython/rev/e3283b4ae61d

    New changeset 621e0a3249c2 by Serhiy Storchaka in branch '2.7':
    Backported tests for issue bpo-18531.
    https://hg.python.org/cpython/rev/621e0a3249c2

    @iritkatriel
    Copy link
    Member

    This seems fixed in 3.11:

    >>> from collections import defaultdict
    >>> '{foo}{bar}'.format(**defaultdict(str))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    KeyError: 'foo'

    @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
    docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants