python/mypy

Generic function can't make function call with mapping unpacking

NeilGirdhar opened this issue ยท 12 comments

from typing import Any, Optional, TypeVar

_T = TypeVar('_T')

def f(b: bool = True, **kwargs: Any) -> None:
    pass

def g(x: _T) -> _T:
    f(**{'x': x})
    return x

gives

a.py:9: error: Argument 1 to "f" has incompatible type "**Dict[str, _T]"; expected "bool"

Simplified sample:

def f(b: bool = True, **kwargs: str) -> None:
    pass

f(**{'x': 'x'})

This works in runtime, but mypy's output is not in sync. I will try to fix it!
Thanks for the report!

@sobolevn Wow, thanks for simplifying what I thought was a MWE! And double thanks for helping to fix it!

Related #1969

I am confused by this explicit test: https://github.com/python/mypy/blame/master/test-data/unit/check-kwargs.test#L478-L489

Maybe I am missing something ๐Ÿค”

I am confused by this explicit test

Yeah, that test also seems just wrong to me ๐Ÿ˜„ .

Is there any progress on this issue?

PR #11589 by @sobolevn was closed, and @JukkaL suggested making strict **kwargs type checking optional, which is really good idea.

Recently pyright introduced strcit kwargs type checking (microsoft/pyright#5545), and they want to be consistent with other type-checkers, so they're doing it without option to disable it.

Maybe someone have workarounds for this? my best guess is to cast kwargs to Any before unpacking.

Mypy's behavior here with dictionary unpacking is consistent with its behavior with iterable unpacking, so I assume it's intended behavior.

def f1(b: bool = True, **kwargs: str) -> None:
    pass

f1(**{"x": "x"}) # Mypy: incompatible type

def f2(b: bool = True, *args: str) -> None:
    pass

f2(*["a", "b", "c"]) # Mypy: incompatible type

I recently changed pyright's behavior to match mypy's in this regard because it affects overload matching behaviors, and it's important for overload matching to be consistent across type checkers.

If my assumption is incorrect and there is general agreement that this behavior is undesirable, then I'd recommend that it be changed for both dictionary unpacking and iterable unpacking for consistency.

Edit: Actually, the dictionary unpack and iterable unpack cases are not equivalent. In the iterable unpack case, the runtime will always map the first result of the unpacked iterable to parameter b. So ignore what I said above about consistency. There is still a question about whether mypy's current behavior with dictionary unpacking is intended. I assumed it was, so I matched the behavior in pyright, but I've received bug reports from several pyright users complaining about the change. I'm willing to revert the change in pyright if there's agreement that mypy's behavior is not desirable and there are plans to change it accordingly.

@last-partizan, here are a couple of workarounds:

  1. Modify the target function to specify that the first parameter (the one with a default argument) is positional-only:
def f(b: bool = True, /, **kwargs: str) -> None:
    pass
  1. Provide a first argument:
f(True, **{"x": "x"})

@hauntsaninja can we get your opinion on this?

Current behaviour is a bug and should be fixed, or it works like it should?

ambv commented

There's two things here.

First, the Mypy error message is clearly a bug. It claims the first argument to the f() call is the exploded dict expression but that's not true. The keyword arguments get exploded from the provided dict, and in effect, the first argument is actually never provided.

Second, while I can see how we got there, I can't think how the current Mypy behavior can be considered useful. It's not exactly a trivial fix:

  • the literal gets assigned the type dict[str, str] because maybe somewhere along the way we want to mutate it by adding more key-value pairs;
  • the signature is actually a TypedDict(total=False) with the "b" key being boolean and all other keys being strings. We cannot currently map this type exactly to what TypedDict allows us to express (see #6131).

If you specify the kwargs dictionary literal as a TypedDict type, Mypy is happy with the result:

from typing import TypedDict, NotRequired

def f(b: bool = True, **kwargs: str) -> None:
    pass

class FArgs(TypedDict, total=False):
    b: bool
    x: str

some_kwargs: FArgs = {'x': 'x'}

f(**some_kwargs)

So solely on the fact that the variable is later used in dictionary unpacking on a signature, we would have to have Mypy infer {'x': 'x'} being an anonymously built TypedDict(total=False) type with b: bool and other keys being strings. This seems to go contrary to how inference works now, which is "infer early and report incompatible usage as errors".

I don't think it's worth only fixing the direct "dict literal into double star on a function call" as it's pretty artificial. But as soon as we name the dictionary and declare it somewhere else, it's easier to see that inferring its type based on the later double-star unpacking isn't an obvious proposition.

What did pyright do before it was made to behave like Mypy?

What did pyright do before it was made to behave like Mypy?

It's nothing as complex as what you're suggesting above. In the general case, you can't know whether a dict[str, str] will contain a given key. A type checker needs to decide whether it wants to be more conservative (potentially producing a false positive) or less conservative (potentially producing a false negative). Mypy has opted in this case for the former, and a false positive error results. It's easy to switch the assumption to the latter and assume that if a parameter with a default argument is present, it doesn't need to be supplied by an unpacked dict argument.

In most cases, mypy opts for eliminating false positives at the expense of potential false negatives. I think that's a good philosophy in general. For example, neither mypy nor pyright produce an error here even though a runtime error will result.

def f(*, a: str, **kwargs: str) -> None:
    pass

f(**{"x": "x"}) # Runtime error: missing keyword argument 'a'

f(a="", **{"a": "x"}) # Runtime error: multiple values for keyword argument 'a'

So I think there's a good argument to be made that mypy is being inconsistent in opting for the more conservative approach and preferring potential false positives over potential false negatives.

ambv commented

I agree with you in general, but in the example on this issue dict[str, str] -- while it can have more keys -- cannot have a key "b" that holds a boolean value. So the only thing to do is to emit an error unless you do the shenanigans I talk about above.

What do you think about making "immediately unpacked dict", a special case, and to infere it as TypedDict?

In my case, it's syntax sugar for unittest.mock.patch.

from unittest.mock import patch
# Instead of this
m = patch("requests.post")
m.configure_mock(**{"return_value.json.return_value": 1})
# I can write this:
m = patch("requests.post", **{"return_value.json.return_value": 1})

There is no alternative to this syntax, i cannot use keyword arguments, because . cannot be in normal kwargs.

Yes, we can do some special analysis for both cases:

  1. When it is a TypedDict as @last-partizan suggested
  2. When value type part of dict[str, X] does not match some arguments types as @ambv suggested