-
-
Notifications
You must be signed in to change notification settings - Fork 132
Implement sentinels as types, support isinstance and match statments #715
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
base: main
Are you sure you want to change the base?
Conversation
The `repr` parameters position was replaced by `module_name` to conform to PEP 661. Added copy and pickle tests. Updated documentation for Sentinel. `_marker` was defined before `caller` which causes minor issues, resolved by setting its module name manually.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #715 +/- ##
==========================================
- Coverage 97.38% 96.75% -0.63%
==========================================
Files 3 3
Lines 7689 7711 +22
==========================================
- Hits 7488 7461 -27
- Misses 201 250 +49
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Adds support for isinstance and match statements Methods required to make sentinels pretend they are types are no longer necessary `self._name` replaced with `self.__name__` Updated tests and documentation
a2efeb9 to
9dc25bc
Compare
|
This should be in PEP 661 first before we guarantee support for additional behaviors in typing-extensions. |
I guess I got ahead of myself following the PEP 661 discussions. The only additional behavior here is Does this apply to #617 too? Because I thought that was following the specification after I removed all the extraneous changes in the latest push, but I never received feedback on that which is why I made a separate PR for this one because I couldn't tell if anything was wrong with #617 or if simply nobody was free to look at it at the time. |
|
If we implement sentinels as classes, users will surely start relying on this fact. I don't think we should do that unless and until PEP 661 calls for it. Unfortunately PEP 661 stalled out again... I wrote to Tal a few months ago offering to help push it to completion, but he thought he'd be able to work on it. Maybe I should reach out to him again, but I'm more busy again now too. |
Two main things here:
typefunction to dynamically generate new sentinel types._SentinelMetais setup so that sentinels can be used with themselves inisinstancechecks.This new type logic ended up being simpler than the workarounds required to make sentinels pretend they are types so I ended up removing those workarounds. I'm not fully experienced with their usage but they were covered by tests.
This PR is based on #617 so
reprbeing made a keyword parameter is included. With sentinels being types now it's more important to get__module__correct anyways. Pickle is supported and tested.self.__name__takes the place ofself._namenow that sentinels are types.Updated documentation for sentinels, in addition to #617 it now explains
isinstanceandmatchstatements.Improved the error message for calling a sentinel to include the name of the sentinel.
isinstance(MySentinel, typing_extensions.Sentinel)currently returnsFalse, but that can be changed in__instancecheck__if necessary.Relevant documentation:
__instancecheck__typefunctionCloses #720