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

Conversion of non &[u8] slices as return types #1200

Closed
sebpuetz opened this issue Sep 17, 2020 · 2 comments
Closed

Conversion of non &[u8] slices as return types #1200

sebpuetz opened this issue Sep 17, 2020 · 2 comments

Comments

@sebpuetz
Copy link
Contributor

sebpuetz commented Sep 17, 2020

Currently something like

fn get_special_tokens_mask(&self) -> &[u32] {
    self.get_some_u32slice()
}

fails with the trait pyo3::callback::IntoPyCallbackOutput<_> is not implemented for &[u32]. Which happens because IntoPy<PyObject> is only implemented for Vec<T> where T: IntoPy<PyObject>. It's currently not possible to add an implementation for &[T] where T: IntoPy<PyObject> because there is a specialized implementation for PyBytes: impl<'a> IntoPy<PyObject> for &'a [u8].

I tried switching this implementation to IntoPy<&PyBytes> but so far haven't been succesful because of lifetime mismatches.

It's possible to work around this issue in user code by converting the slice to PyObject through to_object(py), but I think it'd be more ergonomic (and prevent unnecessary copies in user code) if slices could be converted automatically. I've seen this a few times already that Rust functions call to_vec() on a slice in order to get compiling code since Vec<T> has the IntoPy implementation.


A possible approach would be specifying a lifetime on the Python argument in IntoPy. Then we can write the following:

impl<'a, 'b> IntoPy<&'b PyBytes> for &'a [u8] {
    fn into_py(self, py: Python<'b>) -> &'b PyBytes {
        PyBytes::new(py, self)
    }
}

But I'm not sure if this actually works because I'm getting hundreds of compilation errors from the added lifetime ;)

@davidhewitt
Copy link
Member

I've been thinking about this too. I agree it'd be very nice to have full slice support.

I think that it could be hard to achieve this without either waiting for specialization to be stable (I think we might only need min_specialization in this case; not sure), or removing the &[u8] -> PyBytes special case.

impl<'a, 'b> IntoPy<&'b PyBytes> for &'a [u8] won't help, because really the only useful implementation of IntoPy is IntoPy<PyObject> which all our callback.rs utility code uses to convert Rust types to Python.

@davidhewitt
Copy link
Member

This was solved in PyO3 0.23 with IntoPyObject 🎉

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