BUG: dtype: Fix up the list-based struct dtype string representation

5 files changed, 56 insertions, 21 deletions

diff --git a/numpy/core/src/multiarray/descriptor.c b/numpy/core/src/multiarray/descriptor.c
index b9ee95708..2d3b2dbbc 100644
--- a/numpy/core/src/multiarray/descriptor.c
+++ b/numpy/core/src/multiarray/descriptor.c
@@ -1336,7 +1336,7 @@ static PyMemberDef arraydescr_members[] = {
 static PyObject *
 arraydescr_subdescr_get(PyArray_Descr *self)
 {
-    if (self->subarray == NULL) {
+    if (!PyDataType_HASSUBARRAY(self)) {
         Py_INCREF(Py_None);
         return Py_None;
     }
@@ -1432,7 +1432,7 @@ arraydescr_typename_get(PyArray_Descr *self)
 static PyObject *
 arraydescr_base_get(PyArray_Descr *self)
 {
-    if (self->subarray == NULL) {
+    if (!PyDataType_HASSUBARRAY(self)) {
         Py_INCREF(self);
         return (PyObject *)self;
     }
@@ -1443,7 +1443,7 @@ arraydescr_base_get(PyArray_Descr *self)
 static PyObject *
 arraydescr_shape_get(PyArray_Descr *self)
 {
-    if (self->subarray == NULL) {
+    if (!PyDataType_HASSUBARRAY(self)) {
         return PyTuple_New(0);
     }
     /*TODO
@@ -2214,7 +2214,7 @@ arraydescr_setstate(PyArray_Descr *self, PyObject *args)
         }
 
         self->subarray = PyArray_malloc(sizeof(PyArray_ArrayDescr));
-        if (self->subarray == NULL) {
+        if (!PyDataType_HASSUBARRAY(self)) {
             return PyErr_NoMemory();
         }
         self->subarray->base = (PyArray_Descr *)PyTuple_GET_ITEM(subarray, 0);
@@ -2438,7 +2438,7 @@ PyArray_DescrNewByteorder(PyArray_Descr *self, char newendian)
         Py_DECREF(new->fields);
         new->fields = newfields;
     }
-    if (new->subarray) {
+    if (PyDataType_HASSUBARRAY(new)) {
         Py_DECREF(new->subarray->base);
         new->subarray->base = PyArray_DescrNewByteorder(
                 self->subarray->base, newendian);
@@ -2590,14 +2590,30 @@ arraydescr_struct_list_str(PyArray_Descr *dtype)
             return 0;
         }
         PyUString_ConcatAndDel(&ret, PyUString_FromString("("));
-        PyUString_ConcatAndDel(&ret, PyObject_Repr(key));
-        PyUString_ConcatAndDel(&ret, PyUString_FromString(","));
-        tmp = arraydescr_short_construction_repr(fld_dtype);
-        PyUString_ConcatAndDel(&ret, tmp);
         /* Check for whether to do titles as well */
         if (title != NULL && title != Py_None) {
-            PyUString_ConcatAndDel(&ret, PyUString_FromString(","));
+            PyUString_ConcatAndDel(&ret, PyUString_FromString("("));
             PyUString_ConcatAndDel(&ret, PyObject_Repr(title));
+            PyUString_ConcatAndDel(&ret, PyUString_FromString(", "));
+            PyUString_ConcatAndDel(&ret, PyObject_Repr(key));
+            PyUString_ConcatAndDel(&ret, PyUString_FromString("), "));
+        }
+        else {
+            PyUString_ConcatAndDel(&ret, PyObject_Repr(key));
+            PyUString_ConcatAndDel(&ret, PyUString_FromString(", "));
+        }
+        /* Special case subarray handling here */
+        if (PyDataType_HASSUBARRAY(fld_dtype)) {
+            tmp = arraydescr_short_construction_repr(
+                            fld_dtype->subarray->base);
+            PyUString_ConcatAndDel(&ret, tmp);
+            PyUString_ConcatAndDel(&ret, PyUString_FromString(", "));
+            PyUString_ConcatAndDel(&ret,
+                            PyObject_Str(fld_dtype->subarray->shape));
+        }
+        else {
+            tmp = arraydescr_short_construction_repr(fld_dtype);
+            PyUString_ConcatAndDel(&ret, tmp);
         }
         PyUString_ConcatAndDel(&ret, PyUString_FromString(")"));
         if (i != names_size - 1) {
@@ -2724,7 +2740,7 @@ arraydescr_subarray_str(PyArray_Descr *dtype)
     ret = PyUString_FromString("(");
     p = arraydescr_short_construction_repr(dtype->subarray->base);
     PyUString_ConcatAndDel(&ret, p);
-    PyUString_ConcatAndDel(&ret, PyUString_FromString(","));
+    PyUString_ConcatAndDel(&ret, PyUString_FromString(", "));
     PyUString_ConcatAndDel(&ret, PyObject_Str(dtype->subarray->shape));
     PyUString_ConcatAndDel(&ret, PyUString_FromString(")"));
 
diff --git a/numpy/core/src/multiarray/dtype_transfer.c b/numpy/core/src/multiarray/dtype_transfer.c
index fa5573ad5..02990f193 100644
--- a/numpy/core/src/multiarray/dtype_transfer.c
+++ b/numpy/core/src/multiarray/dtype_transfer.c
@@ -1974,7 +1974,7 @@ get_subarray_transfer_function(int aligned,
     npy_intp src_size = 1, dst_size = 1;
 
     /* Get the subarray shapes and sizes */
-    if (src_dtype->subarray != NULL) {
+    if (PyDataType_HASSUBARRAY(src_dtype)) {
        if (!(PyArray_IntpConverter(src_dtype->subarray->shape,
                                             &src_shape))) {
             PyErr_SetString(PyExc_ValueError,
@@ -1984,7 +1984,7 @@ get_subarray_transfer_function(int aligned,
         src_size = PyArray_MultiplyList(src_shape.ptr, src_shape.len);
         src_dtype = src_dtype->subarray->base;
     }
-    if (dst_dtype->subarray != NULL) {
+    if (PyDataType_HASSUBARRAY(dst_dtype)) {
        if (!(PyArray_IntpConverter(dst_dtype->subarray->shape,
                                             &dst_shape))) {
             if (src_shape.ptr != NULL) {
@@ -2848,7 +2848,7 @@ get_setdstzero_transfer_function(int aligned,
         *out_transferdata = NULL;
     }
     /* If there are subarrays, need to wrap it */
-    else if (dst_dtype->subarray != NULL) {
+    else if (PyDataType_HASSUBARRAY(dst_dtype)) {
         PyArray_Dims dst_shape = {NULL, -1};
         npy_intp dst_size = 1;
         PyArray_StridedTransferFn *contig_stransfer;
@@ -2961,7 +2961,7 @@ get_decsrcref_transfer_function(int aligned,
         return NPY_SUCCEED;
     }
     /* If there are subarrays, need to wrap it */
-    else if (src_dtype->subarray != NULL) {
+    else if (PyDataType_HASSUBARRAY(src_dtype)) {
         PyArray_Dims src_shape = {NULL, -1};
         npy_intp src_size = 1;
         PyArray_StridedTransferFn *stransfer;
@@ -3114,7 +3114,8 @@ PyArray_GetDTypeTransferFunction(int aligned,
     if (src_itemsize == dst_itemsize && src_dtype->kind == dst_dtype->kind &&
                 !PyDataType_HASFIELDS(src_dtype) &&
                 !PyDataType_HASFIELDS(dst_dtype) &&
-                src_dtype->subarray == NULL && dst_dtype->subarray == NULL &&
+                !PyDataType_HASSUBARRAY(src_dtype) &&
+                !PyDataType_HASSUBARRAY(dst_dtype) &&
                 src_type_num != NPY_DATETIME && src_type_num != NPY_TIMEDELTA) {
         /* A custom data type requires that we use its copy/swap */
         if (src_type_num >= NPY_NTYPES || dst_type_num >= NPY_NTYPES) {
@@ -3193,7 +3194,8 @@ PyArray_GetDTypeTransferFunction(int aligned,
     }
 
     /* Handle subarrays */
-    if (src_dtype->subarray != NULL || dst_dtype->subarray != NULL) {
+    if (PyDataType_HASSUBARRAY(src_dtype) ||
+                                PyDataType_HASSUBARRAY(dst_dtype)) {
         return get_subarray_transfer_function(aligned,
                         src_stride, dst_stride,
                         src_dtype, dst_dtype,
diff --git a/numpy/core/src/multiarray/getset.c b/numpy/core/src/multiarray/getset.c
index 7f78b2dd1..e039ed611 100644
--- a/numpy/core/src/multiarray/getset.c
+++ b/numpy/core/src/multiarray/getset.c
@@ -438,7 +438,7 @@ array_descr_set(PyArrayObject *self, PyObject *arg)
 
     if ((newtype->elsize != self->descr->elsize) &&
         (self->nd == 0 || !PyArray_ISONESEGMENT(self) ||
-         newtype->subarray)) {
+         PyDataType_HASSUBARRAY(newtype))) {
         goto fail;
     }
     if (PyArray_ISCONTIGUOUS(self)) {
@@ -474,7 +474,7 @@ array_descr_set(PyArrayObject *self, PyObject *arg)
 
     /* fall through -- adjust type*/
     Py_DECREF(self->descr);
-    if (newtype->subarray) {
+    if (PyDataType_HASSUBARRAY(newtype)) {
         /*
          * create new array object from data and update
          * dimensions, strides and descr from it
diff --git a/numpy/core/src/multiarray/hashdescr.c b/numpy/core/src/multiarray/hashdescr.c
index 60a9a7361..d4dbdf7e8 100644
--- a/numpy/core/src/multiarray/hashdescr.c
+++ b/numpy/core/src/multiarray/hashdescr.c
@@ -55,7 +55,7 @@ static int _is_array_descr_builtin(PyArray_Descr* descr)
         if (descr->fields != NULL && descr->fields != Py_None) {
                 return 0;
         }
-        if (descr->subarray != NULL) {
+        if (PyDataType_HASSUBARRAY(descr)) {
                 return 0;
         }
         return 1;
@@ -223,7 +223,7 @@ static int _array_descr_walk(PyArray_Descr* descr, PyObject *l)
                 return -1;
             }
         }
-        if(descr->subarray != NULL) {
+        if(PyDataType_HASSUBARRAY(descr)) {
             st = _array_descr_walk_subarray(descr->subarray, l);
             if (st) {
                 return -1;
diff --git a/numpy/core/tests/test_regression.py b/numpy/core/tests/test_regression.py
index a4cc2d725..a51f3cee4 100644
--- a/numpy/core/tests/test_regression.py
+++ b/numpy/core/tests/test_regression.py
@@ -864,6 +864,23 @@ class TestRegression(TestCase):
                      "('bottom', [('bleft', ('>f4', (8, 64)), (1,)), "
                      "('bright', '>f4', (8, 36))])]")
 
+        dt = np.dtype({'names': ['r','g','b'], 'formats': ['u1', 'u1', 'u1'],
+                        'offsets': [0, 1, 2],
+                        'titles': ['Red pixel', 'Green pixel', 'Blue pixel']})
+        assert_equal(str(dt),
+                    "[(('Red pixel', 'r'), 'u1'), "
+                    "(('Green pixel', 'g'), 'u1'), "
+                    "(('Blue pixel', 'b'), 'u1')]")
+
+        dt = np.dtype({'names': ['r','b'], 'formats': ['u1', 'u1'],
+                        'offsets': [0, 2],
+                        'titles': ['Red pixel', 'Blue pixel']})
+        assert_equal(str(dt),
+                    "{'names':['r','b'], "
+                    "'formats':['u1','u1'], "
+                    "'offsets':[0,2], "
+                    "'titles':['Red pixel','Blue pixel']}")
+
     def test_nonnative_endian_fill(self, level=rlevel):
         """ Non-native endian arrays were incorrectly filled with scalars before
         r5034.