Support packed struct cython.array cast + better dtype error checking

Cython/Compiler/Buffer.py

@@ -713,6 +713,9 @@ def get_type_information_cname(code, dtype, maxdepth=None):
             assert False
 
         rep = str(dtype)
+
+        flags = "0"
+
         if dtype.is_int:
             if dtype.signed == 0:
                 typegroup = 'U'
@@ -724,6 +727,8 @@ def get_type_information_cname(code, dtype, maxdepth=None):
             typegroup = 'R'
         elif dtype.is_struct:
             typegroup = 'S'
+            if dtype.packed:
+                flags = "__PYX_BUF_FLAGS_PACKED_STRUCT"
         elif dtype.is_pyobject:
             typegroup = 'O'
         else:
@@ -735,13 +740,14 @@ def get_type_information_cname(code, dtype, maxdepth=None):
         else:
             is_unsigned = "0"
 
-        typecode.putln(('static __Pyx_TypeInfo %s = { "%s", %s, sizeof(%s), \'%s\', %s };'
+        typecode.putln(('static __Pyx_TypeInfo %s = { "%s", %s, sizeof(%s), \'%s\', %s, %s };'
                         ) % (name,
                              rep,
                              structinfo_name,
                              declcode,
                              typegroup,
                              is_unsigned,
+                             flags,
                         ), safe=True)
     return name
 

Cython/Compiler/ExprNodes.py

@@ -6242,7 +6242,7 @@ class CythonArrayNode(ExprNode):
     Used when a pointer of base_type is cast to a memoryviewslice with that
     base type. i.e.
 
-        <int[::1, :]> p
+        <int[:M:1, :N]> p
 
     creates a fortran-contiguous cython.array.
 
@@ -6302,13 +6302,14 @@ class CythonArrayNode(ExprNode):
         self.operand.analyse_types(env)
         array_dtype = self.base_type_node.base_type_node.analyse(env)
 
+        MemoryView.validate_memslice_dtype(self.pos, array_dtype)
+
         if not self.operand.type.is_ptr:
             return error(self.operand.pos, ERR_NOT_POINTER)
 
         elif not self.operand.type.base_type.same_as(array_dtype):
             return error(self.operand.pos, ERR_BASE_TYPE)
 
-        #self.operand = self.operand.coerce_to(PyrexTypes.c_char_ptr_type, env)
         if not self.operand.is_name:
             self.operand = self.operand.coerce_to_temp(env)
 
@@ -6320,7 +6321,7 @@ class CythonArrayNode(ExprNode):
 
         self.coercion_type = PyrexTypes.MemoryViewSliceType(array_dtype, axes)
         #self.type = py_object_type
-        self.type = env.global_scope().context.cython_scope.lookup("array").type
+        self.type = self.get_cython_array_type(env)
         assert self.type
 
         env.use_utility_code(MemoryView.cython_array_utility_code)
@@ -6332,6 +6333,12 @@ class CythonArrayNode(ExprNode):
 
         self.temp_code = code.funcstate.allocate_temp(self.type, True)
 
+    def infer_type(self, env):
+        return self.get_cython_array_type(env)
+
+    def get_cython_array_type(self, env):
+        return env.global_scope().context.cython_scope.lookup("array").type
+
     def generate_result_code(self, code):
         import Buffer
 

Cython/Compiler/MemoryView.py

@@ -161,6 +161,27 @@ def src_conforms_to_dst(src, dst):
 
     return True
 
+def valid_memslice_dtype(dtype):
+    """
+    Return whether type dtype can be used as the base type of a
+    memoryview slice
+    """
+    if dtype.is_complex and dtype.real_type.is_int:
+        return False
+
+    return (
+        dtype.is_error or
+        dtype.is_ptr or
+        dtype.is_numeric or
+        dtype.is_struct or
+        dtype.is_pyobject or
+        (dtype.is_typedef and valid_memslice_dtype(dtype.typedef_base_type))
+    )
+
+def validate_memslice_dtype(pos, dtype):
+    if not valid_memslice_dtype(dtype):
+        error(pos, "Invalid base type for memoryview slice")
+
 
 class MemoryViewSliceBufferEntry(Buffer.BufferEntry):
     def __init__(self, entry):

Cython/Compiler/Nodes.py

@@ -828,10 +828,8 @@ class MemoryViewSliceTypeNode(CBaseTypeNode):
             self.type = PyrexTypes.ErrorType()
             return self.type
 
+        MemoryView.validate_memslice_dtype(self.pos, base_type)
         self.type = PyrexTypes.MemoryViewSliceType(base_type, axes_specs)
-        if self.type.dtype.is_memoryviewslice:
-            error(self.pos, "Memoryview slices may not be used as the "
-                            "base type for memoryview slices")
 
         self.use_memview_utilities(env)
         return self.type

Cython/Utility/Buffer.c

@@ -42,12 +42,15 @@ static void __Pyx_RaiseBufferFallbackError(void) {
 /* Run-time type information about structs used with buffers */
 struct __Pyx_StructField_;
 
+#define __PYX_BUF_FLAGS_PACKED_STRUCT (1 << 0)
+
 typedef struct {
   const char* name; /* for error messages only */
   struct __Pyx_StructField_* fields;
   size_t size;     /* sizeof(type) */
   char typegroup; /* _R_eal, _C_omplex, Signed _I_nt, _U_nsigned int, _S_truct, _P_ointer, _O_bject */
   char is_unsigned;
+  int flags;
 } __Pyx_TypeInfo;
 
 typedef struct __Pyx_StructField_ {
@@ -544,7 +547,7 @@ static struct __pyx_typeinfo_string __Pyx_TypeInfoToFormat(__Pyx_TypeInfo *type)
         case 'I':
         case 'U':
             if (size == 1)
-                *buf = 'c';
+                *buf = 'b';
             else if (size == 2)
                 *buf = 'h';
             else if (size == 4)
@@ -559,16 +562,15 @@ static struct __pyx_typeinfo_string __Pyx_TypeInfoToFormat(__Pyx_TypeInfo *type)
             *buf = 'P';
             break;
         case 'C':
-          {
-             __Pyx_TypeInfo complex_type = *type;
+         {
+            __Pyx_TypeInfo complex_type = *type;
             complex_type.typegroup = 'R';
             complex_type.size /= 2;
 
             *buf++ = 'Z';
-            /* Note: What about short/int/long complex? Probably not used? */
             *buf = __Pyx_TypeInfoToFormat(&complex_type).string[0];
             break;
-          }
+         }
         case 'R':
             if (size == 4)
                 *buf = 'f';

Cython/Utility/MemoryView.pyx

@@ -28,13 +28,13 @@ cdef class array:
         Py_ssize_t len
         char *format
         int ndim
-        Py_ssize_t *shape
-        Py_ssize_t *strides
+        Py_ssize_t *_shape
+        Py_ssize_t *_strides
         Py_ssize_t itemsize
         unicode mode
         bytes _format
         void (*callback_free_data)(char *data)
-        cdef object _memview
+        # cdef object _memview
 
     def __cinit__(array self, tuple shape, Py_ssize_t itemsize, format,
                   mode=u"c", bint allocate_buffer=True):
@@ -54,12 +54,12 @@ cdef class array:
         self._format = format
         self.format = self._format
 
-        self.shape = <Py_ssize_t *> malloc(sizeof(Py_ssize_t)*self.ndim)
-        self.strides = <Py_ssize_t *> malloc(sizeof(Py_ssize_t)*self.ndim)
+        self._shape = <Py_ssize_t *> malloc(sizeof(Py_ssize_t)*self.ndim)
+        self._strides = <Py_ssize_t *> malloc(sizeof(Py_ssize_t)*self.ndim)
 
-        if not self.shape or not self.strides:
-            free(self.shape)
-            free(self.strides)
+        if not self._shape or not self._strides:
+            free(self._shape)
+            free(self._strides)
             raise MemoryError("unable to allocate shape or strides.")
 
         cdef int idx
@@ -69,20 +69,20 @@ cdef class array:
             if dim <= 0:
                 raise ValueError("Invalid shape.")
 
-            self.shape[idx] = dim
+            self._shape[idx] = dim
             idx += 1
 
         stride = itemsize
         if mode == "fortran":
             idx = 0
             for dim in shape:
-                self.strides[idx] = stride
+                self._strides[idx] = stride
                 stride = stride * dim
                 idx += 1
         elif mode == "c":
             idx = self.ndim-1
             for dim in shape[::-1]:
-                self.strides[idx] = stride
+                self._strides[idx] = stride
                 stride = stride * dim
                 idx -= 1
         else:
@@ -111,8 +111,8 @@ cdef class array:
         info.buf = self.data
         info.len = self.len
         info.ndim = self.ndim
-        info.shape = self.shape
-        info.strides = self.strides
+        info.shape = self._shape
+        info.strides = self._strides
         info.suboffsets = NULL
         info.itemsize = self.itemsize
         info.readonly = 0
@@ -122,6 +122,8 @@ cdef class array:
         else:
             info.format = NULL
 
+        info.obj = self
+
     def __dealloc__(array self):
         if self.callback_free_data != NULL:
             self.callback_free_data(self.data)
@@ -130,13 +132,13 @@ cdef class array:
 
         self.data = NULL
 
-        if self.strides:
-            free(self.strides)
-            self.strides = NULL
+        if self._strides:
+            free(self._strides)
+            self._strides = NULL
 
-        if self.shape:
-            free(self.shape)
-            self.shape = NULL
+        if self._shape:
+            free(self._shape)
+            self._shape = NULL
 
         self.format = NULL
         self.itemsize = 0
@@ -145,11 +147,14 @@ cdef class array:
         @cname('__pyx_cython_array_get_memview')
         def __get__(self):
             # Make this a property as 'data' may be set after instantiation
-            if self._memview is None:
-                flags = PyBUF_ANY_CONTIGUOUS|PyBUF_FORMAT|PyBUF_WRITABLE
-                self._memview = __pyx_memoryview_new(self, flags)
+            #if self._memview is None:
+            #    flags = PyBUF_ANY_CONTIGUOUS|PyBUF_FORMAT|PyBUF_WRITABLE
+            #    self._memview = __pyx_memoryview_new(self, flags)
+
+            #return self._memview
+            flags =  PyBUF_ANY_CONTIGUOUS|PyBUF_FORMAT|PyBUF_WRITABLE
+            return  __pyx_memoryview_new(self, flags)
 
-            return self._memview
 
     def __getattr__(self, attr):
         return getattr(self.memview, attr)
@@ -221,6 +226,15 @@ cdef extern from *:
 
     PyObject *Py_None
 
+    cdef enum:
+        PyBUF_C_CONTIGUOUS,
+        PyBUF_F_CONTIGUOUS,
+        PyBUF_ANY_CONTIGUOUS
+        PyBUF_FORMAT
+        PyBUF_WRITABLE
+        PyBUF_STRIDES
+        PyBUF_INDIRECT
+
 @cname('__pyx_MemviewEnum')
 cdef class Enum(object):
     cdef object name
@@ -348,11 +362,34 @@ cdef class memoryview(object):
             itemp[i] = c
 
     def __getbuffer__(self, Py_buffer *info, int flags):
-        # Note: self.view.obj must not be NULL
-        Py_INCREF(self.view.obj)
-        info[0] = self.view
+        if flags & PyBUF_STRIDES:
+            info.shape = self.view.shape
+        else:
+            info.shape = NULL
+
+        if flags & PyBUF_STRIDES:
+            info.strides = self.view.strides
+        else:
+            info.strides = NULL
+
+        if flags & PyBUF_INDIRECT:
+            info.suboffsets = self.view.suboffsets
+        else:
+            info.suboffsets = NULL
+
+        if flags & PyBUF_FORMAT:
+            info.format = self.view.format
+        else:
+            info.format = NULL
+
+        info.buf = self.view.buf
+        info.ndim = self.view.ndim
+        info.itemsize = self.view.itemsize
+        info.len = self.view.len
+        info.readonly = 0
         info.obj = self
 
+
     # Some properties that have the same sematics as in NumPy
     property T:
         @cname('__pyx_memoryview_transpose')
@@ -374,11 +411,18 @@ cdef class memoryview(object):
     property strides:
         @cname('__pyx_memoryview_get_strides')
         def __get__(self):
+            if self.view.strides == NULL:
+                # Note: we always ask for strides, so if this is not set it's a bug
+                raise ValueError("Buffer view does not expose strides")
+
             return tuple([self.view.strides[i] for i in xrange(self.view.ndim)])
 
     property suboffsets:
         @cname('__pyx_memoryview_get_suboffsets')
         def __get__(self):
+            if self.view.suboffsets == NULL:
+                return [-1] * self.view.ndim
+
             return tuple([self.view.suboffsets[i] for i in xrange(self.view.ndim)])
 
     property ndim:
@@ -409,6 +453,9 @@ cdef class memoryview(object):
 
             return self._size
 
+    # The __array_interface__ is broken as it does not properly convert PEP 3118 format
+    # strings into NumPy typestrs. NumPy 1.5 support the new buffer interface though.
+    '''
     property __array_interface__:
         @cname('__pyx_numpy_array_interface')
         def __get__(self):
@@ -424,10 +471,11 @@ cdef class memoryview(object):
                     data = (PyLong_FromVoidPtr(self.view.buf), False),
                     shape = self.shape,
                     strides = self.strides,
-                    typestr = "%s%d" % (self.format, self.view.itemsize),
+                    typestr = "%s%d" % (self.view.format, self.view.itemsize),
                     version = 3)
 
             return self._array_interface
+    '''
 
     def __len__(self):
         if self.view.ndim >= 1:
@@ -848,12 +896,17 @@ cdef memoryview_copy(memoryview memview):
 cdef extern from *:
     ctypedef struct __Pyx_StructField
 
+    cdef enum:
+        __PYX_BUF_FLAGS_PACKED_STRUCT
+        __PYX_BUF_FLAGS_INTEGER_COMPLEX
+
     ctypedef struct __Pyx_TypeInfo:
       char* name
       __Pyx_StructField* fields
       size_t size
       char typegroup
       char is_unsigned
+      int flags
 
     ctypedef struct __Pyx_StructField:
       __Pyx_TypeInfo* type
@@ -882,6 +935,11 @@ cdef format_from_typeinfo(__Pyx_TypeInfo *type):
     if type.typegroup == 'S':
         assert type.fields != NULL and type.fields.type != NULL
 
+        if type.flags & __PYX_BUF_FLAGS_PACKED_STRUCT:
+            alignment = '^'
+        else:
+            alignment = ''
+
         parts = ["T{"]
         field = type.fields
 
@@ -889,8 +947,7 @@ cdef format_from_typeinfo(__Pyx_TypeInfo *type):
             parts.append(format_from_typeinfo(field.type))
             field += 1
 
-        parts.append("}")
-        result = "".join(parts)
+        result = alignment.join(parts) + '}'
     else:
         fmt = __Pyx_TypeInfoToFormat(type)
         result = fmt.string

tests/run/cythonarray.pyx

@@ -60,7 +60,7 @@ def full_or_strided():
 def dont_allocate_buffer():
     """
     >>> dont_allocate_buffer()
-    callback called 1
+    callback called
     """
     cdef cy.array result = cy.array((10, 10), itemsize=sizeof(int), format='i', allocate_buffer=False)
     assert result.data == NULL

tests/run/cythonarrayutil.pxi

@@ -2,7 +2,7 @@ from libc.stdlib cimport malloc, free
 cimport cython
 
 cdef void callback(char *data):
-    print "callback called %d" % <long> data
+    print "callback called"
 
 def create_array(shape, mode, use_callback=False):
     cdef cython.array result = cython.array(shape, itemsize=sizeof(int),

tests/run/numpy_memoryview.pyx

@@ -9,6 +9,7 @@ cimport numpy as np
 import numpy as np
 
 include "cythonarrayutil.pxi"
+include "mockbuffers.pxi"
 
 ctypedef np.int32_t dtype_t
 
@@ -27,10 +28,23 @@ def ae(*args):
         if x != args[0]:
             raise AssertionError(args)
 
+__test__ = {}
+
+def testcase(f):
+    __test__[f.__name__] = f.__doc__
+    return f
+
+def testcase_numpy_1_5(f):
+    major, minor, *rest = np.__version__.split('.')
+    if (int(major), int(minor)) >= (1, 5):
+        __test__[f.__name__] = f.__doc__
+    return f
+
 #
 ### Test slicing memoryview slices
 #
 
+@testcase
 def test_partial_slicing(array):
     """
     >>> test_partial_slicing(a)
@@ -46,6 +60,7 @@ def test_partial_slicing(array):
     ae(b.strides[0], c.strides[0], obj.strides[0])
     ae(b.strides[1], c.strides[1], obj.strides[1])
 
+@testcase
 def test_ellipsis(array):
     """
     >>> test_ellipsis(a)
@@ -87,7 +102,7 @@ def test_ellipsis(array):
 #
 ### Test slicing memoryview objects
 #
-
+@testcase
 def test_partial_slicing_memoryview(array):
     """
     >>> test_partial_slicing_memoryview(a)
@@ -104,6 +119,7 @@ def test_partial_slicing_memoryview(array):
     ae(b.strides[0], c.strides[0], obj.strides[0])
     ae(b.strides[1], c.strides[1], obj.strides[1])
 
+@testcase
 def test_ellipsis_memoryview(array):
     """
     >>> test_ellipsis_memoryview(a)
@@ -143,6 +159,7 @@ def test_ellipsis_memoryview(array):
     ae(e.shape[0], e_obj.shape[0])
     ae(e.strides[0], e_obj.strides[0])
 
+@testcase
 def test_transpose():
     """
     >>> test_transpose()
@@ -172,26 +189,10 @@ def test_transpose():
 
     print a[3, 2], a.T[2, 3], a_obj[3, 2], a_obj.T[2, 3], numpy_obj[3, 2], numpy_obj.T[2, 3]
 
-def test_coerce_to_numpy():
-    """
-    >>> test_coerce_to_numpy()
-    34
-    34
-    2
-    """
-    cyarray = create_array(shape=(8, 5), mode="c", use_callback=True)
-    numarray = np.asarray(cyarray)
-
-    print cyarray[6, 4]
-    del cyarray
-    print numarray[6, 4]
-
-    numarray[6, 4] = 2
-    print numarray[6, 4]
-
+@testcase
 def test_numpy_like_attributes(cyarray):
     """
-    >>> cyarray = create_array(shape=(8, 5), mode="c", use_callback=True)
+    >>> cyarray = create_array(shape=(8, 5), mode="c")
     >>> test_numpy_like_attributes(cyarray)
     >>> test_numpy_like_attributes(cyarray.memview)
     """
@@ -205,3 +206,164 @@ def test_numpy_like_attributes(cyarray):
 
     cdef int[:, :] mslice = numarray
     assert (<object> mslice).base is numarray
+
+
+ctypedef int td_cy_int
+cdef extern from "bufaccess.h":
+    ctypedef td_cy_int td_h_short # Defined as short, but Cython doesn't know this!
+    ctypedef float td_h_double # Defined as double
+    ctypedef unsigned int td_h_ushort # Defined as unsigned short
+ctypedef td_h_short td_h_cy_short
+
+cdef void dealloc_callback(char *data):
+    print "deallocating..."
+
+def index(cython.array array):
+    array.callback_free_data = dealloc_callback
+    print np.asarray(array)[3, 2]
+
+@testcase_numpy_1_5
+def test_coerce_to_numpy():
+    """
+    Test coercion to NumPy arrays, especially with automatically
+    generated format strings.
+
+    >>> test_coerce_to_numpy()
+    (97, 98, 600L, 700, 800)
+    deallocating...
+    (600, 700)
+    deallocating...
+    ((100, 200), (300, 400), 500)
+    deallocating...
+    (97, 900)
+    deallocating...
+    99
+    deallocating...
+    111
+    deallocating...
+    222
+    deallocating...
+    333
+    deallocating...
+    11.1
+    deallocating...
+    12.2
+    deallocating...
+    13.3
+    deallocating...
+    (14.4+15.5j)
+    deallocating...
+    (16.6+17.7j)
+    deallocating...
+    (18.8+19.9j)
+    deallocating...
+    22
+    deallocating...
+    33.33
+    deallocating...
+    44
+    deallocating...
+    """
+    #
+    ### First set up some C arrays that will be used to hold data
+    #
+    cdef MyStruct mystructs[20]
+    cdef SmallStruct smallstructs[20]
+    cdef NestedStruct nestedstructs[20]
+    cdef PackedStruct packedstructs[20]
+
+    cdef char chars[20]
+    cdef short shorts[20]
+    cdef int ints[20]
+    cdef long long longlongs[20]
+    cdef td_h_short externs[20]
+
+    cdef float floats[20]
+    cdef double doubles[20]
+    cdef long double longdoubles[20]
+
+    cdef float complex floatcomplex[20]
+    cdef double complex doublecomplex[20]
+    cdef long double complex longdoublecomplex[20]
+
+    cdef td_h_short h_shorts[20]
+    cdef td_h_double h_doubles[20]
+    cdef td_h_ushort h_ushorts[20]
+
+    cdef Py_ssize_t idx = 17
+
+    #
+    ### Initialize one element in each array
+    #
+    mystructs[idx] = {
+        'a': 'a',
+        'b': 'b',
+        'c': 600,
+        'd': 700,
+        'e': 800,
+    }
+
+    smallstructs[idx] = { 'a': 600, 'b': 700 }
+
+    nestedstructs[idx] = {
+        'x': { 'a': 100, 'b': 200 },
+        'y': { 'a': 300, 'b': 400 },
+        'z': 500,
+    }
+
+    packedstructs[idx] = { 'a': 'a', 'b': 900 }
+
+    chars[idx] = 99
+    shorts[idx] = 111
+    ints[idx] = 222
+    longlongs[idx] = 333
+    externs[idx] = 444
+
+    floats[idx] = 11.1
+    doubles[idx] = 12.2
+    longdoubles[idx] = 13.3
+
+    floatcomplex[idx] = 14.4 + 15.5j
+    doublecomplex[idx] = 16.6 + 17.7j
+    longdoublecomplex[idx] = 18.8 + 19.9j
+
+    h_shorts[idx] = 22
+    h_doubles[idx] = 33.33
+    h_ushorts[idx] = 44
+
+    #
+    ### Create a NumPy array and see if our element can be correctly retrieved
+    #
+    index(<MyStruct[:4, :5]> <MyStruct *> mystructs)
+    index(<SmallStruct[:4, :5]> <SmallStruct *> smallstructs)
+    index(<NestedStruct[:4, :5]> <NestedStruct *> nestedstructs)
+    index(<PackedStruct[:4, :5]> <PackedStruct *> packedstructs)
+
+    index(<char[:4, :5]> <char *> chars)
+    index(<short[:4, :5]> <short *> shorts)
+    index(<int[:4, :5]> <int *> ints)
+    index(<long long[:4, :5]> <long long *> longlongs)
+
+    index(<float[:4, :5]> <float *> floats)
+    index(<double[:4, :5]> <double *> doubles)
+    index(<long double[:4, :5]> <long double *> longdoubles)
+
+    index(<float complex[:4, :5]> <float complex *> floatcomplex)
+    index(<double complex[:4, :5]> <double complex *> doublecomplex)
+    index(<long double complex[:4, :5]> <long double complex *> longdoublecomplex)
+
+    index(<td_h_short[:4, :5]> <td_h_short *> h_shorts)
+    index(<td_h_double[:4, :5]> <td_h_double *> h_doubles)
+    index(<td_h_ushort[:4, :5]> <td_h_ushort *> h_ushorts)
+
+
+@testcase_numpy_1_5
+def test_memslice_getbuffer():
+    """
+    >>> test_memslice_getbuffer()
+    [[ 0  2  4]
+     [10 12 14]]
+    callback called
+    """
+    cdef int[:, :] array = create_array((4, 5), mode="c", use_callback=True)
+    print np.asarray(array)[::2, ::2]