-
-
Notifications
You must be signed in to change notification settings - Fork 11.8k
Closed
Milestone
Description
It's still very early in the morning and I may simply not be awake yet, but it looks to me like the __array__ changes in gh-25168 are incomplete and don't have a regression test for the old signature:
In [1]: import numpy as np
In [2]: class MyArr:
...: def __init__(self, x):
...: self._x = x
...: def __array__(self, dtype=None):
...: return self._x
...:
In [3]: x = np.arange(3)
In [4]: np.asarray(x)
Out[4]: array([0, 1, 2])
In [5]: np.asarray(MyArr(x))
...
TypeError: MyArr.__array__() got an unexpected keyword argument 'copy'
In [6]: np.__version__
Out[6]: '2.0.0.dev0+git20240301.06b169b'I didn't follow the review of the C code in detail, so I'm probably missing some details, but it doesn't look like the current implementation does what was intended. In particular:
- only pass along the new
copykeyword if it's not the default value ofNone, to have a smaller impact, - pass the actual True/False/None values; it now only passes
FalseorNone(never_copy ? Py_False : Py_None;), - when retrying without the new keyword, t
8774
he
dtypekeyword should still be passed, so the code should popcopyand retry withkwargsrather than passing NULL.
It looks like the changes should be along these lines (incomplete):
$ git diff
diff --git a/numpy/_core/src/multiarray/ctors.c b/numpy/_core/src/multiarray/ctors.c
index 4b9c3add3f..897e141336 100644
--- a/numpy/_core/src/multiarray/ctors.c
+++ b/numpy/_core/src/multiarray/ctors.c
@@ -2442,26 +2442,46 @@ PyArray_FromArrayAttr_int(
PyObject *copy = never_copy ? Py_False : Py_None;
PyObject *kwargs = PyDict_New();
- PyDict_SetItemString(kwargs, "copy", copy);
+
+ /*
+ * Only if the value of `copy` isn't the default one, we try to pass it
+ * along; for backwards compatibility we then retry if it fails because the
+ * signature of the __array__ method being called does not have `copy`.
+ */
+ int copy_passed = 0;
+ if (copy != Py_None) {
+ /* XXX: why can `copy` only be False or None, not True? */
+ copy_passed = 1;
+ PyDict_SetItemString(kwargs, "copy", copy);
+ }
PyObject *args = descr != NULL ? PyTuple_Pack(1, descr) : PyTuple_New(0);
new = PyObject_Call(array_meth, args, kwargs);
if (PyErr_Occurred()) {
+ PyObject *errmsg_substr = PyUnicode_FromString(
+ "__array__() got an unexpected keyword argument 'copy'");
+ if (errmsg_substr == NULL) {
+ return NULL;
+ }
PyObject *type, *value, *traceback;
PyErr_Fetch(&type, &value, &traceback);
- if (PyUnicode_Check(value) && PyUnicode_CompareWithASCIIString(value,
- "__array__() got an unexpected keyword argument 'copy'") == 0) {
+ if (PyUnicode_Check(value) && PyUnicode_Contains(value, errmsg_substr) > 0) {
Py_DECREF(type);
Py_XDECREF(value);
Py_XDECREF(traceback);
+ Py_DECREF(errmsg_substr);
if (PyErr_WarnEx(PyExc_UserWarning,
"__array__ should implement 'dtype' and 'copy' keywords", 1) < 0) {
return NULL;
}
- Py_SETREF(new, PyObject_Call(array_meth, args, NULL));
+ if (copy_passed) { /* try again */
+ PyDict_DelItemString(kwargs, "copy");
+ Py_SETREF(new, PyObject_Call(array_meth, args, kwargs));
+ }
} else {
PyErr_Restore(type, value, traceback);
+ Py_DECREF(errmsg_substr);
return NULL;
}
}mtsokol