MNT: respond to review comments

Author: ngoldbaum

numpy/_core/src/multiarray/convert_datatype.c

@@ -651,6 +651,35 @@ PyArray_CanCastTo(PyArray_Descr *from, PyArray_Descr *to)
 }
 
 
+/*
+ * This function returns true if the two types can be safely cast at
+ * *minimum_safety* casting level. Sets the *view_offset* if that is set
+ * for the cast. If ignore_error is set, the error indicator is cleared
+ * if there are any errors in cast setup and returns false, otherwise
+ * the error indicator is left set and returns -1.
+ */
+NPY_NO_EXPORT npy_intp
+PyArray_SafeCast(PyArray_Descr *type1, PyArray_Descr *type2,
+                 npy_intp* view_offset, NPY_CASTING minimum_safety,
+                 npy_intp ignore_error)
+{
+    if (type1 == type2) {
+        *view_offset = 0;
+        return 1;
+    }
+
+    NPY_CASTING safety = PyArray_GetCastInfo(type1, type2, NULL, view_offset);
+    if (safety < 0) {
+        if (ignore_error) {
+            PyErr_Clear();
+            return 0;
+        }
+        return -1;
+    }
+    return PyArray_MinCastSafety(safety, minimum_safety) == minimum_safety;
+}
+
+
 /* Provides an ordering for the dtype 'kind' character codes */
 NPY_NO_EXPORT int
 dtype_kind_to_ordering(char kind)

numpy/_core/src/multiarray/convert_datatype.h

@@ -102,6 +102,11 @@ PyArray_GetCastInfo(
         PyArray_Descr *from, PyArray_Descr *to, PyArray_DTypeMeta *to_dtype,
         npy_intp *view_offset);
 
+NPY_NO_EXPORT npy_intp
+PyArray_SafeCast(PyArray_Descr *type1, PyArray_Descr *type2,
+                 npy_intp* view_offset, NPY_CASTING minimum_safety,
+                 npy_intp ignore_errors);
+
 NPY_NO_EXPORT int
 PyArray_CheckCastSafety(NPY_CASTING casting,
         PyArray_Descr *from, PyArray_Descr *to, PyArray_DTypeMeta *to_dtype);

numpy/_core/src/multiarray/ctors.c

@@ -1908,12 +1908,10 @@ PyArray_FromArray(PyArrayObject *arr, PyArray_Descr *newtype, int flags)
     }
 
     arrflags = PyArray_FLAGS(arr);
-    npy_intp view_offset;
-    npy_intp is_safe = PyArray_SafeCast(oldtype, newtype, &view_offset, NPY_NO_CASTING, 1);
-    npy_intp view_safe = (is_safe && (view_offset == 0));
 
-           /* If a guaranteed copy was requested */
-    copy = (flags & NPY_ARRAY_ENSURECOPY) ||
+
+    copy = /* If a guaranteed copy was requested */
+           (flags & NPY_ARRAY_ENSURECOPY) ||
            /* If C contiguous was requested, and arr is not */
            ((flags & NPY_ARRAY_C_CONTIGUOUS) &&
                    (!(arrflags & NPY_ARRAY_C_CONTIGUOUS))) ||
@@ -1925,8 +1923,13 @@ PyArray_FromArray(PyArrayObject *arr, PyArray_Descr *newtype, int flags)
                    (!(arrflags & NPY_ARRAY_F_CONTIGUOUS))) ||
            /* If a writeable array was requested, and arr is not */
            ((flags & NPY_ARRAY_WRITEABLE) &&
-                   (!(arrflags & NPY_ARRAY_WRITEABLE))) ||
-           !view_safe;
+                   (!(arrflags & NPY_ARRAY_WRITEABLE)));
+
+    if (!copy) {
+        npy_intp view_offset;
+        npy_intp is_safe = PyArray_SafeCast(oldtype, newtype, &view_offset, NPY_NO_CASTING, 1);
+        copy = copy || !(is_safe && (view_offset == 0));
+    }
 
     if (copy) {
         if (flags & NPY_ARRAY_ENSURENOCOPY ) {

numpy/_core/src/multiarray/multiarraymodule.c

@@ -1464,33 +1464,6 @@ PyArray_EquivTypes(PyArray_Descr *type1, PyArray_Descr *type2)
 }
 
 
-/*
- * This function returns true if the two types can be safely cast at
- * *minimum_safety* casting level. Sets the view_offset if that is set
- * for the cast. If ignore_error is 1, errors in cast setup are ignored.
- */
-NPY_NO_EXPORT npy_intp
-PyArray_SafeCast(PyArray_Descr *type1, PyArray_Descr *type2,
-                 npy_intp* view_offset, NPY_CASTING minimum_safety,
-                 npy_intp ignore_error)
-{
-    if (type1 == type2) {
-        *view_offset = 0;
-        return 1;
-    }
-
-    NPY_CASTING safety = PyArray_GetCastInfo(type1, type2, NULL, view_offset);
-    if (safety < 0) {
-        if (ignore_error) {
-            PyErr_Clear();
-            return 0;
-        }
-        return -1;
-    }
-    return PyArray_MinCastSafety(safety, minimum_safety) == minimum_safety;
-}
-
-
 /*NUMPY_API*/
 NPY_NO_EXPORT unsigned char
 PyArray_EquivTypenums(int typenum1, int typenum2)

numpy/_core/src/multiarray/multiarraymodule.h

@@ -21,9 +21,4 @@ NPY_VISIBILITY_HIDDEN extern PyObject * npy_ma_str_convert_if_no_array;
 NPY_VISIBILITY_HIDDEN extern PyObject * npy_ma_str_cpu;
 NPY_VISIBILITY_HIDDEN extern PyObject * npy_ma_str_array_err_msg_substr;
 
-NPY_NO_EXPORT npy_intp
-PyArray_SafeCast(PyArray_Descr *type1, PyArray_Descr *type2,
-                 npy_intp* view_offset, NPY_CASTING minimum_safety,
-                 npy_intp ignore_errors);
-
 #endif  /* NUMPY_CORE_SRC_MULTIARRAY_MULTIARRAYMODULE_H_ */

numpy/_core/src/umath/ufunc_object.c

@@ -62,7 +62,6 @@
 #include "legacy_array_method.h"
 #include "abstractdtypes.h"
 #include "mapping.h"
-#include "multiarraymodule.h"
 
 /* TODO: Only for `NpyIter_GetTransferFlags` until it is public */
 #define NPY_ITERATOR_IMPLEMENTATION_CODE

numpy/_core/tests/test_stringdtype.py

@@ -460,15 +460,21 @@ def test_creation_functions():
 
 def test_create_with_copy_none(string_list):
     arr = np.array(string_list, dtype=StringDType())
+    # create another stringdtype array with an arena that has a different
+    # in-memory layout than the first array
     arr_rev = np.array(string_list[::-1], dtype=StringDType())
 
+    # this should create a copy and the resulting array
+    # shouldn't share an allocator or arena with arr_rev, despite
+    # explicitly passing arr_rev.dtype
     arr_copy = np.array(arr, copy=None, dtype=arr_rev.dtype)
     np.testing.assert_array_equal(arr, arr_copy)
     assert arr_copy.base is None
 
     with pytest.raises(ValueError, match="Unable to avoid copy"):
         np.array(arr, copy=False, dtype=arr_rev.dtype)
 
+    # because we're using arr's dtype instance, the view is safe
     arr_view = np.array(arr, copy=None, dtype=arr.dtype)
     np.testing.assert_array_equal(arr, arr)
     np.testing.assert_array_equal(arr_view[::-1], arr_rev)