Revert "Fixing signature for Mapping.get's default parameter"

[ilevkivskyi]

Reverts #2810. cc @mrkmndz

#2810 causes significant troubles in real code. Consider this example:

m: Mapping[str, List[str]]

x = m.get('x', [])

reveal_type(x)

This used to produce builtins.list[builtins.str] but now it produces Union[builtins.list[builtins.str], builtins.list[<nothing>]] causing subsequent failures because of invariance.

The old signature provided type context for inference of the default argument, and was not wrong in any sense.

@mrkmndz Potentially we might need to do the same for werkzeug.datastructures.TypeConversionDict and werkzeug.datastructures.Headers, since this type signature is better for type inference.

[ilevkivskyi]

The tests will likely fail because of an update in mypy tests, I will revert that now, and then re-run the tests here.

[srittau]

Off topic, but this is one example why having a test suite for typeshed is a good idea. This could be used to test and document the reason why certain typings are as they are.

[JelleZijlstra]

I'm OK with reverting. I'd like to hear more from the Pyre team on why they thought this change was necessary though.

[mrkmndz]

The problem with this signature is that it results in a unbound type variable in the return type in the case of the default parameter coinciding with the value type of the mapping. Does mypy just drop elements with unbound type variables from unions in returns?

Arguably the other solution here is to keep my change and to allow literal empty lists to subclass any defined list (which is what pyre does, but currently in too many places) and thus collapse that union.

[ilevkivskyi]

Does mypy just drop elements with unbound type variables from unions in returns?

Mypy infers List[str] for _T from Union[List[str], _T] :> List[str] (if there are no other constraints), we can probably do better, but I don't see how mypy's type inference is related to this. The problem is that with the new signature it is impossible to infer any meaningful type for any empty collection (not just list) used as a default, since there is no type context.

Arguably the other solution here is to keep my change and to allow literal empty lists to subclass any defined list

And how this can be put into the type system? Should we just special-case lists, what to do with user-defined generic collections? TBH, I don't think this is a good idea.

Unless this will cause some immediate troubles for you, I will merge this since it already causes troubles for us, otherwise I think we can wait couple days.

[mrkmndz]

What we're doing is allowing SomeParametric[] to subclass any other SomeParametric, regardless of variance. This is only problematic if you allow that type to exist attached to a variable (which we're banning), but in the context of an argument or return it would be fine. In this case that means that List[] is a subclass of List[str] which collapses the union

[ilevkivskyi]

[...] This is only problematic if you allow that type to exist attached to a variable (which we're banning)

Hm, actually mypy also bans assigning List[<nothing>] to a variable, but this change still looks a bit risky and it is probably better to consider it in calmer conditions. IIUC reverting will not cause immediate troubles for you, so I am merging this now.

[mrkmndz]

Sure, we can work around it, but I think it'd be worth considering allowing that special case exception to subclassing before changing werkzeug as well.

[ilevkivskyi]

but I think it'd be worth considering allowing that special case exception to subclassing before changing werkzeug as well.

OK