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

Revisit FromPyObject #426

Closed
kngwyu opened this issue Apr 5, 2019 · 1 comment
Closed

Revisit FromPyObject #426

kngwyu opened this issue Apr 5, 2019 · 1 comment

Comments

@kngwyu
Copy link
Member

kngwyu commented Apr 5, 2019

This is an experimental proposal. Any feedback is welcome, but please consider that it's experimental.

TL; DR

We should have two traits FromPyType and FromPyProto instead of FromPyObject.

The problem of FromPyObject

Currently, we have FromPyObject as a main interface to get a Rust type from a python object.
It is defined as

pub trait FromPyObject<'source>: Sized {
    /// Extracts `Self` from the source `PyObject`.
    fn extract(ob: &'source PyAny) -> PyResult<Self>;
}

It's general since it requires PyAny, but it has a problem: we can't raise compile error using Rust's type system.
E.g.,

let l = PyComplex::from_doubles(py, 3.0, 1.2);
let v: Vec<i32> = FromPyObject::extract(l.as_ref()).unwrap();

this code raises a runtime error.
To prevent this, we should allow just a set of python types to become a rust type.
E.g., we should have

trait FromPyType<T: PyTypeInfo> {
    fn from_py(py: Python, obj: PyRef<T>) -> Self:
}
impl FromPyType<PyComplex> for Complex<f32> {
...
}

The problem of FromPyType

Its problem is we can not use it for conversions using object protocols.
We have some conversion codes using object protocol.
For example, to extract Vec from a Python object, we try either Sequence and Buffer protocol, or only Sequence protocol via specialization, like

impl<'a, T> FromPyObject<'a> for Vec<T>
where
    T: FromPyObject<'a>,
{
    default fn extract(obj: &'a PyAny) -> PyResult<Self> {
        extract_sequence(obj)
    }
}

impl<'source, T> FromPyObject<'source> for Vec<T>
where
    for<'a> T: FromPyObject<'a> + buffer::Element + Copy,
{
    fn extract(obj: &'source PyAny) -> PyResult<Self> {
        // first try buffer protocol
        if let Ok(buf) = buffer::PyBuffer::get(obj.py(), obj) {
            ...
        }
    }
}

(https://github.com/PyO3/pyo3/blob/72003ec37a16634e3fdff1cd7f03ab8814ecde42/src/types/sequence.rs#L237-#L264)
This looks difficult to give an abstraction via type systems.
One solution is to use types corresponding to each protocols.

impl<T> FromPyType<PyBuffer> for Vec<T> {...}
impl<T> FromPyType<PySequence> for Vec<T> {...}

FromPyProto

It's undesirable that we have to convert PyList or so into PySequence when we want to do conversion.
So we can use trait instead of struct PyBuffer or so.

impl<'p, T: PyBufferProtocol<'p>, U> FromPyProto<T> for Vec<U> {...}
impl<'p> PyBufferProtocol<'p> for &'p PyList {...}

But this also needs specialization and... I'm still looking for a better design.

@davidhewitt
Copy link
Member

Given we haven't done this in over four years, I'm going to close this one.

@davidhewitt davidhewitt closed this as not planned Won't fix, can't repro, duplicate, stale Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants