Skip to content

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

Merged
v923z merged 1 commit intov923z:legacyfrom
jepler:array-memoryview-legacy
Feb 19, 2021
Merged

ndarray: Fix memoryview(ulab.array(...))#329
v923z merged 1 commit intov923z:legacyfrom
jepler:array-memoryview-legacy

Conversation

@jepler
Copy link
Copy Markdown
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 #328.
@jepler jepler requested a review from v923z February 19, 2021 14:30
Comment thread code/ndarray.c
// 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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