[red-knot] knot_extensions Python API
#15103
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
| @@ -341,3 +341,4 @@ zipfile._path: 3.12- | |||
| zipimport: 3.0- | |||
| zlib: 3.0- | |||
| zoneinfo: 3.9- | |||
| red_knot: 3.0- | |||
There was a problem hiding this comment.
We would obviously not put this in our vendored copy of typeshed. This was just the easiest option for this PoC.
There was a problem hiding this comment.
I tried to implement this, but it seems a bit involved. I'm opening this PR for review first, so we can discuss if we actually want this, before I invest more time.
There was a problem hiding this comment.
Mypy has a solution for this for mypy_extensions that doesn't involve tinkering with typeshed's VERSIONS file. mypy_extensions exists as a runtime package published to PyPI, but the runtime package isn't py.typed. The type information for the package is published as a separate stubs package, types-mypy-extensions, and the source code for that stubs package is found in the stubs/mypy-extensions directory in typeshed.
The stubs/ directory in typeshed confusingly does not contain all typeshed's stubs. It only contains typeshed's stubs for third-party packages outside the stdlib. Unlike typeshed's standard-library stubs, the stubs for these third-party packages in typeshed are not generally vendored by mypy (though pyright does vendor them), as they're published as standalone packages to PyPI that you can (uv) pip install if you want your type checker to benefit from the type information provided by these packages. However, mypy makes an exception for the mypy_extensions package: it needs these types to always be available, and so it vendors just that package from typeshed's stubs/ directory: https://github.com/python/mypy/tree/master/mypy/typeshed.
Is this relevant for us? Eh, after typing this all out, I'm not sure 🤷 We almost certainly don't want to contribute our stubs for this package to typeshed upstream just yet, so that would mean that we'd be "injecting" these stubs into a stubs/knot_extensions package at typeshed-sync time. This would involve a bunch of special casing in both our module resolver (we'd need to understand the stubs/ directory) and our typeshed-sync GitHub workflow. So a solution where we just apply a patch to typeshed's VERSIONS file at typeshed-sync time might indeed be simplest for now...
There was a problem hiding this comment.
The type information for the package is published as a separate stubs package,
types-mypy-extensions, and the source code for that stubs package is found in thestubs/mypy-extensionsdirectory in typeshed.
Oh, interesting.
Unlike typeshed's standard-library stubs, the stubs for these third-party packages in typeshed are not generally vendored by mypy (though pyright does vendor them), as they're published as standalone packages to PyPI that you can
(uv) pip installif you want your type checker to benefit from the type information provided by these packages. However, mypy makes an exception for themypy_extensionspackage
🙃
Is this relevant for us? Eh, after typing this all out, I'm not sure 🤷
I think so! If there is precedent for having these stubs in typeshed, that seems like a valid (future) solution to me.
We almost certainly don't want to contribute our stubs for this package to typeshed upstream just yet
Of course
This would involve a bunch of special casing in both our module resolver (we'd need to understand the
stubs/directory) and our typeshed-sync GitHub workflow.
Seems like something I could try. Should we wait for the next Red Knot meeting to decide on whether we want this changeset or not?
So a solution where we just apply a patch to typeshed's
VERSIONSfile at typeshed-sync time might indeed be simplest for now...
To be clear, here you are talking about yet another (simpler) solution that wouldn't put red_knot.pyi in stubs/knot_extensions, but in stdlib/, and then apply a patch to stdlib/VERSIONS, in order not having to add special casing logic to the module resolver?
There was a problem hiding this comment.
To be clear, here you are talking about yet another (simpler) solution that wouldn't put
red_knot.pyiinstubs/knot_extensions, but instdlib/, and then apply a patch tostdlib/VERSIONS, in order not having to add special casing logic to the module resolver?
correct!
| class _IsDisjointFrom[S, T]: ... | ||
| class _IsFullyStatic[T]: ... | ||
|
|
||
| def is_equivalent_to[S, T](x: S, y: T) -> _IsEquivalentTo[S, T]: ... |
There was a problem hiding this comment.
These functions could potentially be annotated using https://discuss.python.org/t/pep-747-typeexpr-type-hint-for-a-type-expression/55984 (?)
sharkdp
force-pushed
the
david/type-level-api
branch
from
6cd62cf to
f286bc5
Compare
sharkdp
force-pushed
the
david/type-level-api
branch
from
9795b39 to
9943e95
Compare
| @@ -341,3 +341,4 @@ zipfile._path: 3.12- | |||
| zipimport: 3.0- | |||
| zlib: 3.0- | |||
| zoneinfo: 3.9- | |||
| red_knot: 3.0- | |||
There was a problem hiding this comment.
Mypy has a solution for this for mypy_extensions that doesn't involve tinkering with typeshed's VERSIONS file. mypy_extensions exists as a runtime package published to PyPI, but the runtime package isn't py.typed. The type information for the package is published as a separate stubs package, types-mypy-extensions, and the source code for that stubs package is found in the stubs/mypy-extensions directory in typeshed.
The stubs/ directory in typeshed confusingly does not contain all typeshed's stubs. It only contains typeshed's stubs for third-party packages outside the stdlib. Unlike typeshed's standard-library stubs, the stubs for these third-party packages in typeshed are not generally vendored by mypy (though pyright does vendor them), as they're published as standalone packages to PyPI that you can (uv) pip install if you want your type checker to benefit from the type information provided by these packages. However, mypy makes an exception for the mypy_extensions package: it needs these types to always be available, and so it vendors just that package from typeshed's stubs/ directory: https://github.com/python/mypy/tree/master/mypy/typeshed.
Is this relevant for us? Eh, after typing this all out, I'm not sure 🤷 We almost certainly don't want to contribute our stubs for this package to typeshed upstream just yet, so that would mean that we'd be "injecting" these stubs into a stubs/knot_extensions package at typeshed-sync time. This would involve a bunch of special casing in both our module resolver (we'd need to understand the stubs/ directory) and our typeshed-sync GitHub workflow. So a solution where we just apply a patch to typeshed's VERSIONS file at typeshed-sync time might indeed be simplest for now...
Seeing that mypy and pyre have similar public APIs, I could imagine (part of) this becoming a public API for red knot that isn't feature-gated? The Do we need to decide this right away? If not, can this stay public for now, or should we approach it very carefully and hide everything (by default)? |
| @@ -341,3 +341,4 @@ zipfile._path: 3.12- | |||
| zipimport: 3.0- | |||
| zlib: 3.0- | |||
| zoneinfo: 3.9- | |||
| red_knot: 3.0- | |||
There was a problem hiding this comment.
To be clear, here you are talking about yet another (simpler) solution that wouldn't put
red_knot.pyiinstubs/knot_extensions, but instdlib/, and then apply a patch tostdlib/VERSIONS, in order not having to add special casing logic to the module resolver?
correct!
I'm generally leaning toward keeping the API intentionally limited and only expose features that we want to support long term (and consider part of the public API) because users will using them and it's very easy that we forget to revisit this decision before the release (unless we note it down somewhere?) |
sharkdp
changed the title
knot_extensions Python API
|
|
||
| use std::result::Result; | ||
|
|
||
| #[derive(Debug)] |
There was a problem hiding this comment.
| #[derive(Debug)] | |
| // TODO: redundant once more generalized call-signature checking is implemented? | |
| #[derive(Debug)] |
There was a problem hiding this comment.
Note that this error is also returned when passing a wrong number of arguments to special forms like Not and TypeOf, which will not be handled by call signature checking.
There was a problem hiding this comment.
Oh, I missed that. Hmm, I think I'd probably prefer to emit those errors in the same match as we emit the equivalent errors for all our other special forms here:
ruff/crates/red_knot_python_semantic/src/types/infer.rs
Lines 4963 to 4980 in 706d87f
There was a problem hiding this comment.
My initial idea (and my initial implementation) was trying to be very defensive in the sense that I tried to move the type API "out of the way", with most of the code in a separate module and only very few places where it interacted with existing type inference code. With all of your (great) suggestions, we have now moved to a version that has a tighter coupling to the existing code. This is fine for me, but it might be rather annoying to feature-gate all of this (see @MichaReiser's comment).
I can write separate argument handling logic for TypeOf/Not similar to what we have for Annotated, or try to unite that somehow, but it will make the coupling even stronger (and maybe lead to more code overall?). Does it make sense to wait with this until with have decisions on some higher-level questions (do we want to keep the currently proposed API? do we want to make it public or will all of the code be feature-gated? …)?
| match function.into_function_literal() { | ||
| Some(function) | ||
| if file_to_module(db, function.body_scope(db).file(db)) | ||
| .is_some_and(|module| module.is_known(KnownModule::KnotExtensions)) => | ||
| { | ||
| let function = TypeApiFunction::try_from(function.name(db).as_str()).ok()?; |
There was a problem hiding this comment.
Oh, when I suggested using KnownFunction, I was imagining we'd set the known field on the FunctionType at inference time, like we do for our other KnownFunctions -- something like this (which is admittedly more verbose than what you currently have, but means that the known() and is_known() methods on FunctionType will continue to work as expected for all KnownFunction variants):
diff --git a/crates/red_knot_python_semantic/src/semantic_index/definition.rs b/crates/red_knot_python_semantic/src/semantic_index/definition.rs
index fc75d252d..d8d60d048 100644
--- a/crates/red_knot_python_semantic/src/semantic_index/definition.rs
+++ b/crates/red_knot_python_semantic/src/semantic_index/definition.rs
@@ -74,6 +74,11 @@ impl<'db> Definition<'db> {
Some(KnownModule::Typing | KnownModule::TypingExtensions)
)
}
+
+ pub(crate) fn is_knot_extensions_definition(self, db: &'db dyn Db) -> bool {
+ file_to_module(db, self.file(db))
+ .is_some_and(|module| module.is_known(KnownModule::KnotExtensions))
+ }
}
#[derive(Copy, Clone, Debug)]
diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs
index 64103b7a1..e42b2fa52 100644
--- a/crates/red_knot_python_semantic/src/types.rs
+++ b/crates/red_knot_python_semantic/src/types.rs
@@ -3137,6 +3137,17 @@ impl KnownFunction {
}
}
+ pub fn into_type_api_function(self) -> Option<TypeApiFunction> {
+ match self {
+ Self::TypeApiFunction(function) => Some(function),
+ Self::RevealType
+ | Self::Len
+ | Self::Final
+ | Self::NoTypeCheck
+ | Self::ConstraintFunction(_) => None,
+ }
+ }
+
fn try_from_definition_and_name<'db>(
db: &'db dyn Db,
definition: Definition<'db>,
@@ -3155,6 +3166,31 @@ impl KnownFunction {
"no_type_check" if definition.is_typing_definition(db) => {
Some(KnownFunction::NoTypeCheck)
}
+ "static_assert" if definition.is_knot_extensions_definition(db) => Some(
+ KnownFunction::TypeApiFunction(TypeApiFunction::StaticAssert),
+ ),
+ "is_subtype_of" if definition.is_knot_extensions_definition(db) => {
+ Some(KnownFunction::TypeApiFunction(TypeApiFunction::IsSubtypeOf))
+ }
+ "is_disjoint_from" if definition.is_knot_extensions_definition(db) => {
+ Some(KnownFunction::TypeApiFunction(TypeApiFunction::IsDisjointFrom))
+ }
+ "is_equivalent_to" if definition.is_knot_extensions_definition(db) => Some(
+ KnownFunction::TypeApiFunction(TypeApiFunction::IsEquivalentTo),
+ ),
+ "is_assignable_to" if definition.is_knot_extensions_definition(db) => Some(
+ KnownFunction::TypeApiFunction(TypeApiFunction::IsAssignableTo),
+ ),
+ "is_fully_static" if definition.is_knot_extensions_definition(db) => Some(
+ KnownFunction::TypeApiFunction(TypeApiFunction::IsFullyStatic),
+ ),
+ "is_singleton" if definition.is_knot_extensions_definition(db) => {
+ Some(KnownFunction::TypeApiFunction(TypeApiFunction::IsSingleton))
+ }
+ "is_single_valued" if definition.is_knot_extensions_definition(db) => Some(
+ KnownFunction::TypeApiFunction(TypeApiFunction::IsSingleValued),
+ ),
+
_ => None,
}
}
diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs
index b92722c36..cdea434c0 100644
--- a/crates/red_knot_python_semantic/src/types/infer.rs
+++ b/crates/red_knot_python_semantic/src/types/infer.rs
@@ -69,12 +69,12 @@ use crate::types::{
typing_extensions_symbol, Boundness, CallDunderResult, Class, ClassLiteralType, FunctionType,
InstanceType, IntersectionBuilder, IntersectionType, IterationOutcome, KnownClass,
KnownFunction, KnownInstanceType, MetaclassCandidate, MetaclassErrorKind, SliceLiteralType,
- Symbol, Truthiness, TupleType, Type, TypeAliasType, TypeApiFunction, TypeArrayDisplay,
+ Symbol, Truthiness, TupleType, Type, TypeAliasType, TypeArrayDisplay,
TypeVarBoundOrConstraints, TypeVarInstance, UnionBuilder, UnionType,
};
use crate::unpack::Unpack;
use crate::util::subscript::{PyIndex, PySlice};
-use crate::{Db, KnownModule};
+use crate::Db;
use super::context::{InNoTypeCheck, InferContext, WithDiagnostics};
use super::diagnostic::{
@@ -4234,22 +4234,21 @@ impl<'db> TypeInferenceBuilder<'db> {
) -> Option<Type<'db>> {
let db = self.db();
- match function.into_function_literal() {
- Some(function)
- if file_to_module(db, function.body_scope(db).file(db))
- .is_some_and(|module| module.is_known(KnownModule::KnotExtensions)) =>
- {
- let function = TypeApiFunction::try_from(function.name(db).as_str()).ok()?;
-
+ match function
+ .into_function_literal()
+ .and_then(|function| function.known(db))
+ .and_then(KnownFunction::into_type_api_function)
+ {
+ Some(api_function) => {
let argument_types = arguments.args.iter().map(|arg| {
- if function == TypeApiFunction::StaticAssert {
+ if api_function.is_static_assert() {
self.infer_expression(arg)
} else {
self.infer_type_expression(arg)
}
});
- let result = type_api::resolve_predicate(db, function, argument_types);
+ let result = type_api::resolve_predicate(db, api_function, argument_types);
match result {
Ok(ty) => Some(ty),
diff --git a/crates/red_knot_python_semantic/src/types/type_api.rs b/crates/red_knot_python_semantic/src/types/type_api.rs
index f1d5f945e..72360d8fb 100644
--- a/crates/red_knot_python_semantic/src/types/type_api.rs
+++ b/crates/red_knot_python_semantic/src/types/type_api.rs
@@ -81,21 +81,9 @@ pub enum TypeApiFunction {
IsSingleValued,
}
-impl TryFrom<&str> for TypeApiFunction {
- type Error = ();
-
- fn try_from(value: &str) -> Result<Self, Self::Error> {
- match value {
- "static_assert" => Ok(Self::StaticAssert),
- "is_equivalent_to" => Ok(Self::IsEquivalentTo),
- "is_subtype_of" => Ok(Self::IsSubtypeOf),
- "is_assignable_to" => Ok(Self::IsAssignableTo),
- "is_disjoint_from" => Ok(Self::IsDisjointFrom),
- "is_fully_static" => Ok(Self::IsFullyStatic),
- "is_singleton" => Ok(Self::IsSingleton),
- "is_single_valued" => Ok(Self::IsSingleValued),
- _ => Err(()),
- }
+impl TypeApiFunction {
+ pub const fn is_static_assert(self) -> bool {
+ matches!(self, TypeApiFunction::StaticAssert)
}
}But this would make the problem you pointed out elsewhere even worse -- we'd be integrating the handling of these functions into red-knot in a way such that they'd be harder to feature-gate.
I'm not sure feature-gating is a realistic possibility though... if we want to stop external users from making use of these APIs for now (which I agree is a reasonable idea), I think we might want to do that using a runtime check somehow rather than a compile-time check. While it would be possible to try to squirrel away all the logic in a separate module, I think in the long run it'll be really confusing to have the logic for some special forms in a different place to the logic for all our other special forms. (And same for type-predicate functions -- I don't really want the logic for is_subtype_of to live in a completely different submodule to the logic for assert_type, since they're so similar conceptually!)
Summary
Adds a type-check-time Python API that allows us to create and manipulate types and to test various of their properties. For example, this can be used to write a Markdown test to make sure that
A & Bis a subtype ofAandB, but not of an unrelated classC(something that requires quite a bit more code to do in Rust):I think this functionality is also helpful for interactive debugging sessions, in order to query various properties of Red Knot's type system. Which is something that otherwise requires a custom Rust unit test, some boilerplate code and constant re-compilation.
Comparison Rust ↔ Python:
is_assignable_totestsRust
Python (grouped by topic instead of assignable/not-assignable)
Other ideas
assert_true(…)to replacereveal_type(…) # revealed: Literal[True]knot_extensions.Unknownas a spelling for theUnknowntype.Should we have a way to spell the empty intersection type? LikeIt's probably okay not to support this. EmptyIntersection[()]?Unions orUnions with a single type are not supported either (even if they would have a well defined meaning).typing.assert_type, and maybe alias it asknot_extensions.assert_typeTo do
knot_extensionsmodule in a dedicated packageTypeOfis needed. Maybe introduce something likeknot_extensions.ClassLiteral[…]instead for that specific purpose?Test Plan
New Markdown tests