-
Notifications
You must be signed in to change notification settings - Fork 770
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
Overriding base class constructor when subclassing builtins #4443
Comments
Thanks for the question & offer to help. You are exactly right, and there's even a (years-old) FIXME in our code to make this happen: Line 65 in 52dc139
I think personally I'd like to see a syntax which is as close to Python as possible. Taking the Python equivalent would be def __new__(cls, x, *args, **kwargs):
return super().__new__(cls, *args, **kwargs) My thought is, how close could we get to the Python syntax? I haven't ever loved the #[pymethods]
impl PyDefaultDict {
#[pyo3(name = "__new__", signature = (x, *args, **kwargs)]
fn new<'py>(
py: Python<'py>
cls,
x: i32,
args: &Bound<'_, PyAny>,
kwargs: Option<&Bound<'_, PyAny>>,
) -> Bound<'py, Self> {
super::<Self>()
.new_args(args, kwargs)
.build(Self)
}
} |
(Sorry fat finger error in closing!) |
super::<Self>()
.new_args(args, kwargs)
.build(Self) This is probably not quite the right API, but the main point to make is that what I think we'll have to do is feed the |
@davidhewitt aside, (I was the original question-asker on SO), while this is looked into, are you aware of any workaround for wrapping a base class with extra |
I think this is a case where PyO3 needs improving; our current |
Then what about: // The name `PyNativeInitializer` is already occupied...
pub struct PyNativeArgsInitializer { ... }
impl PyNativeArgsInitializer {
pub fn new0() -> Self { ... }
pub fn new1(args: impl IntoPy<Py<PyTuple>>) { ... }
pub fn new(args: impl IntoPy<Py<PyTuple>>, kwargs: Option<Py<PyDict>>) { ... }
}
pub struct PyNativeNonInitializedMarker<T> { ... }
impl<T: PyClass> PyClass for PyNativeNonInitializedMarker<T> {
type BaseClass = T::BaseClass;
}
pub struct PyNativeMarker<T> { ... }
impl<Native> PyClassInitializer<PyNativeMarker<PyNativeTypeInitializer<Native>>> {
pub fn from_native(init: PyNativeArgsInitializer) -> Self { ... }
} I had to complicate the API a bit to make it working with the existing Usage is the same as with existing PyClassInitializer::from_native(PyNativeArgsInitializer::new0()).add_subclass(MyClass {}).add_subclass(MySecondClass {}) Then we can remove the existing infrastructure that allows you to initialize a native-derived class without |
Interesting idea. Those names remind me a bit of #4413, however in this case I really like the suggestion of using So overall I'm 👍 on |
Why is vectorcall a concern? Can't it work with this API? |
Am I allowed to already include the breaking change (for the next major version) in my PR? This will simplify it considerably. |
I think best to open the pr assuming the breaking change and let's then work out what's best during the review process. I assume vectorcall is a non issue because we're already inside a tp_call handler, which doesn't use vectorcall. |
Hmm... Even after I allow subclasses to configure their superclasses parameters, this issue still isn't fixed. This is because
@davidhewitt, what do you think is the best option? |
Also, I realized that very much the same machinery (or very similar) we use to support subclassing builtins, we can use in the future to support subclassing arbitrary Python classes, which sounds very cool. The design will need to be fleshed out (for example: how do we declare the Python class), but I think it can have great potential. |
It appears there is no way to override the arguments to the base class constructor when subclassing builtin types. It will always be passed the same arguments as
#[new]
.For example:
In Python:
PyClassInitializer
doesn't allow that either.Originally asked on Stack Overflow: https://stackoverflow.com/q/78873598/7884305.
I'm willing to work on this, if I get input on the API.
I envision an API similar to:
I don't know if it is actually possible (I don't know enough the Python C API).
The text was updated successfully, but these errors were encountered: