ENH: dtype: Make handling of struct dtype align= flag more rigorous (also fixes ticket #1912)

This adds an 'aligned'= property to the format dict, which gets put
in the str() representation when necessary to preserve it.

3 files changed, 111 insertions, 31 deletions

diff --git a/numpy/core/src/multiarray/descriptor.c b/numpy/core/src/multiarray/descriptor.c
index f9327ee54..5a18fbc0d 100644
--- a/numpy/core/src/multiarray/descriptor.c
+++ b/numpy/core/src/multiarray/descriptor.c
@@ -851,10 +851,29 @@ _convert_from_dict(PyObject *obj, int align)
         || (offsets && (n > PyObject_Length(offsets)))
         || (titles && (n > PyObject_Length(titles)))) {
         PyErr_SetString(PyExc_ValueError,
-                "all items in the dictionary must have the same length.");
+                "'names', 'formats', 'offsets', and 'titles' dicct "
+                "entries must have the same length");
         goto fail;
     }
 
+    /*
+     * If a property 'aligned' is in the dict, it overrides the align flag
+     * to be True if it not already true.
+     */
+    tmp = PyDict_GetItemString(obj, "aligned");
+    if (tmp != NULL) {
+        if (tmp == Py_True) {
+            align = 1;
+        }
+        else if (tmp != Py_False) {
+            PyErr_SetString(PyExc_ValueError,
+                    "NumPy dtype descriptor includes 'aligned' entry, "
+                    "but its value is neither True nor False");
+            Py_DECREF(new);
+            return NULL;
+        }
+    }
+
     totalsize = 0;
     for (i = 0; i < n; i++) {
         PyObject *tup, *descr, *index, *title, *name, *off;
@@ -2748,14 +2767,14 @@ arraydescr_struct_list_str(PyArray_Descr *dtype)
         /* Special case subarray handling here */
         if (PyDataType_HASSUBARRAY(fld_dtype)) {
             tmp = arraydescr_short_construction_repr(
-                            fld_dtype->subarray->base);
+                            fld_dtype->subarray->base, 0);
             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);
+            tmp = arraydescr_short_construction_repr(fld_dtype, 0);
             PyUString_ConcatAndDel(&ret, tmp);
         }
         PyUString_ConcatAndDel(&ret, PyUString_FromString(")"));
@@ -2773,7 +2792,7 @@ arraydescr_struct_list_str(PyArray_Descr *dtype)
  * in a dict format.
  */
 static PyObject *
-arraydescr_struct_dict_str(PyArray_Descr *dtype)
+arraydescr_struct_dict_str(PyArray_Descr *dtype, int includealignedflag)
 {
     PyObject *names, *key, *fields, *ret, *tmp, *tup, *title;
     Py_ssize_t i, names_size;
@@ -2813,7 +2832,7 @@ arraydescr_struct_dict_str(PyArray_Descr *dtype)
         if (title != NULL && title != Py_None) {
             has_titles = 1;
         }
-        tmp = arraydescr_short_construction_repr(fld_dtype);
+        tmp = arraydescr_short_construction_repr(fld_dtype, 0);
         PyUString_ConcatAndDel(&ret, tmp);
         if (i != names_size - 1) {
             PyUString_ConcatAndDel(&ret, PyUString_FromString(","));
@@ -2857,22 +2876,36 @@ arraydescr_struct_dict_str(PyArray_Descr *dtype)
             }
         }
     }
-    /* Finally, the itemsize */
-    PyUString_ConcatAndDel(&ret,
-            PyUString_FromFormat("], 'itemsize':%d}", (int)dtype->elsize));
+    if (includealignedflag && (dtype->flags&NPY_ALIGNED_STRUCT)) {
+        /* Finally, the itemsize/itemsize and aligned flag */
+        PyUString_ConcatAndDel(&ret,
+                PyUString_FromFormat("], 'itemsize':%d, 'aligned':True}",
+                        (int)dtype->elsize));
+    }
+    else {
+        /* Finally, the itemsize/itemsize*/
+        PyUString_ConcatAndDel(&ret,
+                PyUString_FromFormat("], 'itemsize':%d}", (int)dtype->elsize));
+    }
 
     return ret;
 }
 
 /* Produces a string representation for a structured dtype */
 static PyObject *
-arraydescr_struct_str(PyArray_Descr *dtype)
+arraydescr_struct_str(PyArray_Descr *dtype, int includealignflag)
 {
-    if (is_dtype_struct_simple_unaligned_layout(dtype)) {
+    /*
+     * The list str representation can't include the 'align=' flag,
+     * so if it is requested and the struct has the aligned flag set,
+     * we must use the dict str instead.
+     */
+    if (!(includealignflag && (dtype->flags&NPY_ALIGNED_STRUCT)) &&
+                        is_dtype_struct_simple_unaligned_layout(dtype)) {
         return arraydescr_struct_list_str(dtype);
     }
     else {
-        return arraydescr_struct_dict_str(dtype);
+        return arraydescr_struct_dict_str(dtype, includealignflag);
     }
 }
 
@@ -2883,7 +2916,7 @@ arraydescr_subarray_str(PyArray_Descr *dtype)
     PyObject *p, *ret;
 
     ret = PyUString_FromString("(");
-    p = arraydescr_short_construction_repr(dtype->subarray->base);
+    p = arraydescr_short_construction_repr(dtype->subarray->base, 0);
     PyUString_ConcatAndDel(&ret, p);
     PyUString_ConcatAndDel(&ret, PyUString_FromString(", "));
     PyUString_ConcatAndDel(&ret, PyObject_Str(dtype->subarray->shape));
@@ -2898,7 +2931,7 @@ arraydescr_str(PyArray_Descr *dtype)
     PyObject *sub;
 
     if (PyDataType_HASFIELDS(dtype)) {
-        sub = arraydescr_struct_str(dtype);
+        sub = arraydescr_struct_str(dtype, 1);
     }
     else if (PyDataType_HASSUBARRAY(dtype)) {
         sub = arraydescr_subarray_str(dtype);
@@ -2921,7 +2954,7 @@ arraydescr_struct_repr(PyArray_Descr *dtype)
     PyObject *sub, *s;
 
     s = PyUString_FromString("dtype(");
-    sub = arraydescr_struct_str(dtype);
+    sub = arraydescr_struct_str(dtype, 0);
     if (sub == NULL) {
         return NULL;
     }
@@ -2944,20 +2977,26 @@ arraydescr_struct_repr(PyArray_Descr *dtype)
  * additional constructor parameters are given, will reproduce
  * the exact memory layout.
  *
- * This does not preserve the 'align=True' parameter or sticky
- * NPY_ALIGNED_STRUCT flag for struct arrays like the regular
- * repr does, because the 'align' flag is not part of first
- * dtype constructor parameter.
+ * If 'includealignflag' is true, this includes the 'align=True' parameter
+ * inside the struct dtype construction dict when needed. Use this flag
+ * if you want a proper repr string without the 'dtype()' part around it.
+ *
+ * If 'includealignflag' is false, this does not preserve the
+ * 'align=True' parameter or sticky NPY_ALIGNED_STRUCT flag for
+ * struct arrays like the regular repr does, because the 'align'
+ * flag is not part of first dtype constructor parameter. This
+ * mode is intended for a full 'repr', where the 'align=True' is
+ * provided as the second parameter.
  */
 NPY_NO_EXPORT PyObject *
-arraydescr_short_construction_repr(PyArray_Descr *dtype)
+arraydescr_short_construction_repr(PyArray_Descr *dtype, int includealignflag)
 {
     PyObject *ret;
     PyArray_DatetimeMetaData *meta;
     char byteorder[2];
 
     if (PyDataType_HASFIELDS(dtype)) {
-        return arraydescr_struct_str(dtype);
+        return arraydescr_struct_str(dtype, includealignflag);
     }
     else if (PyDataType_HASSUBARRAY(dtype)) {
         return arraydescr_subarray_str(dtype);
@@ -3037,16 +3076,20 @@ arraydescr_short_construction_repr(PyArray_Descr *dtype)
             if (meta == NULL) {
                 return NULL;
             }
-            ret = PyUString_FromFormat("%sM8", byteorder);
-            return append_metastr_to_string(meta, 0, ret);
+            ret = PyUString_FromFormat("'%sM8", byteorder);
+            ret = append_metastr_to_string(meta, 0, ret);
+            PyUString_ConcatAndDel(&ret, PyUString_FromString("'"));
+            return ret;
 
         case NPY_TIMEDELTA:
             meta = get_datetime_metadata_from_dtype(dtype);
             if (meta == NULL) {
                 return NULL;
             }
-            ret = PyUString_FromFormat("%sm8", byteorder);
-            return append_metastr_to_string(meta, 0, ret);
+            ret = PyUString_FromFormat("'%sm8", byteorder);
+            ret = append_metastr_to_string(meta, 0, ret);
+            PyUString_ConcatAndDel(&ret, PyUString_FromString("'"));
+            return ret;
 
         default:
             PyErr_SetString(PyExc_RuntimeError, "Internal error: NumPy dtype "
diff --git a/numpy/core/src/multiarray/descriptor.h b/numpy/core/src/multiarray/descriptor.h
index 4f8a90582..d936d0b31 100644
--- a/numpy/core/src/multiarray/descriptor.h
+++ b/numpy/core/src/multiarray/descriptor.h
@@ -11,16 +11,25 @@ NPY_NO_EXPORT PyArray_Descr *
 _arraydescr_fromobj(PyObject *obj);
 
 /*
- * This creates a shorter repr using the 'kind' and 'itemsize',
- * instead of the longer type name. It also creates the input
- * for constructing a dtype rather than the full dtype function
- * call.
+ * This creates a shorter repr using 'kind' and 'itemsize',
+ * instead of the longer type name. This is the object passed
+ * as the first parameter to the dtype constructor, and if no
+ * additional constructor parameters are given, will reproduce
+ * the exact memory layout.
  *
- * This does not preserve the 'align=True' parameter
- * for structured arrays like the regular repr does.
+ * If 'includealignflag' is true, this includes the 'align=True' parameter
+ * inside the struct dtype construction dict when needed. Use this flag
+ * if you want a proper repr string without the 'dtype()' part around it.
+ *
+ * If 'includealignflag' is false, this does not preserve the
+ * 'align=True' parameter or sticky NPY_ALIGNED_STRUCT flag for
+ * struct arrays like the regular repr does, because the 'align'
+ * flag is not part of first dtype constructor parameter. This
+ * mode is intended for a full 'repr', where the 'align=True' is
+ * provided as the second parameter.
  */
 NPY_NO_EXPORT PyObject *
-arraydescr_short_construction_repr(PyArray_Descr *dtype);
+arraydescr_short_construction_repr(PyArray_Descr *dtype, int includealignflag);
 
 #ifdef NPY_ENABLE_SEPARATE_COMPILATION
 extern NPY_NO_EXPORT char *_datetime_strings[];
diff --git a/numpy/core/tests/test_dtype.py b/numpy/core/tests/test_dtype.py
index 0e1bfe182..6dc3e7554 100644
--- a/numpy/core/tests/test_dtype.py
+++ b/numpy/core/tests/test_dtype.py
@@ -297,6 +297,25 @@ class TestString(TestCase):
                      "('bottom', [('bleft', ('>f4', (8, 64)), (1,)), "
                      "('bright', '>f4', (8, 36))])]")
 
+        # If the sticky aligned flag is set to True, it makes the
+        # str() function use a dict representation with an 'aligned' flag
+        dt = np.dtype([('top', [('tiles', ('>f4', (64, 64)), (1,)),
+                                ('rtile', '>f4', (64, 36))],
+                                (3,)),
+                       ('bottom', [('bleft', ('>f4', (8, 64)), (1,)),
+                                   ('bright', '>f4', (8, 36))])],
+                       align=True)
+        assert_equal(str(dt),
+                    "{'names':['top','bottom'], "
+                     "'formats':[([('tiles', ('>f4', (64, 64)), (1,)), "
+                                  "('rtile', '>f4', (64, 36))], (3,)),"
+                                 "[('bleft', ('>f4', (8, 64)), (1,)), "
+                                  "('bright', '>f4', (8, 36))]], "
+                     "'offsets':[0,76800], "
+                     "'itemsize':80000, "
+                     "'aligned':True}")
+        assert_equal(np.dtype(eval(str(dt))), dt)
+
         dt = np.dtype({'names': ['r','g','b'], 'formats': ['u1', 'u1', 'u1'],
                         'offsets': [0, 1, 2],
                         'titles': ['Red pixel', 'Green pixel', 'Blue pixel']})
@@ -328,6 +347,10 @@ class TestString(TestCase):
                     " 'titles':['Red pixel','Blue pixel'],"
                     " 'itemsize':3}")
 
+        dt = np.dtype([('a', '<m8[D]'), ('b', '<M8[us]')])
+        assert_equal(str(dt),
+                    "[('a', '<m8[D]'), ('b', '<M8[us]')]")
+
     def test_complex_dtype_repr(self):
         dt = np.dtype([('top', [('tiles', ('>f4', (64, 64)), (1,)),
                                 ('rtile', '>f4', (64, 36))], (3,)),
@@ -372,5 +395,10 @@ class TestString(TestCase):
                     "'titles':['Red pixel','Blue pixel'], "
                     "'itemsize':4})")
 
+        dt = np.dtype([('a', '<M8[D]'), ('b', '<m8[us]')])
+        assert_equal(repr(dt),
+                    "dtype([('a', '<M8[D]'), ('b', '<m8[us]')])")
+
+
 if __name__ == "__main__":
     run_module_suite()