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

Generic lists #903

Closed
skreborn opened this issue May 5, 2020 · 8 comments · Fixed by #934
Closed

Generic lists #903

skreborn opened this issue May 5, 2020 · 8 comments · Fixed by #934

Comments

@skreborn
Copy link

skreborn commented May 5, 2020

❓ Support request

Apologies if this is not the correct forum to post support requests - I couldn't find a more appropriate channel.

ℹ Setup

I have the following struct defined.

#[derive(Clone)]
pub struct Array<T: ToPyObject + IntoPy<PyObject>> {
  item_size: Option<u32>,
  items: Vec<T>,
}

Now, #[pyclass] does not allow for using generics, which is too bad, but I'm sure there's a technical reason behind it. That's quite alright, I suppose - I thought I could simply define another struct that's the Python representation like so.

#[pyclass(name = Array)]
#[derive(Clone)]
pub struct PyArray {
  #[pyo3(get)]
  item_size: Option<u32>,
  #[pyo3(get)]
  items: ?, 
}

Here is my conversion code as of now.

impl<T: ToPyObject + IntoPy<PyObject>> IntoPy<PyObject> for Array<T> {
  fn into_py(self, py: Python) -> PyObject {
    PyArray {
      item_size: self.item_size,
      items: ?,
    }.to_object(py)
  }
}

⚠ Problem

What I cannot figure out, however, is how can I actually have pyo3 convert to and from this representation.

  • What should be the type of PyArray::items?
  • What can I do to convert the generic Vec<T> in my impl block?
@birkenfeld
Copy link
Member

I think items has to be Vec<PyObject>, and you convert by creating a new Vec with something like self.items.map(|v| v.into()).collect().

@skreborn
Copy link
Author

skreborn commented May 5, 2020

Thank you for getting back to me. Unfortunately, Vec<PyObject> does not impl Clone, so neither #[derive(Clone)], nor #[pyclass(name = Array)] deem the struct acceptable.

Py<PyList> is acceptable by #[pyclass(name = Array)] as of the latest commit (I believe), but it still isn't Cloneable. Now, I think I might be able to live without it being so, but I'm still curious if there's a way to work around this issue.

If this is what's currently possible, then all I need is to convert from a Vec<T> to Py<PyList>, which I might or might not be able to achieve with my limited knowledge on the subject, but it should be the least of my worries.

@birkenfeld
Copy link
Member

I don't believe Clone is required for #[pyclass]. At least, I have no problem compiling this:

#[pyclass]
struct Test {
    items: Vec<PyObject>
}

@davidhewitt
Copy link
Member

I don't believe Clone is required for #[pyclass]. At least, I have no problem compiling this:

I think it's required (for the blanket impl) for how #[pyo3(get)] works - so I think this won't compile:

#[pyclass]
struct Test {
    #[pyo3(get)]
    items: Vec<PyObject>
}

I have an idea how to fix this, will raise a PR later...

@birkenfeld
Copy link
Member

Indeed, I didn't think of the getter attribute. For the fix, are you thinking of a ClonePy trait (analogous to FromPy and IntoPy)?

@davidhewitt
Copy link
Member

For the fix, are you thinking of a ClonePy trait (analogous to FromPy and IntoPy)?

I think ClonePy is a bit janky - I think I might be able to go one better and add Clone to PyObject - but it's an experimental idea 😄

@kngwyu
Copy link
Member

kngwyu commented May 6, 2020

We cannot clone(=refcnt += 1) PyObject without py token so ClonePy might be realistic.

But I think what is really useful is zero-copy get for Vec<T>, which can be possible using array/buffer protocol, though we're far from the right design.
Currently, only rust-numpy has zero-copy conversion implementation. I want to migrate this feature into PyO3 itself with a reasonable trait design.

@davidhewitt
Copy link
Member

For clone - see #908 😄

Currently, only rust-numpy has zero-copy conversion implementation. I want to migrate this feature into PyO3 itself with a reasonable trait design.

Would be very happy to see this! In my experience in C++ this used template specialization, so hopefully we can find a way without specialization here.

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

Successfully merging a pull request may close this issue.

4 participants