Skip to content
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

Creating instances of python sub classes of a Rust struct from factory methods on the struct #4608

Open
Divy1211 opened this issue Oct 10, 2024 · 1 comment

Comments

@Divy1211
Copy link

Divy1211 commented Oct 10, 2024

The Problem

I have a struct in rust that is intended to be sub classed by python classes, and I need to provide factory class methods to this struct. Currently, there seems to be no direct way to instantiate an instance of a sub class with an already existing instance of the Rust struct.

Minimal Example

use pyo3::prelude::*;
use pyo3::types::PyType;
use rand::random;

#[pyclass(subclass)]
pub struct Spam {
    #[pyo3(get)]
    eggs: i32,
}

#[pymethods]
impl Spam {    
    #[classmethod]
    fn mk<'py>(cls: Bound<'py, PyType>) -> Self {
        // Naive attempt to return Spam, considering it may
        // automatically get converted to subtype like `#[new]`
        Spam {
            eggs: random(),
        }
    }
}

and in python:

from test_pyo3 import Spam

class Child(Spam):
    pass

print(Child.mk()) # <builtins.Spam object at 0x74993d11bf10>

The mk function doesn't automatically convert the returned Spam into a Child instance

Current Workaround

The best I've managed to do is the following:

#[pymethods]
impl Spam {
    #[new]
    fn new_py() -> Self {
        Spam {
            eggs: -1,
        }
    }

    #[classmethod]
    fn mk<'py>(cls: Bound<'py, PyType>) -> PyResult<Bound<'py, PyAny>> {
        let spam = Spam {
            eggs: random(),
        };
        
        let sub = cls.call0()?;
        let mut parent_spam = sub.downcast::<Spam>()?.borrow_mut();
        *parent_spam = spam;
        
        Ok(sub)
    }
}
print(Child.mk()) # <__main__.Child object at 0x744d73506d50>

However, this is not ideal, since I had to define a new, and in a real scenario if new is non trivial to call/or is costly, this is not feasible.

Suggestion

Would it be possible to add something like this:

#[pymethods]
impl Spam {    
    #[classmethod]
    fn mk<'py>(cls: Bound<'py, PyType>) -> Bound<'py, PyAny> {
        cls.from_super(Spam {
            eggs: random(),
        })
    }
}

Note: This still keeps it possible to return the base type if so desired, since it doesn't automatically converting a classmethod's return value to the subtype like #[new]

Implementation

I'd be happy to give implementing this a go, but I'm not sure how hard it might be, and may need some guidance/mentoring!

@davidhewitt
Copy link
Member

davidhewitt commented Oct 11, 2024

Great question. Closely related to this is other subclassing issues like #4443, #1647, #1644 and #1637 (comment)

I think the API you propose fits a very relevant need, but I'd slightly prefer if instead of adding one new function a bit ad-hoc we tried to do a more complete review of what we are currently missing and where we don't match Python semantics.

Ideally we can then do a small API refresh to close a bunch of related awkward cases. Maybe targeting 0.24

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants