Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mapping.get overload has covariant type as argument #9555

Closed
randolf-scholz opened this issue Jan 17, 2023 · 4 comments
Closed

Mapping.get overload has covariant type as argument #9555

randolf-scholz opened this issue Jan 17, 2023 · 4 comments

Comments

@randolf-scholz
Copy link
Contributor

randolf-scholz commented Jan 17, 2023

typeshed/stdlib/typing.pyi

Lines 576 to 589 in 2b9f200

class Mapping(Collection[_KT], Generic[_KT, _VT_co]):
# TODO: We wish the key type could also be covariant, but that doesn't work,
# see discussion in https://github.com/python/typing/pull/273.
@abstractmethod
def __getitem__(self, __key: _KT) -> _VT_co: ...
# Mixin methods
@overload
def get(self, __key: _KT) -> _VT_co | None: ...
@overload
def get(self, __key: _KT, default: _VT_co | _T) -> _VT_co | _T: ...
def items(self) -> ItemsView[_KT, _VT_co]: ...
def keys(self) -> KeysView[_KT]: ...
def values(self) -> ValuesView[_VT_co]: ...
def __contains__(self, __o: object) -> bool: ...

Shouldn't the second overload simply be

@overload 
def get(self, __key: _KT, default: _T) -> _VT_co | _T: ... 

Why is there a covariant variable being used as a function input? Is there any case where this simplified signature would be incompatible with the current signature?

@srittau
Copy link
Collaborator

srittau commented Jan 17, 2023

This was previously tried in #2810, but then reverted in #2817, which has extensive discussion about this.

@srittau srittau closed this as not planned Won't fix, can't repro, duplicate, stale Jan 17, 2023
@randolf-scholz
Copy link
Contributor Author

randolf-scholz commented Jan 17, 2023

@srittau Looking at the example given in the PR

m: Mapping[str, List[str]]
x = m.get('x', [])
reveal_type(x)  # list[<nothing>] instead of list[str]

it seems that the issue is that there is a reasonable expectation that for Mapping.get, if _T is a subtype of _VT_co, then the return value should be interpreted as _VT_co. It seems that to do this correctly, some kind of conditional @overload would be required

@overload  # if _T ≤ _VT_co
def get(self, __key: _KT, default: _T) -> _VT_co: ... 
@overload  # else
def get(self, __key: _KT, default: _T) -> _T: ... 

@srittau
Copy link
Collaborator

srittau commented Jan 17, 2023

@randolf-scholz If you want to write an exploratory PR that would certainly be welcome. We have a much better test infrastructure nowadays that allows us to judge the impact of any changes.

@randolf-scholz
Copy link
Contributor Author

@srittau I think with the current typing system it's unfortunately impossible. One would need something like TypeScript's Conditional Types

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants