Introduce a helper function to map types to protocols + fix for dict kwargs false positive

[randolf-scholz]

Fixes: #20424

Following my comments in #20416, this PR introduces a new helper function as_type that can be used to determine whether a type can be matched to a certain protocol or not.

As an example application, I fix a bug in is_valid_keyword_var_arg that stems from checking

kwargs <: SupportsKeyAndGetItem[str, Any]

This check is too eager, because SupportsKeyAndGetItem is invariant in the key type, it will produce a false positive when kwargs is for instance dict[Literal["foo", "bar"], int]. The correct test is:

Does there exist T <: str so that kwargs <: SupportsKeyAndGetItem[T, Any]

which can be checked with the new helper function.

Updated tests

I updated testLiteralKwargs to test:

• both good and bad dict argument
• both good and bad Mapping argument
• both good and bad SupportsKeyAndGetitem argument

[randolf-scholz]

had to put these there due to circular import issues

[ilevkivskyi]

This function should definitely not be in this module (ideally we shouldn't even have the existing typeops import below, but that is a separate story).

Also the scope of this function is misleadingly broad. It should probably accept a TypeInfo as target (which probably must be a protocol), and then use fill_typevars(...) as a constraint inference target.

Finally, it is worth doing some performance measurements, we don't want any visible slow-down for something that is only needed for rare edge cases.

[randolf-scholz]

I put it there since the similar map_instance_to_supertype is in that module. Which module would be appropriate?

Regarding the scope, is this due to design philosophy? It seems useful that one could test for instance if something is a Mapping[T, SomeFixedType] for some T <: str.

If we were to use TypeInfo + fill_typevars, how do you suggest passing the necessary extra information that T <: str? I don't really understand what advantage making target a TypeInfo would give here, it seems like it would make things more complicated.

[ilevkivskyi]

If we were to use TypeInfo + fill_typevars, how do you suggest passing the necessary extra information that T <: str?

Very simple: you don't pass it. Say I want to check, can Foo be some kind of Iterable, this functions may say "yes, it is Iterable[str]". Then you decide whether this solution is something that works for you or not.

[sterliakov]

Just FYI, #20435 could also benefit from this helper function to map to Iterable protocol in a sane fashion

[randolf-scholz]

OK, so I renamed the function to solve_as_subtype, dropped the direction argument and moved it to mypy/subtypes.

I ran some minimal benchmarks with hyperfine:

1. hyperfine 'pytest -k kwargs -n 0'
   master: Time (mean ± σ): 5.836 s ± 0.209 s, Range (min … max): 5.558 s … 6.092 s 10 runs
   PR: Time (mean ± σ): 5.732 s ± 0.176 s, Range (min … max): 5.511 s … 5.991 s 10 runs
2. hyperfine 'mypy tmp.py --no-incremental --cache-dir=/dev/null || true' (tmp.py holds testLiteralKwargs` unit test)
   master: Time (mean ± σ): 2.724 s ± 0.040 s, Range (min … max): 2.676 s … 2.788 s 10 runs
   PR: Time (mean ± σ): 2.702 s ± 0.038 s, Range (min … max): 2.646 s … 2.791 s 10 runs
3. hyperfine --warmup 1 'mypy tmp.py --no-incremental --cache-dir=/dev/null || true' (tmp.py holds func(**{'a': 1, 'b': 2}) x 400)
   master: Time (mean ± σ): 2.742 s ± 0.037 s, Range (min … max): 2.679 s … 2.776 s 10 runs
   PR: Time (mean ± σ): 3.009 s ± 0.044 s, Range (min … max): 2.945 s … 3.079 s 10 runs

The first two show no difference (most time likely spent on setup/teardown), the last one show roughly a 10% performance degradation.

[randolf-scholz]

A few more changes:

• added a fast path for dict, test 3 from my previous comment is now comparably fast to master
• added a fast fail path if the the argument is not a SupportsKeyAndGetitem.
• added flavor-check for ParamSpec
• added factorization over union + test (this was producing false positives on master as well: https://mypy-play.net/?mypy=master&python=3.12&gist=5bd4a2ba27f5d5edec09efb4486f65d9)
• fixed confusing error message Argument after ** must be a mapping when the argument is a union of mappings.

[ilevkivskyi]

Here are some more comments.

[ilevkivskyi]

A caller may accidentally pass a type with some unrelated type variables (for example inside a generic function or class). As I said before, target should really be a TypeInfo.

[randolf-scholz]

The applytype functions should check the validity of solution and give an appropriate error (i.e. bad solution vs no solution).

Very simple: you don't pass it. Say I want to check, can Foo be some kind of Iterable, this functions may say "yes, it is Iterable[str]". Then you decide whether this solution is something that works for you or not.

I believe this may be an incorrect procedure in general; it seems to implicitly assume that if S={A[X] <: B[X, Y, Z], X <: Y} is solvable, then a solution can be found by first solving X⁎=solve({A[X] <: B[X, Y, Z]}, X) and then considering X⁎ & Y if necessary.

This seems to work in simple cases, for instance, say we need to solve X <: Y such that list[X] <: Iterable[Z]. The solver will return X⁎ = Z. If Z <: Y, then X⁎=Z is fine, and otherwise X⁎&Y / meet(X⁎, Y) is a solution.

But I believe we need to include the upper bound X<:Y in general to find a correct solution in the first place. Take for instance X <: int and list[X] <: list[str] | list[int]. Without the X <: int constraint, the solver may pick¹ X⁎=str, but neither str not str & int (↯) are a proper solution of the joint constraints {list[X] <: list[str] | list[int], X <: int}. The proper solution is X⁎=int.

The recursive case when B explicitly depends on X may be problematic as well.

Footnotes

1. in fact, mypy seems to eagerly select the constraints here: https://github.com/python/mypy/blob/0cc21d99b8ab0c587fca66b697a50e6b59abf06d/mypy/constraints.py#L399-L408 ↩

[randolf-scholz]

I opened #20516 with a bug report that reproduces this incorrect behavior.

[ilevkivskyi]

That issue has literary nothing to do with the current discussion (it is actually a good illustration of the confusion I mentioned). In those examples you have in #20516, X is not something that needs to be solved for. It is simply an unrelated type like anything else (for example, exactly same problem appears if I have class X(int): ...). In that issue we solve for T in def list[T](...) -> ..., that one doesn't have any non-trivial upper bound.

Anyway, do you really think you can convince someone who has been working on mypy for 10 years that you know better how it should work?

[randolf-scholz]

Anyway, do you really think you can convince someone who has been working on mypy for 10 years that you know better how it should work?

No, but I'd really like to understand why it should work that way, and what is the error in my thought process in #20419 (comment), so I can learn and grow.

[ilevkivskyi]

If we were to use TypeInfo + fill_typevars, how do you suggest passing the necessary extra information that T <: str?

Very simple: you don't pass it. Say I want to check, can Foo be some kind of Iterable, this functions may say "yes, it is Iterable[str]". Then you decide whether this solution is something that works for you or not.

[github-actions]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅