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

ndarray: Fix memoryview(ulab.array(...)) #329

Merged
merged 1 commit into from
Feb 19, 2021

Conversation

jepler
Copy link
Collaborator

@jepler jepler commented Feb 19, 2021

For now this only handles the 1D case. In theory it would work for any dense array, however, I found that ndarray_is_dense didn't behave for me so I implemented this instead.

Add a test. Before the change, this test would segfault.

Closes #328.

For now this only handles the 1D case.  In theory it would work for
any dense array, however, I found that ndarray_is_dense didn't behave
for me so I implemented this instead.

Add a test.  Before the change, this test would segfault.

Closes v923z#328.
@jepler jepler requested a review from v923z February 19, 2021 14:30
// buffer_p.get_buffer() returns zero for success, while mp_get_buffer returns true for success
return !mp_get_buffer(self->array, bufinfo, flags);
if (self->ndim != 1 || self->strides[0] > 1) {
// For now, only allow fetching buffer of a 1d-array
Copy link
Owner

Choose a reason for hiding this comment

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

This should be OK, though, while we can't specify the shape anyway. Once #305 is done, one could initialise from a buffer with shape and strides.

@v923z
Copy link
Owner

v923z commented Feb 19, 2021

@jepler Many thanks! I haven't even had time to look at @dhalbert's bug report, before the fix arrived.

@v923z
Copy link
Owner

v923z commented Feb 19, 2021

however, I found that ndarray_is_dense didn't behave for me so I implemented this instead.

@jepler Do you mean that the function is corrupt? I might have to look into that, then.

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 this pull request may close these issues.

2 participants