Merge pull request numpy#21582 from seberg/small-fixups-snape

MAINT: Fix some small refcounting and similar issues

numpy/core/src/multiarray/_multiarray_tests.c.src

@@ -1035,7 +1035,7 @@ get_all_cast_information(PyObject *NPY_UNUSED(mod), PyObject *NPY_UNUSED(args))
     PyObject *classes = PyObject_CallMethod(
             (PyObject *)&PyArrayDescr_Type, "__subclasses__", "");
     if (classes == NULL) {
-        return NULL;
+        goto fail;
     }
     Py_SETREF(classes, PySequence_Fast(classes, NULL));
     if (classes == NULL) {
@@ -2010,6 +2010,10 @@ get_struct_alignments(PyObject *NPY_UNUSED(self), PyObject *args) {
     PyObject *ret = PyTuple_New(3);
     PyObject *alignment, *size, *val;
 
+    if (ret == NULL) {
+        return NULL;
+    }
+
 /**begin repeat
  * #N = 1,2,3#
  */
@@ -2019,6 +2023,7 @@ get_struct_alignments(PyObject *NPY_UNUSED(self), PyObject *args) {
     Py_DECREF(alignment);
     Py_DECREF(size);
     if (val == NULL) {
+        Py_DECREF(ret);
         return NULL;
     }
     PyTuple_SET_ITEM(ret, @N@-1, val);

numpy/core/src/multiarray/common.c

@@ -136,7 +136,6 @@ _zerofill(PyArrayObject *ret)
         PyArray_FillObjectArray(ret, zero);
         Py_DECREF(zero);
         if (PyErr_Occurred()) {
-            Py_DECREF(ret);
             return -1;
         }
     }

numpy/core/src/multiarray/descriptor.c

@@ -2164,6 +2164,7 @@ arraydescr_names_set(
 
     N = PyTuple_GET_SIZE(self->names);
     if (!PySequence_Check(val) || PyObject_Size((PyObject *)val) != N) {
+        /* Should be a TypeError, but this should be deprecated anyway. */
         PyErr_Format(PyExc_ValueError,
                 "must replace all names at once with a sequence of length %d",
                 N);
@@ -2172,16 +2173,17 @@ arraydescr_names_set(
     /* Make sure all entries are strings */
     for (i = 0; i < N; i++) {
         PyObject *item;
-        int valid = 1;
+        int valid;
         item = PySequence_GetItem(val, i);
         valid = PyUnicode_Check(item);
-        Py_DECREF(item);
         if (!valid) {
             PyErr_Format(PyExc_ValueError,
                     "item #%d of names is of type %s and not string",
                     i, Py_TYPE(item)->tp_name);
+            Py_DECREF(item);
             return -1;
         }
+        Py_DECREF(item);
     }
     /* Invalidate cached hash value */
     self->hash = -1;

numpy/core/src/multiarray/getset.c

@@ -808,6 +808,7 @@ array_imag_get(PyArrayObject *self, void *NPY_UNUSED(ignored))
             return NULL;
         }
         if (_zerofill(ret) < 0) {
+            Py_DECREF(ret);
             return NULL;
         }
         PyArray_CLEARFLAGS(ret, NPY_ARRAY_WRITEABLE);

numpy/core/tests/test_dtype.py

@@ -223,7 +223,8 @@ def test_refcount_dictionary_setting(self):
         assert refcounts == refcounts_new
 
     def test_mutate(self):
-        # Mutating a dtype should reset the cached hash value
+        # Mutating a dtype should reset the cached hash value.
+        # NOTE: Mutating should be deprecated, but new API added to replace it.
         a = np.dtype([('yo', int)])
         b = np.dtype([('yo', int)])
         c = np.dtype([('ye', int)])
@@ -237,6 +238,16 @@ def test_mutate(self):
         assert_dtype_equal(a, b)
         assert_dtype_not_equal(a, c)
 
+    def test_mutate_error(self):
+        # NOTE: Mutating should be deprecated, but new API added to replace it.
+        a = np.dtype("i,i")
+
+        with pytest.raises(ValueError, match="must replace all names at once"):
+            a.names = ["f0"]
+
+        with pytest.raises(ValueError, match=".*and not string"):
+            a.names = ["f0", b"not a unicode name"]
+
     def test_not_lists(self):
         """Test if an appropriate exception is raised when passing bad values to
         the dtype constructor.