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

Return mutable array from rust to python #185

Closed
Rikorose opened this issue Jun 10, 2021 · 6 comments · Fixed by #235
Closed

Return mutable array from rust to python #185

Rikorose opened this issue Jun 10, 2021 · 6 comments · Fixed by #235

Comments

@Rikorose
Copy link

It would be nice to have a function like into_pyarray_mut to be able to do the following:

fn some_fn<'py>(py: Python<'py>) -> PyResult<&'py mut PyArray1<f32>> {
    let mut zeros: Array1<f32> = ndarray::Array1::zeros((10));
    Ok(zeros.into_pyarray_mut(py))
}

Is this possible in a safe way?

@kngwyu
Copy link
Member

kngwyu commented Jun 12, 2021

&'py PyArray is actually mutable in Python. We also provide PyReadonlyArray for immutable arrays

@kngwyu kngwyu closed this as completed Jun 12, 2021
@Rikorose
Copy link
Author

Thanks for your answer. Unfortunately, I still struggle to write to an array, which was allocated via rust-numpy. Maybe you can spot a mistake?
Here is my code:

  fn ret_some_array<'py>(&self, py: Python<'py>) -> PyResult<&'py PyArray1<f32>> {
        let v = vec![1f32; 10];
        Ok(v.into_pyarray(py))
    }

On the python side I get the following error:

a = ret_some_array()
print(a.flags)
# a.setflags(write=True)  # -> ValueError: cannot set WRITEABLE flag to True of this array
# a[0] = 0  # -> ValueError: assignment destination is read-only

flags:

a.flags:   C_CONTIGUOUS : True
               F_CONTIGUOUS : True
               OWNDATA : False
               WRITEABLE : False
               ALIGNED : True
               WRITEBACKIFCOPY : False
               UPDATEIFCOPY : False

@asmh1989
Copy link

@Rikorose

#[macro_export]
macro_rules! nparray_return {
    ($s:expr) => {{
        let dot = $s;
        unsafe {
            (*dot.as_array_ptr()).flags |= NPY_ARRAY_WRITEABLE;
        }
        dot
    }};
}

@kngwyu
Copy link
Member

kngwyu commented Jun 24, 2021

@Rikorose
Ah, I understand. The array created by into_pyarray is actually not writable since it can be returned to the Rust world eventually when destructing. I think we need to change the API a bit.

@kngwyu kngwyu reopened this Jun 24, 2021
@kngwyu
Copy link
Member

kngwyu commented Jun 24, 2021

I keep this issue opening until I fix this inconsistent API issue.

Rikorose added a commit to Rikorose/DeepFilterNet that referenced this issue Dec 7, 2021
Rikorose added a commit to Rikorose/DeepFilterNet that referenced this issue Dec 7, 2021
@adamreichold
Copy link
Member

The array created by into_pyarray is actually not writable since it can be returned to the Rust world eventually when destructing. I think we need to change the API a bit.

I don't really follow the reasoning here. The underlying SliceBox is dropped only when there are no more NumPy arrays referencing the data owned by it hence there cannot be any aliasing and we should be safe to set the WRITEABLE flag in Array::from_boxed_slice as we do have ownership at that point.

AnamRasool-pixel pushed a commit to AnamRasool-pixel/Executorch-export-DFN that referenced this issue Sep 9, 2024
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