Skip to content

"multiple values for keyword" when passing keyword arguments after forwarding **args #2582

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

Closed
z33ky opened this issue Dec 15, 2016 · 14 comments

Comments

@z33ky
Copy link

z33ky commented Dec 15, 2016

This is #2580 with the --fast-parser option.

def spam(x, y): pass

def ok(**kwargs) -> None:
    spam(x=1, **kwargs)  # OK

def err(**kwargs) -> None:
    spam(**kwargs, x=1)  # error: "spam" gets multiple values for keyword argument "x"

I wouldn't think the order matters. The Python interpreter does not have an issue with it.

@gvanrossum
Copy link
Member

@sixolet Is this something that your function checking code would fix?

@sixolet
Copy link
Collaborator

sixolet commented Dec 15, 2016

No, it doesn't handle call sites at all, only function subtyping.

I think there's a good question as to whether to assume double-splatting in a **kwargs to a function should be considered to fulfill all of the required keyword arguments not otherwise fulfilled, or fulfill none of them. Either way this is the wrong error message 😝 , and it should either provide a similar message for both ok and err or no error for either.

@sixolet
Copy link
Collaborator

sixolet commented Dec 15, 2016

Having thought about it for a minute: I think my favorite argument is that it should fulfill no required keyword arguments, and both ok and err above should produce an error.

The code above would either want a # type: ignore annotation or to be spelled this way:

def ok(**kwargs) -> None:
    spam(x=1, y=kwargs['y'])

def also_ok(**kwargs) -> None:
    spam(y=kwargs['y'], x=1)

@sixolet
Copy link
Collaborator

sixolet commented Dec 15, 2016

(And I'd be happy to take an hour or two and make it so)

@gvanrossum
Copy link
Member

gvanrossum commented Dec 16, 2016 via email

@z33ky
Copy link
Author

z33ky commented Dec 16, 2016

kwargs.update({'x': 1}); spam(**kwargs) would also be valid outside lambdas.

@gvanrossum
Copy link
Member

The issue is that mypy doesn't keep track of the individual key/value pairs in a dict -- in general that won't work (even though there are some examples where it might seem to be trivial). Therefore it has no insight into whether a particular call involving **kwds will or will not satisfy the callee's requirements. I think this is a sufficiently common pattern that it would be annoying to have to flag this each time with a # type: ignore, even though I realize that allowing it to pass without error may cause unsafe code to be missed.

A particularly common use case is a wrapper function. E.g. maybe I have some shortcut for calling logging.error():

def err(*args: Any, **kwds: Any):
    logging.error(*args, **kwds)

This should definitely satisfy the requirements of logging.error(). Now in this particular case it's due to the *args, not due to the **kwds, but I think the same reasoning should apply here. The intention is that err() has the same signature as logging.error() at run time, and we really don't want to have to repeat that signature here (it's pretty complicated).

@sixolet
Copy link
Collaborator

sixolet commented Dec 16, 2016

Yeah, I guess making it so that you can never supply required arguments with *some_list or **some_dict is a bit overzealous in the way people actually write code.

Are we okay with your err example being an idiom that ends up with a function that has a "secret" required argument -- message is required in practice, but not by the signature of err.

Finally, I have a weird related set of corner cases that we should decide how to handle:

def foo(a, b, c, d): pass
lst = [1]
foo(0, *lst, c=2, d=3) # Interpreter is fine with this, but only as long as lst is exactly len 1
foo(0, *lst, c=2) # This can never be ok.
lst = [1, 2, 3]
foo(0, *lst) # This is again fine, but only as long as lst is exactly len 3

@gvanrossum
Copy link
Member

Are we okay with your err example being an idiom that ends up with a function that has a "secret" required argument -- message is required in practice, but not by the signature of err.

Yes, I think that's a case where the *args, **dict just screams "dynamic code, don't bother checking". If someone decides they want the secret arg to become public they can update the signature for err().

For the *lst example, I similarly feel that the call screams "dynamic code here" and we should let it pass, even the one that can never be okay. Mypy doesn't consider the length of a list part of its type (unlike for Tuple!), and I don't think it's worth our time trying to solve the constraints on the list size or the constraints on the rest of the argument list. When there's a problem it would need a really long error message explaining what's wrong and it probably still wouldn't be helpful in most cases. I bet most users couldn't reproduce the rules either, or even determine the outcome in most of these examples without trying them out, or even explain why some of examples work and other don't one we tell them which ones fail or pass...

I could also see rejecting all these examples because there's always a list length that will make it invalid, but that's probably not going to make us any friends either. So we should probably just not check this aspect of such calls.

@charlax
Copy link
Contributor

charlax commented Aug 6, 2018

Jumping in to add one potential use case, perhaps for a plugin.

I just stumbled upon this today. The use case is the following:

  • REST HTTP API
  • I'm using marshmallow to valid the input.
  • I'm then instantiating a dataclass with the validated input dict.

Rough code:

import uuid
from dataclasses import dataclass

from marshmallow import Schema, fields


@dataclass
class Contact:
    id: uuid.UUID
    firstname: str


class ContactCreate(Schema):
    firstname = fields.Str(required=True)


params = ContactCreate().load({'firstname': 'Louis'})
contact = Contact(**params, id=uuid.uuid4())

This gives:

% export MYPYPATH='../.python/stubs' && pipenv run mypy api/test_.py
api/test_.py:18: error: "Contact" gets multiple values for keyword argument "id"

I totally agree with @gvanrossum's rationale above.

Nonetheless, do you think this would a good use case for a mypy plugin here? Could be interesting for validators packages such as marshmallow (note: I'm not affiliated with them) to provide a plugin that provides this extra context and validates the params.

@danielbraun89
Copy link

This issue still persists, should be addressed ASAP

@lvieirajr
Copy link

Any updates on this?
I still see this same error on the most recent version of mypy.

@hauntsaninja
Copy link
Collaborator

I think we actually just fixed this, see #9573

@lvieirajr
Copy link

Any idea when this will be released in a new version?
I have this happening in a couple projects

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants