Refactor Sentinel to conform to PEP 661 #617
Conversation
`repr` parameter removed, explicit repr tests removed `__repr__` modified to match PEP implementation (removed angle brackets) Added `module_name` parameter following PEP implementation and tweaking to use `_caller` helper function. `name` required support for qualified names to follow PEP implementation. Added `__reduce__` to track Sentinel by name and module_name. Added a Sentinel registry to preserve Sentinel identity across multiple calls to the class. Added tests for this. Added an import step to allow forward compatibility with other sentinel libraries. Import step is tested. This was not required by the PEP but it is required for typing_extensions to have a forward compatible type. Added copy and pickle tests. Updated documentation for Sentinel.
|
All commit authors signed the Contributor License Agreement. |
I think breaking changes are expected for draft PEPs, so imo no deprecation process should be used. Users should be aware that changes are expected. However, I think we can:
|
|
cc @taleinat |
|
I'm not willing to remove the |
Then I will change If compatibility is important enough then I can also have the code assume Configuration like |
Adds tests for both the keyword and old positional repr parameters
Changes in Sentinel such as swapping it with a theoretical typing.Sentinel will affect private methods, so the callable used with `__reduce__` must be at the top-level to be stable.
My attempt at implementing PEP 661 since I was unhappy with #594. I'm hoping that this is a PR of decent quality and not just me desperately wanting pickle support.
Changes made to Sentinel:
reprparameter removed, explicit repr tests removed,reprwas rejected by the PEP__repr__modified to match PEP implementation (removed angle brackets)module_nameparameter following PEP implementation and tweaking that to use the local_callerhelper function.namerequired support for qualified names to follow PEP and ensure forward compatibility, this is tested. Unless sentinels in class scopes are forbidden then qualified names are not optional.__reduce__to track Sentinel by name and module_name.This is not from the PEP but it is required for typing_extensions to be forward compatible with any official sentinel type.
For a while I still supported the
reprparameter and after following the PEP 661 discussion I ended up implementing the truthiness tweaks mentioned there, but then I ended up scrapping all of that so that I could follow the PEP more closely, but they would be easy to reintroduce later if desired.If you don't like the current
__reduce__function relying on the class directly then I can modify it to rely on a private classmethod instead which will ensure that any later changes to__new__or__reduce__won't cause issues. That said it seems thename, module_nameparameters are one of the few things most have agreed on.The
_import_sentinelstaticmethod can be moved if desired.I'm unsure of how to mention version changes in the docs. Replacing
reprwithmodule_nameis technically a breaking change._sentinel_registry.setdefaultis potentially supposed to be protected with a lock but I'm unsure of if I can import thethreadingmodule without causing platform issues._sentinel_registrycould be replaced with a weak values dictionary but I'm unsure if that's necessary due to how permanent most sentinels are.