bpo-38008: Move builtin protocol whitelist to mapping instead of list - #15647
Conversation
| 'Hashable', 'Sized', 'Container', 'Collection', 'Reversible', | ||
| 'ContextManager', 'AsyncContextManager'] | ||
| _PROTO_WHITELIST = { | ||
| 'Callable': 'collections.abc', |
There was a problem hiding this comment.
I'd like to write code in this way.
'Callable': collections.abc.__name__,
It can catch typo error.
There was a problem hiding this comment.
Alternatively I would just avoid the repetition by making _PROTO_WHITELIST a dictionary mapping module to set of names in it. So that the code below would be
base.__module__ in _PROTO_WHITELIST and
base.__name__ in _PROTO_WHITELIST[base.__module__]
corona10
left a comment
corona10
left a comment
There was a problem hiding this comment.
Please add news entries by using blurb tool.
https://devguide.python.org/committing/#what-s-new-and-news-entries
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
Thanks for PR!
I have just one suggestion, could you please add some tests? (and a news entry as @corona10 suggested)
| 'Hashable', 'Sized', 'Container', 'Collection', 'Reversible', | ||
| 'ContextManager', 'AsyncContextManager'] | ||
| _PROTO_WHITELIST = { | ||
| 'Callable': 'collections.abc', |
There was a problem hiding this comment.
Alternatively I would just avoid the repetition by making _PROTO_WHITELIST a dictionary mapping module to set of names in it. So that the code below would be
base.__module__ in _PROTO_WHITELIST and
base.__name__ in _PROTO_WHITELIST[base.__module__]
ilevkivskyi
left a comment
ilevkivskyi
left a comment
There was a problem hiding this comment.
Thanks, LGTM!
(Also there is a tool called blurb, so that you don't need to create the news items manually.)
|
Thanks @ilevkivskyi, didn't know about that! |
ilevkivskyi
added
needs backport to 3.8
type-bug
|
Thanks @jivid for the PR, and @ilevkivskyi for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8. |
…pythonGH-15647) Fixes https://bugs.python.org/issue38008 (cherry picked from commit 692a0dc) Co-authored-by: Divij Rajkumar <drajkuma1@gmail.com>
…GH-15647) Fixes https://bugs.python.org/issue38008 (cherry picked from commit 692a0dc) Co-authored-by: Divij Rajkumar <drajkuma1@gmail.com>
and rename _PROTO_WHITELIST to _PROTO_ALLOWLIST taken from python/cpython#15647 and python/cpython#21825
taken from python/cpython#15647 and python/cpython#21823
6 participants
https://bugs.python.org/issue38008
https://bugs.python.org/issue38008