pyo3 icon indicating copy to clipboard operation
pyo3 copied to clipboard

Missing create python bounded method from closure

Open victorteokw opened this issue 2 years ago • 3 comments

Thanks for this great package! 👏

I'm implementing dynamic generated heap class with pyo3 in rust for an ORM.

Assign a PythonCFunction to a class won't make it behave like a method. This is different from Python methods which has a __get__ method to bound.

This will not work

let my_method = PyCFunction::new_method_closure(py, cls, Some(function_name), Some("Represent."), |args, kwargs| {
    ...
})?;
my_class.setattr(py, "my_method", my_method)?;

When accessing, the my_method is not bounded. Self is not passed in as the first argument.

There are no args passed

let my_method = PyCFunction::new_method_closure(py, cls, Some(function_name), Some("Represent."), |args, kwargs| {
    println!("see args: {:?} {:?}", args, kwargs) // (,) and None
})?;
my_class.setattr(py, "my_method", my_method)?;

After digging into some details of the implementation, I found that something like this with trampolines need to be implemented

    pub const fn cmethod_function_with_keywords(
        name: &'static str,
        cfunction: PyCMethod,
        doc: &'static str,
    ) -> Self {
        Self {
            ml_name: name,
            ml_meth: PyMethodType::PyCMethod(cfunction),
            ml_flags: ffi::METH_VARARGS | ffi::METH_KEYWORDS | ffi::METH_METHOD,
            ml_doc: doc,
        }
    }

However, I'm not skilled and able to do this. I need your help. 😃

victorteokw avatar Apr 15 '23 11:04 victorteokw

Can I have a little bit more understanding why you can't use the existing proc macros? You say your classes are dynamically generated, can you just create a macro which expands to #[pymethods] and achieve your code generation that way?

If you really want to do this at runtime, you should start from the code we use for wrap_pyfunction and change it to use PyCMethod_New. However, getting all the additional code with method receivers right will probably require a lot of work. I am potentially ok with adding a runtime way to define classes (similar to pybind11) if the code is well factored to implement.

(At the moment we only have the runtime API to initialize modules, so I would like to move the other way and add a declarative API there, see #2367. So maybe we can then split the runtime module / class APIs into a separate feature or crate.)

davidhewitt avatar Apr 18 '23 17:04 davidhewitt

Hi @davidhewitt, thanks for reply.

The way I implement the dynamic class looks like this. I wondered if is there any better ways to do this.

unsafe fn generate_model_class(name: &str, py: Python<'_>) -> PyResult<PyObject> {
    let builtins = py.import("builtins")?;
    let py_type = builtins.getattr("type")?;
    let py_object = builtins.getattr("object")?;
    let dict = PyDict::new(py);
    dict.set_item("__module__", "teo.models")?;
    let init = PyCFunction::new_closure(py, Some("__init__"), Some(INIT_ERROR_MESSAGE), |_args, _kwargs| {
        Err::<(), PyErr>(PyRuntimeError::new_err(INIT_ERROR_MESSAGE))
    })?;
    dict.set_item("__init__", init)?;
    let result = py_type.call1((name, (py_object,), dict))?;
    let copy = result.into_py(py);
    classes_mut().insert(name.to_owned(), copy);
    Ok(result.into_py(py))
}

When add __repr__ method this way, it doesn't function right. Since it's not treated as a bounded function, self is not received. It's not treated as a instance method but a property which is callable.

victorteokw avatar Apr 19 '23 11:04 victorteokw

The classes are generated from different Python user's special string files, these classes don't exist at the compile time.

victorteokw avatar Apr 19 '23 11:04 victorteokw