make the default positional-or-keyword in Mapping.get and MutableMapping.pop

[Akuli]

Fixespython/mypy#11831

I don't think it makes sense for default to be positional-only in Mapping.get and MutableMapping.pop, because classes that inherit from them without overriding get or pop (such as os.environ) can be called with a keyword argument default="lol", and people actually want to do that.

[Akuli]

I can't reproduce the mypy_primer errors locally. For example, with black's literals.py, I get:

(env) akuli@akuli-desktop:~/typeshed$ mv .mypy_cache/ /tmp/
(env) akuli@akuli-desktop:~/typeshed$ mypy --custom-typeshed-dir . literals.py
Success: no issues found in 1 source file
(env) akuli@akuli-desktop:~/typeshed$ mypy --version
mypy 0.930

[Akuli]

Nevermind. I think the mypy_primer run for my first broken commit finished after the one for the fixed commit.

[JelleZijlstra]

I don't think this is the right solution. dict is by far the most common MutableMapping, so if you can't do something on dict, you shouldn't be able to do it on MutableMapping. The errors in the mypy issue should instead be fixed with a more precise type for os.environ.

[AlexWaygood]

dict is by far the most common MutableMapping, so if you can't do something on dict, you shouldn't be able to do it on MutableMapping.

I disagree. The whole point of the mixin methods in collections.abc.(Mutable)Mapping is that you don't have to override them if you're creating a custom mapping type; you define the abstract methods, and then the mixin methods come "for free". But if we have these parameters as positional-only in (Mutable)Mapping, we're saying anybody using these classes to create a custom mapping type has to manually override these mixin methods if they want the type-checker to be happy with them passing keyword arguments.

[Akuli]

I wish there was a way to say "subclasses get this, but it's not a part of the interface that the ABC represents".

[Akuli]

I see a couple ways to solve this:

• Just merge the PR as is. Code that does some_mapping_whatsoever.get("foo", default="bar") will pass type checking and fail at runtime. This is a bit incorrect, but consistent with the "we usually prefer false negatives" in CONTRIBUTING.md.
• Add a script, or modify an existing script, to check that every subclass of Mapping or MutableMapping except dict overrides .get(), and every subclass of MutableMapping overrides .pop(). I think checking this in the CI is necessary, as people adding new mapping classes won't be aware of this gotcha. This is a bit overkill, but correct.

[AlexWaygood]

I see a couple ways to solve this

Option 2 works for stubs defined inside the typeshed project, but doesn't work for your Average Joe creating his own (type-checked) custom mapping type for an application script. Joe's going to be pretty confused when he gets spurious "can't pass a keyword argument to .get()" errors, despite these methods being pos-or-keyword in the CPython source code.

I vote for Option 1.

[sobolevn]

It would be just amazing to fix dict (and other) signatures in CPython 😒

[Akuli]

I think CPython developers would say "it has worked for a long time", and do nothing about it. https://bugs.python.org/issue29935

[JelleZijlstra]

Here's another option:

• We make them pos-or-keyword on Mapping, because that's the reality at runtime.
• We override them in dict to make them positional-only, because that's also the reality at runtime. We'll have to add # type: ignore to appease mypy.

[github-actions]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

[Akuli]

I believe that's what this PR does.

[JelleZijlstra]

So it does, I should have checked.