Consolidate all array writeability checking in new PyArray_RequireWriteable

This is mostly a code cleanup, but it does have a user-visible effect
in that attempting to write to a unwriteable array now consistently
raises ValueError. (It used to randomly raise either ValueError or
RuntimeError.)

Passes numpy.test("full").

15 files changed, 94 insertions, 81 deletions

diff --git a/doc/release/2.0.0-notes.rst b/doc/release/2.0.0-notes.rst
index 3b61afc31..978d9139e 100644
--- a/doc/release/2.0.0-notes.rst
+++ b/doc/release/2.0.0-notes.rst
@@ -148,6 +148,14 @@ New argument to searchsorted
 The function searchsorted now accepts a 'sorter' argument that is a
 permuation array that sorts the array to search.
 
+C API
+-----
+
+New function ``PyArray_RequireWriteable`` provides a consistent
+interface for checking array writeability -- any C code which works
+with arrays whose WRITEABLE flag is not know to be True a priori,
+should make sure to call this function before writing.
+
 Changes
 =======
 
@@ -166,6 +174,11 @@ during alpha or early beta testing, and a decision to either keep the
 stricter behavior, or add in a hack to allow the previous behavior to
 work.
 
+Attempting to write to a read-only array (one with
+``arr.flags.writeable`` set to ``False``) used to raise either a
+RuntimeError, ValueError, or TypeError inconsistently, depending on
+which code path was taken. It now consistently raises a ValueError.
+
 The functions np.diag, np.diagonal, and <ndarray>.diagonal now return a
 view into the original array instead of making a copy. This makes these
 functions more consistent with NumPy's general approach of taking views
diff --git a/numpy/core/code_generators/numpy_api.py b/numpy/core/code_generators/numpy_api.py
index ca89c28ec..4cda287b6 100644
--- a/numpy/core/code_generators/numpy_api.py
+++ b/numpy/core/code_generators/numpy_api.py
@@ -344,6 +344,7 @@ multiarray_funcs_api = {
     'NpyNA_FromDTypeAndPayload':            304,
     'PyArray_AllowNAConverter':             305,
     'PyArray_OutputAllowNAConverter':       306,
+    'PyArray_RequireWriteable':             307,
 }
 
 ufunc_types_api = {
diff --git a/numpy/core/src/multiarray/array_assign_array.c b/numpy/core/src/multiarray/array_assign_array.c
index 03231a995..0551b1089 100644
--- a/numpy/core/src/multiarray/array_assign_array.c
+++ b/numpy/core/src/multiarray/array_assign_array.c
@@ -449,10 +449,7 @@ PyArray_AssignArray(PyArrayObject *dst, PyArrayObject *src,
         return 0;
     }
 
-    /* Check that 'dst' is writeable */
-    if (!PyArray_ISWRITEABLE(dst)) {
-        PyErr_SetString(PyExc_RuntimeError,
-                "cannot assign to a read-only array");
+    if (PyArray_RequireWriteable(dst, NULL) < 0) {
         goto fail;
     }
 
diff --git a/numpy/core/src/multiarray/array_assign_scalar.c b/numpy/core/src/multiarray/array_assign_scalar.c
index bf0676c96..43c4b7350 100644
--- a/numpy/core/src/multiarray/array_assign_scalar.c
+++ b/numpy/core/src/multiarray/array_assign_scalar.c
@@ -336,10 +336,7 @@ PyArray_AssignRawScalar(PyArrayObject *dst,
     int allocated_src_data = 0, dst_has_maskna = PyArray_HASMASKNA(dst);
     npy_longlong scalarbuffer[4];
 
-    /* Check that 'dst' is writeable */
-    if (!PyArray_ISWRITEABLE(dst)) {
-        PyErr_SetString(PyExc_RuntimeError,
-                "cannot assign a scalar value to a read-only array");
+    if (PyArray_RequireWriteable(dst, "cannot assign to read-only array") < 0) {
         return -1;
     }
 
diff --git a/numpy/core/src/multiarray/arrayobject.c b/numpy/core/src/multiarray/arrayobject.c
index 11b04989f..78ef1d94b 100644
--- a/numpy/core/src/multiarray/arrayobject.c
+++ b/numpy/core/src/multiarray/arrayobject.c
@@ -704,6 +704,27 @@ PyArray_CompareString(char *s1, char *s2, size_t len)
 }
 
 
+/*NUMPY_API
+ *
+ * This function does nothing if obj is writeable, and raises an exception
+ * (and returns -1) if obj is not writeable. In the future it may also do
+ * other house-keeping, such as issuing warnings on arrays which are
+ * transitioning to become views. Always call this function at some point
+ * before writing to an array.
+ */
+NPY_NO_EXPORT int
+PyArray_RequireWriteable(PyArrayObject *obj, const char * err)
+{
+    if (!err) {
+        err = "array must be writeable (but isn't)";
+    }
+    if (!PyArray_ISWRITEABLE(obj)) {
+        PyErr_SetString(PyExc_ValueError, err);
+        return -1;
+    }
+    return 0;
+}
+
 /* This also handles possibly mis-aligned data */
 /* Compare s1 and s2 which are not necessarily NULL-terminated.
    s1 is of length len1
diff --git a/numpy/core/src/multiarray/buffer.c b/numpy/core/src/multiarray/buffer.c
index 694207197..8389c70b0 100644
--- a/numpy/core/src/multiarray/buffer.c
+++ b/numpy/core/src/multiarray/buffer.c
@@ -57,14 +57,12 @@ array_getreadbuf(PyArrayObject *self, Py_ssize_t segment, void **ptrptr)
 static Py_ssize_t
 array_getwritebuf(PyArrayObject *self, Py_ssize_t segment, void **ptrptr)
 {
-    if (PyArray_CHKFLAGS(self, NPY_ARRAY_WRITEABLE)) {
-        return array_getreadbuf(self, segment, (void **) ptrptr);
-    }
-    else {
-        PyErr_SetString(PyExc_ValueError, "array cannot be "
-                        "accessed as a writeable buffer");
+    if (PyArray_RequireWriteable(self,
+                                 "cannot get writeable buffer from "
+                                 "non-writeable array") < 0) {
         return -1;
     }
+    return array_getreadbuf(self, segment, (void **) ptrptr);
 }
 
 static Py_ssize_t
@@ -628,10 +626,10 @@ array_getbuffer(PyObject *obj, Py_buffer *view, int flags)
         PyErr_SetString(PyExc_ValueError, "ndarray is not C-contiguous");
         goto fail;
     }
-    if ((flags & PyBUF_WRITEABLE) == PyBUF_WRITEABLE &&
-        !PyArray_ISWRITEABLE(self)) {
-        PyErr_SetString(PyExc_ValueError, "ndarray is not writeable");
-        goto fail;
+    if ((flags & PyBUF_WRITEABLE) == PyBUF_WRITEABLE) {
+        if (PyArray_RequireWriteable(self, NULL) < 0) {
+            goto fail;
+        }
     }
 
     if (view == NULL) {
diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c
index c8548f894..2c6804678 100644
--- a/numpy/core/src/multiarray/ctors.c
+++ b/numpy/core/src/multiarray/ctors.c
@@ -1348,9 +1348,8 @@ PyArray_GetArrayParamsFromObjectEx(PyObject *op,
 
     /* If op is an array */
     if (PyArray_Check(op)) {
-        if (writeable && !PyArray_ISWRITEABLE((PyArrayObject *)op)) {
-            PyErr_SetString(PyExc_RuntimeError,
-                            "cannot write to array");
+        if (writeable
+            && PyArray_RequireWriteable((PyArrayObject *)op, NULL) < 0) {
             return -1;
         }
         Py_INCREF(op);
@@ -1419,9 +1418,10 @@ PyArray_GetArrayParamsFromObjectEx(PyObject *op,
     /* If op supports the PEP 3118 buffer interface */
     if (!PyBytes_Check(op) && !PyUnicode_Check(op) &&
              _array_from_buffer_3118(op, (PyObject **)out_arr) == 0) {
-        if (writeable && !PyArray_ISWRITEABLE(*out_arr)) {
-            PyErr_SetString(PyExc_RuntimeError,
-                                "cannot write to PEP 3118 buffer");
+        if (writeable
+            && PyArray_RequireWriteable(*out_arr,
+                                        "PEP 3118 buffer is not writeable")
+               < 0) {
             Py_DECREF(*out_arr);
             return -1;
         }
@@ -1440,9 +1440,10 @@ PyArray_GetArrayParamsFromObjectEx(PyObject *op,
         }
     }
     if (tmp != Py_NotImplemented) {
-        if (writeable && !PyArray_ISWRITEABLE((PyArrayObject *)tmp)) {
-            PyErr_SetString(PyExc_RuntimeError,
-                                "cannot write to array interface object");
+        if (writeable
+            && PyArray_RequireWriteable((PyArrayObject *)tmp,
+                                        "array interface object is not "
+                                        "writeable") < 0) {
             Py_DECREF(tmp);
             return -1;
         }
@@ -1462,9 +1463,10 @@ PyArray_GetArrayParamsFromObjectEx(PyObject *op,
     if (!writeable) {
         tmp = PyArray_FromArrayAttr(op, requested_dtype, context);
         if (tmp != Py_NotImplemented) {
-            if (writeable && !PyArray_ISWRITEABLE((PyArrayObject *)tmp)) {
-                PyErr_SetString(PyExc_RuntimeError,
-                                    "cannot write to array interface object");
+            if (writeable
+                && PyArray_RequireWriteable((PyArrayObject *)tmp,
+                                            "array interface object is not "
+                                            "writeable") < 0) {
                 Py_DECREF(tmp);
                 return -1;
             }
@@ -2032,12 +2034,12 @@ PyArray_FromArray(PyArrayObject *arr, PyArray_Descr *newtype, int flags)
             order = NPY_CORDER;
         }
 
-        if ((flags & NPY_ARRAY_UPDATEIFCOPY) &&
-                            (!PyArray_ISWRITEABLE(arr))) {
-            Py_DECREF(newtype);
-            PyErr_SetString(PyExc_ValueError,
-                    "cannot copy back to a read-only array");
-            return NULL;
+        if (flags & NPY_ARRAY_UPDATEIFCOPY) {
+            const char * msg = "cannot copy back to a read-only array";
+            if (PyArray_RequireWriteable(arr, msg) < 0) {
+                Py_DECREF(newtype);
+                return NULL;
+            }
         }
         if ((flags & NPY_ARRAY_ENSUREARRAY)) {
             subok = 0;
@@ -2599,9 +2601,7 @@ PyArray_CopyAsFlat(PyArrayObject *dst, PyArrayObject *src, NPY_ORDER order)
 
     NPY_BEGIN_THREADS_DEF;
 
-    if (!PyArray_ISWRITEABLE(dst)) {
-        PyErr_SetString(PyExc_RuntimeError,
-                "cannot write to array");
+    if (PyArray_RequireWriteable(dst, NULL) < 0) {
         return -1;
     }
 
diff --git a/numpy/core/src/multiarray/item_selection.c b/numpy/core/src/multiarray/item_selection.c
index 77faa1eb2..21892045f 100644
--- a/numpy/core/src/multiarray/item_selection.c
+++ b/numpy/core/src/multiarray/item_selection.c
@@ -1114,9 +1114,7 @@ PyArray_Sort(PyArrayObject *op, int axis, NPY_SORTKIND which)
         PyErr_Format(PyExc_ValueError, "axis(=%d) out of bounds", axis);
         return -1;
     }
-    if (!PyArray_ISWRITEABLE(op)) {
-        PyErr_SetString(PyExc_RuntimeError,
-                        "attempted sort on unwriteable array.");
+    if (PyArray_RequireWriteable(op, "attempted sort on read-only array") < 0) {
         return -1;
     }
 
diff --git a/numpy/core/src/multiarray/mapping.c b/numpy/core/src/multiarray/mapping.c
index ec746481c..851a710a8 100644
--- a/numpy/core/src/multiarray/mapping.c
+++ b/numpy/core/src/multiarray/mapping.c
@@ -176,9 +176,7 @@ array_ass_big_item(PyArrayObject *self, npy_intp i, PyObject *v)
         return -1;
     }
 
-    if (!PyArray_ISWRITEABLE(self)) {
-        PyErr_SetString(PyExc_RuntimeError,
-                        "array is not writeable");
+    if (PyArray_RequireWriteable(self, NULL) < 0) {
         return -1;
     }
 
@@ -1502,9 +1500,7 @@ array_ass_sub(PyArrayObject *self, PyObject *ind, PyObject *op)
                         "cannot delete array elements");
         return -1;
     }
-    if (!PyArray_ISWRITEABLE(self)) {
-        PyErr_SetString(PyExc_RuntimeError,
-                        "array is not writeable");
+    if (PyArray_RequireWriteable(self, NULL) < 0) {
         return -1;
     }
 
diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c
index 8129a7021..67332fda3 100644
--- a/numpy/core/src/multiarray/methods.c
+++ b/numpy/core/src/multiarray/methods.c
@@ -532,10 +532,9 @@ PyArray_Byteswap(PyArrayObject *self, npy_bool inplace)
 
     copyswapn = PyArray_DESCR(self)->f->copyswapn;
     if (inplace) {
-        if (!PyArray_ISWRITEABLE(self)) {
-            PyErr_SetString(PyExc_RuntimeError,
-                            "Cannot byte-swap in-place on a " \
-                            "read-only array");
+        const char *msg = "Cannot perform in-place byte-swap on a "
+                          "read-only array";
+        if (PyArray_RequireWriteable(self, msg) < 0) {
             return NULL;
         }
         size = PyArray_SIZE(self);
@@ -748,9 +747,7 @@ array_setscalar(PyArrayObject *self, PyObject *args)
                 "itemset must have at least one argument");
         return NULL;
     }
-    if (!PyArray_ISWRITEABLE(self)) {
-        PyErr_SetString(PyExc_RuntimeError,
-                "array is not writeable");
+    if (PyArray_RequireWriteable(self, NULL) < 0) {
         return NULL;
     }
 
diff --git a/numpy/core/src/multiarray/nditer_constr.c b/numpy/core/src/multiarray/nditer_constr.c
index d02a9ae4f..cfd59abc2 100644
--- a/numpy/core/src/multiarray/nditer_constr.c
+++ b/numpy/core/src/multiarray/nditer_constr.c
@@ -1098,11 +1098,11 @@ npyiter_prepare_one_operand(PyArrayObject **op,
     if (PyArray_Check(*op)) {
         npy_uint32 tmp;
 
-        if (((*op_itflags) & NPY_OP_ITFLAG_WRITE) &&
-                    (!PyArray_CHKFLAGS(*op, NPY_ARRAY_WRITEABLE))) {
-            PyErr_SetString(PyExc_ValueError,
-                    "Operand was a non-writeable array, but "
-                    "flagged as writeable");
+        if ((*op_itflags) & NPY_OP_ITFLAG_WRITE
+            && PyArray_RequireWriteable(*op,
+                                        "Operand array is not writeable, "
+                                        "but the iterator write flag is set"
+                ) < 0) {
             return 0;
         }
         if (!(flags & NPY_ITER_ZEROSIZE_OK) && PyArray_SIZE(*op) == 0) {
diff --git a/numpy/core/src/multiarray/sequence.c b/numpy/core/src/multiarray/sequence.c
index 004aa2d78..8065404c5 100644
--- a/numpy/core/src/multiarray/sequence.c
+++ b/numpy/core/src/multiarray/sequence.c
@@ -119,9 +119,7 @@ array_ass_slice(PyArrayObject *self, Py_ssize_t ilow,
                         "cannot delete array elements");
         return -1;
     }
-    if (!PyArray_ISWRITEABLE(self)) {
-        PyErr_SetString(PyExc_RuntimeError,
-                        "array is not writeable");
+    if (PyArray_RequireWriteable(self, NULL) < 0) {
         return -1;
     }
     tmp = (PyArrayObject *)array_slice(self, ilow, ihigh);
diff --git a/numpy/core/src/umath/ufunc_object.c b/numpy/core/src/umath/ufunc_object.c
index 93f63038a..c42e2d626 100644
--- a/numpy/core/src/umath/ufunc_object.c
+++ b/numpy/core/src/umath/ufunc_object.c
@@ -795,9 +795,8 @@ static int get_ufunc_arguments(PyUFuncObject *ufunc,
         }
         /* If it's an array, can use it */
         if (PyArray_Check(obj)) {
-            if (!PyArray_ISWRITEABLE((PyArrayObject *)obj)) {
-                PyErr_SetString(PyExc_ValueError,
-                                "return array is not writeable");
+            const char *msg = "output array not writeable";
+            if (PyArray_RequireWriteable((PyArrayObject *)obj, msg) < 0) {
                 return -1;
             }
             Py_INCREF(obj);
@@ -894,9 +893,9 @@ static int get_ufunc_arguments(PyUFuncObject *ufunc,
                         }
 
                         if (PyArray_Check(value)) {
-                            if (!PyArray_ISWRITEABLE((PyArrayObject *)value)) {
-                                PyErr_SetString(PyExc_ValueError,
-                                        "return array is not writeable");
+                            const char *msg = "output array not writeable";
+                            PyArrayObject *value_arr = (PyArrayObject *)value;
+                            if (PyArray_RequireWriteable(value_arr, msg) < 0) {
                                 goto fail;
                             }
                             Py_INCREF(value);
diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py
index b2b4bab2b..398643ca6 100644
--- a/numpy/core/tests/test_multiarray.py
+++ b/numpy/core/tests/test_multiarray.py
@@ -29,8 +29,8 @@ class TestFlags(TestCase):
     def test_writeable(self):
         mydict = locals()
         self.a.flags.writeable = False
-        self.assertRaises(RuntimeError, runstring, 'self.a[0] = 3', mydict)
-        self.assertRaises(RuntimeError, runstring, 'self.a[0:1].itemset(3)', mydict)
+        self.assertRaises(ValueError, runstring, 'self.a[0] = 3', mydict)
+        self.assertRaises(ValueError, runstring, 'self.a[0:1].itemset(3)', mydict)
         self.a.flags.writeable = True
         self.a[0] = 5
         self.a[0] = 0
diff --git a/numpy/numarray/_capi.c b/numpy/numarray/_capi.c
index fee07d79d..b6e5c1a3c 100644
--- a/numpy/numarray/_capi.c
+++ b/numpy/numarray/_capi.c
@@ -1077,9 +1077,12 @@ NA_OutputArray(PyObject *a, NumarrayType t, int requires)
     PyArray_Descr *dtype;
     PyArrayObject *ret;
 
-    if (!PyArray_Check(a) || !PyArray_ISWRITEABLE((PyArrayObject *)a)) {
+    if (!PyArray_Check(a)) {
         PyErr_Format(PyExc_TypeError,
-                "NA_OutputArray: only writeable arrays work for output.");
+                "NA_OutputArray: only arrays work for output.");
+        return NULL;
+    }
+    if (PyArray_RequireWriteable((PyArrayObject *)a, NULL) < 0) {
         return NULL;
     }
 
@@ -1127,9 +1130,7 @@ NA_IoArray(PyObject *a, NumarrayType t, int requires)
     /* Guard against non-writable, but otherwise satisfying requires.
        In this case,  shadow == a.
        */
-    if (!PyArray_ISWRITABLE(shadow)) {
-        PyErr_Format(PyExc_TypeError,
-                "NA_IoArray: I/O array must be writable array");
+    if (!PyArray_RequireWriteable(shadow, NULL)) {
         PyArray_XDECREF_ERR(shadow);
         return NULL;
     }
@@ -2488,13 +2489,10 @@ _setFromPythonScalarCore(PyArrayObject *a, long offset, PyObject*value, int entr
 static int
 NA_setFromPythonScalar(PyArrayObject *a, long offset, PyObject *value)
 {
-    if (PyArray_FLAGS(a) & NPY_ARRAY_WRITEABLE)
-        return _setFromPythonScalarCore(a, offset, value, 0);
-    else {
-        PyErr_Format(
-                PyExc_ValueError, "NA_setFromPythonScalar: assigment to readonly array buffer");
+    if (PyArray_RequireWriteable(a, NULL) < 0) {
         return -1;
     }
+    return _setFromPythonScalarCore(a, offset, value, 0);
 }