On 18 September 2012 15:19, Christoph Groth <cwg@falma.de> wrote:

Hi,

Thanks a lot for the recently added generic memoryview support, it's just what we needed to optimize an important part of our program.

However, I believe that there's a problem with Cython's support for objects which do not provide "strides" information (because they are C-contiguous).

The buffer API documentation [1] is a bit ambiguous on this, but it says

"If strides is NULL, the array is interpreted as a standard n-dimensional C-array."

In any case, numpy happily accepts such buffers and computes the "strides" itself.

A thread [2] on Python's devel mailing list also supports the view that setting strides to NULL is OK even if strides were requested.

You might ask: why not simply provide strides when they have been requested? The problem is to find space for them. malloc'ing or PyMem_Alloc'ing memory for strides each time when a buffer is requested is slow, and reserving space for strides within the object is wasteful when "shape" is all that's needed and you have _many_ small objects which expose the buffer interface.

The provided patch works for our use case, but someone with a good understanding of Cython's memoryview support should finish it. I believe the patch will break more complicated buffer use cases.

[1] http://docs.python.org/dev/c-api/buffer.html [2] http://thread.gmane.org/gmane.comp.python.devel/134387

Thank you for considering fixing this issue in Cython,

Christoph

--- Cython/Utility/MemoryView_C.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/Cython/Utility/MemoryView_C.c b/Cython/Utility/MemoryView_C.c index 84e55da..c2524b0 100644 --- a/Cython/Utility/MemoryView_C.c +++ b/Cython/Utility/MemoryView_C.c @@ -207,12 +207,6 @@ static int __Pyx_ValidateAndInit_memviewslice( goto fail; }

- if (!buf->strides) { - PyErr_SetString(PyExc_ValueError, - "buffer does not supply strides necessary for memoryview."); - goto fail; - } - for(i=0; i<ndim; i++) { spec = axes_specs[i];

@@ -320,8 +314,19 @@ __Pyx_init_memviewslice(struct __pyx_memoryview_obj *memview, goto fail; }

+ if (buf->strides) { + for (i = 0; i < ndim; i++) { + memviewslice->strides[i] = buf->strides[i]; + } + } else { + Py_ssize_t stride = buf->itemsize; + for (i = ndim - 1; i >= 0; i--) { + memviewslice->strides[i] = stride; + stride *= buf->shape[i]; + } + } + for (i = 0; i < ndim; i++) { - memviewslice->strides[i] = buf->strides[i]; memviewslice->shape[i] = buf->shape[i]; if (buf->suboffsets) { memviewslice->suboffsets[i] = buf->suboffsets[i]; -- 1.7.10.4

_______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel

Hey Christoph, Thanks for the patch, it looks fine to me. If you want, you could make a pull request and I'll merge it. Mark