Skip to content

BUG,MAINT: Updateifcopy arrays should always be copied. #9249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions numpy/core/src/multiarray/ctors.c
Original file line number Diff line number Diff line change
Expand Up @@ -1849,10 +1849,11 @@ PyArray_FromAny(PyObject *op, PyArray_Descr *newtype, int min_depth,
*/
NPY_NO_EXPORT PyObject *
PyArray_CheckFromAny(PyObject *op, PyArray_Descr *descr, int min_depth,
int max_depth, int requires, PyObject *context)
int max_depth, int flags, PyObject *context)
{
PyObject *obj;
if (requires & NPY_ARRAY_NOTSWAPPED) {

if (flags & NPY_ARRAY_NOTSWAPPED) {
if (!descr && PyArray_Check(op) &&
!PyArray_ISNBO(PyArray_DESCR((PyArrayObject *)op)->byteorder)) {
descr = PyArray_DescrNew(PyArray_DESCR((PyArrayObject *)op));
Expand All @@ -1865,11 +1866,11 @@ PyArray_CheckFromAny(PyObject *op, PyArray_Descr *descr, int min_depth,
}
}

obj = PyArray_FromAny(op, descr, min_depth, max_depth, requires, context);
obj = PyArray_FromAny(op, descr, min_depth, max_depth, flags, context);
if (obj == NULL) {
return NULL;
}
if ((requires & NPY_ARRAY_ELEMENTSTRIDES) &&
if ((flags & NPY_ARRAY_ELEMENTSTRIDES) &&
!PyArray_ElementStrides(obj)) {
PyObject *ret;
ret = PyArray_NewCopy((PyArrayObject *)obj, NPY_ANYORDER);
Expand Down Expand Up @@ -1963,8 +1964,8 @@ PyArray_FromArray(PyArrayObject *arr, PyArray_Descr *newtype, int flags)
}

arrflags = PyArray_FLAGS(arr);
/* 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))) ||
Expand All @@ -1977,7 +1978,10 @@ PyArray_FromArray(PyArrayObject *arr, PyArray_Descr *newtype, int flags)
/* If a writeable array was requested, and arr is not */
((flags & NPY_ARRAY_WRITEABLE) &&
(!(arrflags & NPY_ARRAY_WRITEABLE))) ||
!PyArray_EquivTypes(oldtype, newtype);
/* If the type needs casting */
!PyArray_EquivTypes(oldtype, newtype) ||
/* If the array has updateifcopy flag */
(arrflags & NPY_ARRAY_UPDATEIFCOPY);

if (copy) {
NPY_ORDER order = NPY_KEEPORDER;
Expand Down
35 changes: 26 additions & 9 deletions numpy/core/src/multiarray/multiarraymodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1596,7 +1596,7 @@ static PyObject *
_array_fromobject(PyObject *NPY_UNUSED(ignored), PyObject *args, PyObject *kws)
{
PyObject *op;
PyArrayObject *oparr = NULL, *ret = NULL;
PyArrayObject *ret = NULL;
npy_bool subok = NPY_FALSE;
npy_bool copy = NPY_TRUE;
int ndmin = 0, nd;
Expand All @@ -1618,10 +1618,12 @@ _array_fromobject(PyObject *NPY_UNUSED(ignored), PyObject *args, PyObject *kws)
if (PyTuple_GET_SIZE(args) == 0) {
goto full_path;
}

op = PyTuple_GET_ITEM(args, 0);
if (PyArray_CheckExact(op)) {
PyObject * dtype_obj = Py_None;
oparr = (PyArrayObject *)op;
PyObject *dtype_obj = Py_None;
PyArrayObject *oparr = (PyArrayObject *)op;

/* get dtype which can be positional */
if (PyTuple_GET_SIZE(args) == 2) {
dtype_obj = PyTuple_GET_ITEM(args, 1);
Expand All @@ -1643,16 +1645,23 @@ _array_fromobject(PyObject *NPY_UNUSED(ignored), PyObject *args, PyObject *kws)
}
else {
/* fast path for copy=False rest default (np.asarray) */
PyObject * copy_obj, * order_obj, *ndmin_obj;
PyObject *copy_obj, *order_obj, *ndmin_obj;

/* Always copy arrays with NPY_ARRAY_UPDATEIFCOPY set */
if (PyArray_CHKFLAGS(oparr, NPY_ARRAY_UPDATEIFCOPY)) {
goto full_path;
}

copy_obj = PyDict_GetItem(kws, npy_ma_str_copy);
if (copy_obj != Py_False) {
goto full_path;
}

copy = NPY_FALSE;

/* order does not matter for contiguous 1d arrays */
if (PyArray_NDIM((PyArrayObject*)op) > 1 ||
!PyArray_IS_C_CONTIGUOUS((PyArrayObject*)op)) {
if (PyArray_NDIM(oparr) > 1 ||
!PyArray_IS_C_CONTIGUOUS(oparr)) {
order_obj = PyDict_GetItem(kws, npy_ma_str_order);
if (order_obj != Py_None && order_obj != NULL) {
goto full_path;
Expand Down Expand Up @@ -1696,10 +1705,18 @@ _array_fromobject(PyObject *NPY_UNUSED(ignored), PyObject *args, PyObject *kws)
"NPY_MAXDIMS (=%d)", NPY_MAXDIMS);
goto clean_type;
}


/* fast exit if simple call */
if ((subok && PyArray_Check(op)) ||
(!subok && PyArray_CheckExact(op))) {
oparr = (PyArrayObject *)op;
(!subok && PyArray_CheckExact(op))) {
PyArrayObject *oparr = (PyArrayObject *)op;

/* Always copy arrays with NPY_ARRAY_UPDATEIFCOPY set */
if (PyArray_CHKFLAGS(oparr, NPY_ARRAY_UPDATEIFCOPY)) {
copy = NPY_TRUE;
}

if (type == NULL) {
if (!copy && STRIDING_OK(oparr, order)) {
ret = oparr;
Expand All @@ -1715,8 +1732,8 @@ _array_fromobject(PyObject *NPY_UNUSED(ignored), PyObject *args, PyObject *kws)
oldtype = PyArray_DESCR(oparr);
if (PyArray_EquivTypes(oldtype, type)) {
if (!copy && STRIDING_OK(oparr, order)) {
Py_INCREF(op);
ret = oparr;
Py_INCREF(ret);
goto finish;
}
else {
Expand Down
17 changes: 17 additions & 0 deletions numpy/core/tests/test_multiarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,23 @@ def test_array_cont(self):
assert_(np.ascontiguousarray(d).flags.c_contiguous)
assert_(np.asfortranarray(d).flags.f_contiguous)

def test_from_updateifcopy_array(self):
# a is not contiguous, so a.flat.__array__() will have the
# NPY_ARRAY_UPDATEIFCOPY flag set. We want to make sure that
# normal array construction always makes a copy of that rather
# than returning it.
a = np.arange(10)[::2]
testcases = [np.array(a.flat, copy=True),
np.array(a.flat, copy=False),
np.array(a.flat.__array__(), copy=True),
np.array(a.flat.__array__(), copy=False)]
for b in testcases:
self.assertTrue(b.flags.owndata)
self.assertTrue(b.flags.writeable)
self.assertFalse(b.flags.updateifcopy)
# check that writeback occurred, hence a is writeable
self.assertTrue(a.flags.writeable)


class TestAssignment(TestCase):
def test_assignment_broadcasting(self):
Expand Down