[ty] Add support for sentinels (PEP 661)#25082
Conversation
# Conflicts: # crates/ty_python_semantic/src/types/infer/builder.rs
JelleZijlstra
requested review from
AlexWaygood,
carljm,
dcreager,
ibraheemdev and
sharkdp
as code owners
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 89.36%. The percentage of expected errors that received a diagnostic held steady at 85.49%. The number of fully passing files held steady at 88/134. |
Memory usage reportMemory usage unchanged ✅ |
|
sharkdp
left a comment
sharkdp
left a comment
There was a problem hiding this comment.
This looks great, thank you very much!
There was a problem hiding this comment.
A few other things that we could test (from reading your PEP, please feel free to leave some of these out if you think they don't make sense):
- That we recognize sentinel objects as being truthy, i.e. that
bool(MISSING)is of typeLiteral[True]. - That accessing
.__name__and.__module__on a sentinel does not lead to an error, and that the type of those attributes is reasonable / "not obviously wrong". - That
sentinelcan not be subclassed.
There was a problem hiding this comment.
Thanks, adding some tests.
That sentinel can not be subclassed
We currently have only typing_extensions.Sentinel, which we didn't make @final. builtins.sentinel will be, though testing that specifically in ty doesn't feel that useful, as it should just listen to the @final in the stub.
There was a problem hiding this comment.
Oh and __name__ does error right now because the attribute isn't there in typeshed for typing_extensions.Sentinel. We'll add it soon. This also seems not critical to test since the behavior is specified by the stub.
| Mapping, | ||
| // typing_extensions | ||
| ExtensionsTypeVar, // must be distinct from typing.TypeVar, backports new features | ||
| Sentinel, |
There was a problem hiding this comment.
It looks like typing_extensions.Sentinel is an alias for builtins.sentinel? Should we add explicit tests to make sure that we also understand the latter, i.e. if someone uses
MISSING = sentinel("MISSING")
def next_value(default: int | MISSING = MISSING):
...
There was a problem hiding this comment.
I think we won't recognise the builtin symbol as existing until we upgrade our vendored version of typeshed in a couple of days
There was a problem hiding this comment.
Thanks. I'm merging this for now, but it would be great to add a few more tests once that lands.
sharkdp
left a comment
sharkdp
left a comment
There was a problem hiding this comment.
Thank you for the updates.
4 participants
Summary
Add support for
typing_extensions.Sentinel(PEP 661). Support for the other variants (builtins.sentinel,typing_extensions.sentinel) can be added later as stubs are updated.See PEP 661 and python/typing#2277
Test Plan
New tests added.