Conversation
|
Responding to the open questions in JelleZijlstra/cpython#4 (comment)
Agree with this. Some motivation for being subclassable was provided here:
But I'd rather have this customizable (see below) from the constructor instead of using subclassing for this to be achieved.
You gave a good example in https://discuss.python.org/t/9126/306, to be able to re-use sentinels in CPython itself. Do you happen to know if it requires little or some work?
I would be in favor of not defaulting to On the other hand, some users mentioned customizable bool would add complexity 2. I don't think this is really true? It is just a matter of adding a So to me it would make sense to provide a
As per the issue python/cpython#106403 you referenced to me, I've tried using Footnotes
|
| ============= | ||
|
|
||
| A new ``Sentinel`` class will be added to a new ``sentinellib`` module. | ||
| A new built-in callable named ``sentinel`` will be added. |
There was a problem hiding this comment.
What would be the motivation to make it lowercase? Is it to match existing built-ins that are already all lowercase (and perhaps this is even an established convention)?
There was a problem hiding this comment.
Yes, all other builtin types are lowercase (other than exception classes).
| if self._module_name is None: | ||
| self._module_name = __name__ |
There was a problem hiding this comment.
As per JelleZijlstra/cpython#4 (comment), I'm not sure this matches the C implementation, as it defaults to builtins?
There was a problem hiding this comment.
Yeah I think both cases here are questionable, maybe the best bet is to default it to None and make reduce raise if the module is missing.
willingc
left a comment
There was a problem hiding this comment.
The text looks good. Thanks @JelleZijlstra.
|
Thanks! I added a proposed C API. I am going to hold off on changing anything related to |
DanielNoord
left a comment
There was a problem hiding this comment.
Thanks again for picking this up @JelleZijlstra! Hope my review can help with getting this approved :)
I am going to hold off on changing anything related to bool as I hear some decision was made about that offline.
Is there a way to read about this for those not included in that discussion? 😄
I also left one more comment in the PEP thread (link) that I started to wonder about during review.
| used PyPI package. While other module names are possible, making the feature a | ||
| builtin avoids the naming problem entirely. |
There was a problem hiding this comment.
Does it? Doesn't it make the naming problem more prominent as you explain in the Backwards Compatibility section?
There was a problem hiding this comment.
I do think this is accurate. Backwards compatibility is a problem when code that works in 3.x no longer works when run unchanged in 3.x+1. That would be the case if we picked a new library name, because then in the first version that has this PEP, import sentinels (or whatever name we choose) will start importing from the standard library instead of the third-party library. But if we're adding a new builtin, existing code that uses the name sentinel is unaffected; it just can't easily use the new functionality.
I'll share more as soon as I hear more! |
@taleinat gracefully agreed to let me take over the last part of finishing the PEP.
These changes generally align the PEP with what the SC asked for in https://discuss.python.org/t/pep-661-sentinel-values/9126/234 .
📚 Documentation preview 📚: https://pep-previews--4923.org.readthedocs.build/