Spurious error with dynamic keyword arguments

[bdarnell]

Bug Report

The expression datetime.timedelta(**{"seconds": 1.0}) works correctly in python, but mypy reports it as an error if the python-version parameter is 3.6 or greater. (note that the typeshed definition of timedelta.__init__ is conditioned on the python version, which explains why that parameter has this effect)

To Reproduce

test.py:

import datetime

datetime.timedelta(**{"seconds": 1.0})

Run mypy --python-version 3.5 test.py and mypy --python-version 3.6 test.py

Actual Behavior

With --python-version 3.5, there are no errors. With --python-version 3.6, it reports:

test.py:3: error: Argument 1 to "timedelta" has incompatible type "**Dict[str, float]"; expected "int"
Found 1 error in 1 file (checked 1 source file)

Your Environment

• Mypy version used: tested both 0.740 and 0.790
• Mypy command-line flags: --python-version
• Mypy configuration options from mypy.ini (and other config files): none
• Python version used: 3.8
• Operating system and version: linux

Minimal test case: https://repl.it/@bdarnell/mypy-dynamic-keyword-args-bug-report
CI build in which this was initially discovered: https://travis-ci.org/github/tornadoweb/tornado/jobs/740396440

[hauntsaninja]

Thanks for the report! You're right that the Python version difference is due to typeshed. My guess is something in mypy is looking at keyword-only args when it should be looking at presence of argument name to see if something can be passed by keyword.

[hauntsaninja]

@esoma Thanks again for your earlier kwargs fix. Would you be interested in taking a look at this one too?

[esoma]

@hauntsaninja Can do.

[esoma]

Hopefully this is not too rambling.

I have a solution for this that involves only mapping non-TypedDict **kwargs to parameters which have compatible types, but it has a ripple effect that involves #4001, #9007 and #9629.

So my current solution has the following behavior:

class A: pass
class B: pass
class C: pass

ad: Dict[str, A] = {}
bd: Dict[str, B] = {}
cd: Dict[str, C] = {}

def f(a: A = A(), **bs: B): pass
f(**ad) # no error
f(**ad, **bd) # no error
f(a=A(), **ad) # no error
f(a=A(), **ad, **bd) # no error
f(**bd) # no error
f(**cd) # error

(note that in master all these lines will produce the error related to this issue)

In the error case we are saying that we must consider that a non-TypedDict **kwarg may be empty only if it would be able to map to at least 1 formal parameter.

This is contrary to #4001 and #9007 which essentially say that a dict may always be considered empty. This is because:

Case A

In #9629 @JukkaL presented:

def f(**kwargs: int) -> None: pass
d: Dict[str, str] = {} # value should not matter here, ignore it
f(**d) # produces an error, good

We can imagine two scenarios: one where d is empty and one where it isn't. In the empty d scenario there is no error at runtime and in the other there (presumably if our function actually did something) is an error at runtime.

So this seems like a false-positive sometimes. However, we can reason about the user's intentions. It is unlikely d will always be empty (and if it is then why even have it there), so we can discard the empty d scenario and the error then follows. In this case we don't care if d is sometimes empty, just that we assume it won't always be.

Case B

In #9007 we're given an example similar to this:

def f() -> None: pass
d: Dict[str, str] = {} # value should not matter here, ignore it
f(**d) # produces an error, user in #9007 does not want one

@JukkaL seemed to agree that mypy should not error here. However, I think this runs contrary to the reasoning above. d may be empty, but if it is always empty, then it is pointless. I think this error is actually reasonable and helpful.

Case C

This comes to a head in #4001 where we're given the super() example.

class A:
    def __init__(self, x: int = 0, **kwargs):
        super().__init__(**kwargs) # produces an error, but shouldn't

The reasoning here is that **kwargs in the call to super().__init__ may be empty and so we cannot say that it will cause a problem for when it resolves to object.__init__. This is a common and necessary pattern for super() compatible classes. We come to the conclusion that we cannot assume that **kwargs will never be empty. Our reasoning for case A and B seems to be faulty when we look at this case.

But, I think if we take a closer look we'll find that's not the case. The actual problem, to me, seems to be that we cannot statically analyze the call to super().__init__ at this position. We do not know that super().__init__ is object.__init__.

There may be some way to statically say:

# Case C...
A() # no error
A(x=0) # no error
A(y=0) # error

But that's a different and probably very complicated issue.

It may just be that super().__init__ needs to be special cased.

---

So, basically, since mypy's handling of super() is not perfect, my current solution reverts parts of (case B and C produce errors) #4001 and #9007 from #9629's and @momohatt's solution in order to preserve Case A.

tl;dr

• looking for guidance on whether preserving case A is more important than case C
• if special casing super().__init__ to allow for **kwargs that do not map to any formal parameters is acceptable
• wondering if my assessment of case B (that it should produce an error) is correct

[hauntsaninja]

Thanks for the write up!

My personal opinions:

Multiple inheritance is a hard thing for type checkers to understand. I think there are already multiple places where mypy bets that you're not using multiple inheritance in order to give you useful errors most of the time. So I'd be happy to leave the Case C false positive unfixed for now or in general / consider special casing super().__init__, to preserve Case A errors.

I personally agree we should not go out of our way to support passing empty dicts as kwargs in cases where it would only ever work if the dict was empty. That is, I'm happy to compromise on Case B if it gets in the way of providing useful errors. In fact I'd go a little further than your example and say errors in the following cases are valuable:

f(a=A(), **ad)
f(a=A(), **ad, **bd)

But we might be in the minority here (or at least, on the wrong side of the do-ocracy). It looks like mypy changed its behaviour for the *args case in #7392 and consistency between args and kwargs seems like a good thing.

I see momohatt's thumbs-upped your comment, so it seems like if you opened a PR with your current solution we'd be able to discuss things your solution more concretely / peacefully resolve any incompatibilities with #9629 :-) And Jukka can take the final call on how important Case B support is.

@bdarnell
The reason why this has gotten so complicated is that mypy doesn't really look at the specific value you're passing. It sees timedelta(**dict_from_str_to_float) and freaks out because it doesn't know how many things are in the dict and it fears it could end up with you passing a float to fold.

What's worse is that for many kinds of function(**{...}) mypy will infer Dict[str, object], and object is unlikely to make function happy. mypy also can't currently come close to catching things like:

def fn(string: str, integer: int): ...
fn(**{"string": 123, "integer": "asdf"})

It would be really great if we could infer a TypedDict here.

[esoma]

I've posted a PR (#9705). I'm continuing this conversation there, since it's only incidentally related to this issue through implementation.