Introspection: Basic pyclass(extends) support #5331
Conversation
| @@ -184,15 +184,16 @@ macro_rules! pyobject_native_type_core { | |||
| #[doc(hidden)] | |||
| #[macro_export] | |||
| macro_rules! pyobject_subclassable_native_type { | |||
| ($name:ty, $layout:path $(;$generics:ident)*) => { | |||
There was a problem hiding this comment.
The generics field was unused
There was a problem hiding this comment.
I think it might be used downstream in rust-numpy, where $name may be a generic type 👀
There was a problem hiding this comment.
I do not see an occurence of pyobject_subclassable_native_type! in Rust numpy (the macro it uses is pyobject_native_type_named) and the only other macro calling it in PyO3 is impl_native_exception! that does not allow generics. But happy to revert the change if you prefer.
There was a problem hiding this comment.
Given that all of the macros currently have the same generics syntax, I think it would be better to keep this as-is, yes please.
Tpt
force-pushed
the
tpt/introspection-extends
branch
2 times, most recently
from
3f497da to
0a01104
Compare
| @@ -1139,6 +1139,8 @@ pub trait PyClassBaseType: Sized { | |||
| type BaseNativeType; | |||
| type Initializer: PyObjectInit<Self>; | |||
| type PyClassMutability: PyClassMutability; | |||
| /// Fully qualified name of the base class including modules | |||
| const BASE_NAME: &'static str; | |||
There was a problem hiding this comment.
I have not gated this associated constants with the experimental-inspect feature because it's an internal trait often generated using macro_rules!.
I have not reused IntoPyObject::OUTPUT_TYPE because they might be different (typing.Any vs object...)
Happy to rename the constant to something better.
There was a problem hiding this comment.
Is this information suitable to get from PyTypeInfo (which has name + module) ?
There was a problem hiding this comment.
Maybe. I would need to fix the NAME for a lot of types. Currently it's setup by the macro to be the same as the Rust type name. So, for example, <PyDict as PyTypeInfo>::NAME == "PyDict" and not dict. Happy to do it if you want.
There was a problem hiding this comment.
Hmm, that's a good point. I think that's probably just a historical mistake, it also leads to a lot of sad error messages where we leak the PyDict name out to consumers downstream that probably don't need to know about PyO3.
I think doing that as a precursor PR might be desirable? I can try to do so later, perhaps?
There was a problem hiding this comment.
Indeed. Thanks! It will actually be also useful to generate some type stubs. I hope to be able to do it today, feel free to focus on something else.
Tpt
force-pushed
the
tpt/introspection-extends
branch
3 times, most recently
from
ae15fd0 to
057bfa1
Compare
Tpt
force-pushed
the
tpt/introspection-extends
branch
from
057bfa1 to
e5d3ee2
Compare
Introduces PyClassBaseType::BASE_NAME to store the module name + class name of the base type
Tpt
force-pushed
the
tpt/introspection-extends
branch
from
e5d3ee2 to
fb793af
Compare
| @@ -1139,6 +1139,8 @@ pub trait PyClassBaseType: Sized { | |||
| type BaseNativeType; | |||
| type Initializer: PyObjectInit<Self>; | |||
| type PyClassMutability: PyClassMutability; | |||
| /// Fully qualified name of the base class including modules | |||
| const BASE_NAME: &'static str; | |||
There was a problem hiding this comment.
Is this information suitable to get from PyTypeInfo (which has name + module) ?
| @@ -184,15 +184,16 @@ macro_rules! pyobject_native_type_core { | |||
| #[doc(hidden)] | |||
| #[macro_export] | |||
| macro_rules! pyobject_subclassable_native_type { | |||
| ($name:ty, $layout:path $(;$generics:ident)*) => { | |||
There was a problem hiding this comment.
I think it might be used downstream in rust-numpy, where $name may be a generic type 👀
|
I think this is blocked on #5352 ? Setting to draft for the moment. |
Introduces PyClassBaseType::BASE_NAME to store the module name + class name of the base type