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

stdlib/2and3/builtins: change dict.fromkeys to classmethod #3798

Merged
merged 1 commit into from
Mar 2, 2020

Conversation

bluetech
Copy link
Contributor

@bluetech bluetech commented Mar 1, 2020

The referenced issue python/mypy#328 is fixed.

The referenced issue in mypy is fixed.
@bluetech bluetech force-pushed the dict-fromkeys-classmethod branch from 6c55ac5 to 39a6689 Compare March 1, 2020 21:25
@JelleZijlstra
Copy link
Member

What difference does this actually make? dict.fromkeys is a builtin method and isn't an instance of either the classmethod or staticmethod type.

The only observable difference I can think of is that classmethods can return an instance of a subclass if called on a subclass, and that's indeed what happens if you call fromkeys, but the current stubs don't express that behavior anyway.

The docs (https://docs.python.org/3.8/library/stdtypes.html#dict.fromkeys) do call the method a classmethod, so I'm happy to merge this PR as is, but I'm just curious whether this solves some concrete problem.

@bluetech
Copy link
Contributor Author

bluetech commented Mar 2, 2020

@JelleZijlstra This indeed started in an attempt to make the cls generic (#3800).

Since we won't be going this route, I can change this PR to just drop the TODO note instead. Though it does seem better to keep the door open for generic self, so I do slightly prefer to merge as-is.

@JelleZijlstra JelleZijlstra merged commit 36c6f94 into python:master Mar 2, 2020
@JelleZijlstra
Copy link
Member

Sounds good, 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.

2 participants