Skip to content

Make Mapping covariant in key #1114

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

Closed
JukkaL opened this issue Jan 11, 2016 · 7 comments
Closed

Make Mapping covariant in key #1114

JukkaL opened this issue Jan 11, 2016 · 7 comments

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 11, 2016

Mapping should be covariant. See discussion at #1113.

Note that the key type is used as an argument type to __getitem__ and get, so mypy may complain about a covariant key type variable, but we can just ignore that error in the stub, as covariance is safe in this case. Mapping doesn't support mutation operations and __getitem__ can fail even in fully type safe code.

@gvanrossum
Copy link
Member

gvanrossum commented Jan 11, 2016

See also #738. (Which requested covariance in the value -- that's been implemented for a while now.)

@JukkaL
Copy link
Collaborator Author

JukkaL commented Feb 8, 2016

Actually this would not really be safe for key types, as a custom subclass could allow mutation via the Mapping interface, or it could do mutation operations internally as part of caching or similar. One option would be to only treat values covariantly and have keys be invariant -- I think that it would be safe. Another option would be to not worry about the potential unsafety, as this would only affect pretty marginal use cases. As most mapping keys tend to have simple types (not union types, for example) the decision would likely only affect a small fraction of code.

@pkch
Copy link
Contributor

pkch commented Apr 15, 2017

Maybe define a supertype of Mapping that's covariant in its key type (which, of course, doesn't have __getitem__ or __get__ methods)?

typing.Mapping derives from collections.abc.Mapping; we can't easily change the inheritance hierarchy in collections.abc, but if we only need this for typing purposes, I think it's not too hard to leave the runtime-related libraries untouched.

@gvanrossum gvanrossum changed the title Make Mapping covariant Make Mapping covariant in key? Apr 17, 2017
@JukkaL JukkaL added false-positive mypy gave an error on correct code priority-1-normal labels May 18, 2018
@JukkaL JukkaL changed the title Make Mapping covariant in key? Make Mapping covariant in key Jan 28, 2020
@richvdh
Copy link

richvdh commented Sep 23, 2020

see also: python/typing#445

@erictraut
Copy link

Is there anything remaining for this issue? I think it may be OK to close at this point.

@JelleZijlstra
Copy link
Member

Agree, Mapping has to remain invariant in the key type.

@JelleZijlstra JelleZijlstra closed this as not planned Won't fix, can't repro, duplicate, stale Aug 10, 2023
@tgrue-openai
Copy link

I've run into a case where I believe it'd be nice to have a covariant key.

I have a super class with:
get_values() -> Mapping[str | uuid, str]:

and a subclass with:
get_values() -> Mapping[uuid, str]

IIUC, the later is valid under LSP.

It's nice for people directly using the subclass to know they're always going to have UUIDs so they don't have to handle both types.

While perhaps there are reasons we don't want to make Mapping's key covariant, is the conclusion of this bug that we shouldn't be able to express my pattern above? (I see pkch had one suggestion above).

Thank you. (Also apologies if it's bad form to comment on a closed issue, but it felt worse opening a new one.)

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

No branches or pull requests

8 participants