Skip to content

Fix IntoPyArray for arrays of which the starting pointer is not equal… #194

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 10, 2021
Merged

Fix IntoPyArray for arrays of which the starting pointer is not equal… #194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,14 +415,15 @@ impl<T: Element, D: Dimension> PyArray<T, D> {
py: Python<'py>,
dims: ID,
strides: *const npy_intp,
slice: Box<[T]>,
boxed_slice: Box<[T]>,
data_ptr: Option<*const T>,
) -> &'py Self
where
ID: IntoDimension<Dim = D>,
{
let dims = dims.into_dimension();
let container = SliceBox::new(slice);
let data_ptr = container.data.as_ptr();
let container = SliceBox::new(boxed_slice);
let data_ptr = data_ptr.unwrap_or_else(|| container.data.as_ptr());
let cell = pyo3::PyClassInitializer::from(container)
.create_cell(py)
.expect("Object creation failed.");
Expand Down
20 changes: 15 additions & 5 deletions src/convert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl<T: Element> IntoPyArray for Box<[T]> {
fn into_pyarray<'py>(self, py: Python<'py>) -> &'py PyArray<Self::Item, Self::Dim> {
let len = self.len();
let strides = [mem::size_of::<T>() as npy_intp];
unsafe { PyArray::from_boxed_slice(py, [len], strides.as_ptr(), self) }
unsafe { PyArray::from_boxed_slice(py, [len], strides.as_ptr(), self, None) }
}
}

Expand All @@ -59,10 +59,20 @@ where
type Item = A;
type Dim = D;
fn into_pyarray<'py>(self, py: Python<'py>) -> &'py PyArray<Self::Item, Self::Dim> {
let strides = self.npy_strides();
let dim = self.raw_dim();
let boxed = self.into_raw_vec().into_boxed_slice();
unsafe { PyArray::from_boxed_slice(py, dim, strides.as_ptr(), boxed) }
let (strides, dim) = (self.npy_strides(), self.raw_dim());
let orig_ptr = self.as_ptr();
let is_empty_or_size0 = self.is_empty() || std::mem::size_of::<Self>() == 0;
Copy link

@bluss bluss Jul 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kngwyu I think this size_of should be for the element, size_of::<A> (Self will never be size zero).

@jturner314 and me are maintainers of rust-ndarray and don't mind reviewing changes such as these, when we have time. I'm sorry that this use case is a bit tricky, due to missing low-level APIs - talking with us about what you need could be a good way to improve ndarray for the use cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your kind comments. I'll fix this later.

what you need could be a good way to improve ndarray for the use cases.

I guess @jturner314 already opened rust-ndarray/ndarray#994?
What we need is Box<[T]> and the pointer to the first element of the array. The pointer is used as void* data in https://numpy.org/doc/stable/reference/c-api/array.html#c.PyArray_NewFromDescr

let vec = self.into_raw_vec();
let offset = if is_empty_or_size0 {
0
} else {
unsafe { orig_ptr.offset_from(vec.as_ptr()) as usize }
};
let mut boxed_slice = vec.into_boxed_slice();
// data_ptr is not always the pointer to the 1st element.
// See https://github.com/PyO3/rust-numpy/issues/182 for the detail.
let data_ptr = unsafe { boxed_slice.as_mut_ptr().add(offset) };
unsafe { PyArray::from_boxed_slice(py, dim, strides.as_ptr(), boxed_slice, Some(data_ptr)) }
}
}

Expand Down
13 changes: 13 additions & 0 deletions tests/to_py.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,19 @@ fn into_pyarray_cant_resize() {
})
}

/// Check that into_pyarray works for ndarray of which the pointer of the first element is
/// not at the start. See https://github.com/PyO3/rust-numpy/issues/182 for more
#[test]
fn into_pyarray_collapsed() {
let mut arr = Array2::<f64>::from_shape_fn([3, 4], |(i, j)| (i * 10 + j) as f64);
arr.slice_collapse(s![1.., ..]);
let copy = arr.clone();
pyo3::Python::with_gil(|py| {
let py_arr = arr.into_pyarray(py);
assert_eq!(py_arr.readonly().as_array(), copy);
})
}

#[test]
fn forder_to_pyarray() {
pyo3::Python::with_gil(|py| {
Expand Down