Skip to content

Commit

Permalink
Merge pull request numpy#3645 from charris/enable-diagonal-as-view
Browse files Browse the repository at this point in the history 
ENH: Make the ndarray diagonal method return a view.
  • Loading branch information
@charris
charris committed Aug 22, 2013
2 parents 9464075 + fd6cfd6 commit 202e78d
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 134 deletions.
4 changes: 3 additions & 1 deletion numpy/add_newdocs.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3316,7 +3316,9 @@ def luf(lamdaexpr, *args, **kwargs):
"""
a.diagonal(offset=0, axis1=0, axis2=1)
Return specified diagonals.
Return specified diagonals. In NumPy 1.9 the returned array is a
read-only view instead of a copy as in previous NumPy versions. In
NumPy 1.10 the read-only restriction will be removed.
Refer to :func:`numpy.diagonal` for full documentation.
Expand Down
15 changes: 7 additions & 8 deletions numpy/core/fromnumeric.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1125,16 +1125,15 @@ def diagonal(a, offset=0, axis1=0, axis2=1):
In versions of NumPy prior to 1.7, this function always returned a new,
independent array containing a copy of the values in the diagonal.
In NumPy 1.7, it continues to return a copy of the diagonal, but depending
on this fact is deprecated. Writing to the resulting array continues to
work as it used to, but a FutureWarning will be issued.
In NumPy 1.7 and 1.8, it continues to return a copy of the diagonal,
but depending on this fact is deprecated. Writing to the resulting
array continues to work as it used to, but a FutureWarning is issued.
In NumPy 1.9, it will switch to returning a read-only view on the original
array. Attempting to write to the resulting array will produce an error.
In NumPy 1.9 it returns a read-only view on the original array.
Attempting to write to the resulting array will produce an error.
In NumPy 1.10, it will still return a view, but this view will no longer be
marked read-only. Writing to the returned array will alter your original
array as well.
In NumPy 1.10, it will return a read/write view, Writing to the returned
array will alter your original array.
If you don't write to the array returned by this function, then you can
just ignore all of the above.
Expand Down
15 changes: 7 additions & 8 deletions numpy/core/src/multiarray/item_selection.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2341,14 +2341,13 @@ PyArray_Diagonal(PyArrayObject *self, int offset, int axis1, int axis2)
return NULL;
}

/* For backwards compatibility, during the deprecation period: */
copy = PyArray_NewCopy(ret, NPY_KEEPORDER);
Py_DECREF(ret);
if (!copy) {
return NULL;
}
PyArray_ENABLEFLAGS((PyArrayObject *)copy, NPY_ARRAY_WARN_ON_WRITE);
return copy;
/*
* For numpy 1.9 the diagonal view is not writeable.
* This line needs to be removed in 1.10.
*/
PyArray_CLEARFLAGS(ret, NPY_ARRAY_WRITEABLE);

return ret;
}

/*NUMPY_API
Expand Down
131 changes: 14 additions & 117 deletions numpy/core/tests/test_multiarray.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1343,123 +1343,20 @@ def test_diagonal(self):
# Order of axis argument doesn't matter:
assert_equal(b.diagonal(0, 2, 1), [[0, 3], [4, 7]])

def test_diagonal_deprecation(self):

def collect_warning_types(f, *args, **kwargs):
with warnings.catch_warnings(record=True) as log:
warnings.simplefilter("always")
f(*args, **kwargs)
return [w.category for w in log]

a = np.arange(9).reshape(3, 3)
# All the different functions raise a warning, but not an error, and
# 'a' is not modified:
assert_equal(collect_warning_types(a.diagonal().__setitem__, 0, 10),
[FutureWarning])
assert_equal(a, np.arange(9).reshape(3, 3))
assert_equal(collect_warning_types(np.diagonal(a).__setitem__, 0, 10),
[FutureWarning])
assert_equal(a, np.arange(9).reshape(3, 3))
assert_equal(collect_warning_types(np.diag(a).__setitem__, 0, 10),
[FutureWarning])
assert_equal(a, np.arange(9).reshape(3, 3))
# Views also warn
d = np.diagonal(a)
d_view = d.view()
assert_equal(collect_warning_types(d_view.__setitem__, 0, 10),
[FutureWarning])
# But the write goes through:
assert_equal(d[0], 10)
# Only one warning per call to diagonal, though (even if there are
# multiple views involved):
assert_equal(collect_warning_types(d.__setitem__, 0, 10),
[])

# Other ways of accessing the data also warn:
# .data goes via the C buffer API, gives a read-write
# buffer/memoryview. We don't warn until tp_getwritebuf is actually
# called, which is not until the buffer is written to.
have_memoryview = (hasattr(__builtins__, "memoryview")
or "memoryview" in __builtins__)
def get_data_and_write(getter):
buf_or_memoryview = getter(a.diagonal())
if (have_memoryview and isinstance(buf_or_memoryview, memoryview)):
buf_or_memoryview[0] = np.array(1)
else:
buf_or_memoryview[0] = "x"
assert_equal(collect_warning_types(get_data_and_write,
lambda d: d.data),
[FutureWarning])
if hasattr(np, "getbuffer"):
assert_equal(collect_warning_types(get_data_and_write,
np.getbuffer),
[FutureWarning])
# PEP 3118:
if have_memoryview:
assert_equal(collect_warning_types(get_data_and_write, memoryview),
[FutureWarning])
# Void dtypes can give us a read-write buffer, but only in Python 2:
import sys
if sys.version_info[0] < 3:
aV = np.empty((3, 3), dtype="V10")
assert_equal(collect_warning_types(aV.diagonal().item, 0),
[FutureWarning])
# XX it seems that direct indexing of a void object returns a void
# scalar, which ignores not just WARN_ON_WRITE but even WRITEABLE.
# i.e. in this:
# a = np.empty(10, dtype="V10")
# a.flags.writeable = False
# buf = a[0].item()
# 'buf' ends up as a writeable buffer. I guess no-one actually
# uses void types like this though...
# __array_interface also lets a data pointer get away from us
log = collect_warning_types(getattr, a.diagonal(),
"__array_interface__")
assert_equal(log, [FutureWarning])
# ctypeslib goes via __array_interface__:
try:
# may not exist in python 2.4:
import ctypes
except ImportError:
pass
else:
log = collect_warning_types(np.ctypeslib.as_ctypes, a.diagonal())
assert_equal(log, [FutureWarning])
# __array_struct__
log = collect_warning_types(getattr, a.diagonal(), "__array_struct__")
assert_equal(log, [FutureWarning])

# Make sure that our recommendation to silence the warning by copying
# the array actually works:
diag_copy = a.diagonal().copy()
assert_equal(collect_warning_types(diag_copy.__setitem__, 0, 10),
[])
# There might be people who get a spurious warning because they are
# extracting a buffer, but then use that buffer in a read-only
# fashion. And they might get cranky at having to create a superfluous
# copy just to work around this spurious warning. A reasonable
# solution would be for them to mark their usage as read-only, and
# thus safe for both past and future PyArray_Diagonal
# semantics. So let's make sure that setting the diagonal array to
# non-writeable will suppress these warnings:
ro_diag = a.diagonal()
ro_diag.flags.writeable = False
assert_equal(collect_warning_types(getattr, ro_diag, "data"), [])
# __array_interface__ has no way to communicate read-onlyness --
# effectively all __array_interface__ arrays are assumed to be
# writeable :-(
# ro_diag = a.diagonal()
# ro_diag.flags.writeable = False
# assert_equal(collect_warning_types(getattr, ro_diag,
# "__array_interface__"), [])
if hasattr(__builtins__, "memoryview"):
ro_diag = a.diagonal()
ro_diag.flags.writeable = False
assert_equal(collect_warning_types(memoryview, ro_diag), [])
ro_diag = a.diagonal()
ro_diag.flags.writeable = False
assert_equal(collect_warning_types(getattr, ro_diag,
"__array_struct__"), [])
def test_diagonal_view_notwriteable(self):
# this test is only for 1.9, the diagonal view will be
# writeable in 1.10.
a = np.eye(3).diagonal()
assert_(not a.flags.writeable)
assert_(not a.flags.owndata)

a = np.diagonal(np.eye(3))
assert_(not a.flags.writeable)
assert_(not a.flags.owndata)

a = np.diag(np.eye(3))
assert_(not a.flags.writeable)
assert_(not a.flags.owndata)

def test_diagonal_memleak(self):
# Regression test for a bug that crept in at one point
Expand Down

0 comments on commit 202e78d

Please sign in to comment.