Repository navigation
Simplify Placeholder with the new sentinel() class - #161031
rhettinger wants to merge 2 commits into
Conversation
| return 'Placeholder' | ||
|
|
||
| Placeholder = _PlaceholderType() | ||
| Placeholder = sentinel('Placeholder') # _PlaceholderType() |
There was a problem hiding this comment.
I think you want to retain pickability but I'm not sure that way works (I think you need to add the "functools." part for the module).
There was a problem hiding this comment.
Something like this? Placeholder = sentinel('functools.Placeholder', repr='Placeholder')
My understanding from PEP 661 is that this is already handled by the sentinel's __module__ attribute.
There was a problem hiding this comment.
I believe this should be fine, though you can always add a new test.
I made this same change in #149088, along with the corresponding change to the C implementation. I didn't push forward on it because of pushback around hypothetical code relying on type(Placeholder).
There was a problem hiding this comment.
I'm going to revive Jelle's PR which also handles the C implementation.
Minor code simplification courtesy of the new
sentinel()builtin.