Skip to content

Return Dict from MappingProxyType.copy() #4510

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

Merged
merged 1 commit into from
Nov 2, 2020
Merged

Return Dict from MappingProxyType.copy() #4510

merged 1 commit into from
Nov 2, 2020

Conversation

Dreamsorcerer
Copy link
Contributor

@Dreamsorcerer Dreamsorcerer commented Sep 1, 2020

Is it OK to change this to a Dict? Not sure why it would return Mapping.
Or is there a way to use a TypeVar here, such that the return type is the same as the type passed in at creation time?

I keep encountering typing errors with code like:

TEMPLATE = MappingProxyType({...})

...

data = TEMPLATE.copy()
data["foo"] = ...  # Error, can't assign to Mapping.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is reasonable. MappingProxy is an implementation detail of class.__dict__ that prevents changes to the class attributes via a backdoor (you must use the front door, i.e. setattr). And MappingProxy.copy() indeed returns a dict. The key here is that the returned object is owned by the caller and can be freely mutated -- returning a Mapping prevents that.

@gvanrossum gvanrossum changed the title Return Dict from copy(). Return Dict from copy() Sep 1, 2020
@gvanrossum gvanrossum changed the title Return Dict from copy() Return Dict from MappingProxyType.copy() Sep 1, 2020
@JelleZijlstra
Copy link
Member

Looking at the C implementation (https://github.com/python/cpython/blob/384621c42f9102e31ba2c47feba144af09c989e5/Objects/descrobject.c#L1114), it just calls .copy() on the underlying mapping. Strictly speaking, then, we should make MappingProxyType generic over its underlying mapping type and return that type. But I'm not sure we can preserve the key and value types correctly in that case, and in practice MappingProxyType usually just maps over a dict, so I'm OK with annotating it as returning a dict.

@Dreamsorcerer
Copy link
Contributor Author

Do I need to nudge someone to merge this? Doesn't seems like there's anyone against it...

@srittau srittau merged commit 57b86e0 into python:master Nov 2, 2020
@srittau
Copy link
Collaborator

srittau commented Nov 2, 2020

Thanks for the nudge, I merged it.

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

Successfully merging this pull request may close these issues.

4 participants