The type of __default= in dict.pop and dict.get doesn't look right

dict.pop is currently defined as

Lines 1086 to 1089 in 1851423

    
           @overload 
        
           def pop(self, __key: _KT) -> _VT: ... 
        
           @overload 
        
           def pop(self, __key: _KT, __default: _VT | _T) -> _VT | _T: ...

Note the union with two type parameters as a type of __default=. This type is ambiguous if dict.pop is called with a value of type _VT. For example, given

d: dict[str, str]
d.pop("foo")

a type checker can choose any value for _T, since str is a subtype of str | _T regardless of what _T is.

It seems the union was introduced in 2fdcd2e, which unfortunately does not seem to give any reasoning for that. I think we should drop it and use _T instead in both dict.pop and dict.get (and similarly for MutableMapping), i.e.

class dict(Generic[_KT, _VT]):
   # ...
   @overload
   def pop(self, __key: _KT, __default: _T) -> _T: ...

Yes, I think you're right. Feel free to send a PR!

Will do!

I'm a little confused. In your example, why would d.pop("foo") match the def pop(self, __key: _KT, __default: _VT | _T) -> _VT | _T: ... overload (since __default does not have a default value in that overload)?

But I'm fine with anything as long as the following three cases type check:

from typing import assert_type

def f(d: dict[str, int]):
    assert_type(d.pop("key"), int)
    assert_type(d.pop("key", 1), int)
    assert_type(d.pop("key", None), int | None)

Ooops, sorry. I was meaning to write d.pop("foo", "bar"). I've updated the type to be dict[str, str] to minimize the edits to the original post.

I think all three cases should type check with the proposed definition.

Okay, another correction. The right signature for seems to be

class dict(Generic[_KT, _VT]):
   # ...
   @overload
   def pop(self, __key: _KT, __default: _T) -> _VT | _T: ...

because a dict can return a value from an existing entry.

Remove unnecessary union in the default type in .get() and .pop() met…

More precise overloads for get/pop methods

More precise overloads for get/pop methods (#10501)

Navigation Menu

Repository navigation

The type of __default= in dict.pop and dict.get doesn't look right #10293

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	@overload
	def pop(self, __key: _KT) -> _VT: ...
	@overload
	def pop(self, __key: _KT, __default: _VT \| _T) -> _VT \| _T: ...

The type of __default= in dict.pop and dict.get doesn't look right #10293

Description

Activity

JelleZijlstra commented on Jun 9, 2023

superbobry commented on Jun 9, 2023

hauntsaninja commented on Jun 9, 2023

superbobry commented on Jun 9, 2023

superbobry commented on Jun 9, 2023

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions