From 0addc016ba7000b27509663f4f489c6eb1838056 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Fri, 16 Feb 2018 21:38:23 -0800 Subject: [PATCH] DEP: Deprecate non-tuple multidimensional indices Currently, arr[[None, 0]] and arr[(None, 0)] mean the same thing, yet arr[[0, 0]] and arr[(0, 0)] mean different things. This makes it super hard to make a subclass or duck array that behaves consistently with ndarray. By deprecating this feature, we force downstream library code to stop using it, which in turn makes that library code use approaches that are easier to implement in subclasses and duck types. --- doc/release/1.15.0-notes.rst | 6 ++++++ doc/source/reference/arrays.indexing.rst | 17 +++++++++++------ numpy/core/src/multiarray/mapping.c | 17 +++++++++++++++-- numpy/core/tests/test_deprecations.py | 16 ++++++++++++++++ 4 files changed, 48 insertions(+), 8 deletions(-) diff --git a/doc/release/1.15.0-notes.rst b/doc/release/1.15.0-notes.rst index 0b1408d1fc39..b97e2d8392b7 100644 --- a/doc/release/1.15.0-notes.rst +++ b/doc/release/1.15.0-notes.rst @@ -36,6 +36,12 @@ Deprecations * `np.ma.load`, `np.ma.dump` - these functions already failed on python 3, when called with a string. +* Multidimensional indexing with anything but a tuple is + deprecated. This means that code such as ``ind = [slice(None), 0]``, + ``arr[[slice(None), 0]]`` should be changed to ``arr[tuple(ind)]``. This is + necessary to avoid ambiguity in expressions such as ``arr[[[0, 1], [0, 1]]]`` + which currently is interpreted as ``arr[array([0, 1]), array([0, 1])]``. + In future, this will be interpreted as ``arr[array([[0, 1], [0, 1]])]``. Future Changes ============== diff --git a/doc/source/reference/arrays.indexing.rst b/doc/source/reference/arrays.indexing.rst index b5a44c22a410..ba1bfd31210b 100644 --- a/doc/source/reference/arrays.indexing.rst +++ b/doc/source/reference/arrays.indexing.rst @@ -29,11 +29,15 @@ dimensions. Basic slicing occurs when *obj* is a :class:`slice` object (constructed by ``start:stop:step`` notation inside of brackets), an integer, or a tuple of slice objects and integers. :const:`Ellipsis` and :const:`newaxis` objects can be interspersed with these as -well. In order to remain backward compatible with a common usage in -Numeric, basic slicing is also initiated if the selection object is -any non-ndarray sequence (such as a :class:`list`) containing :class:`slice` -objects, the :const:`Ellipsis` object, or the :const:`newaxis` object, -but not for integer arrays or other embedded sequences. +well. + +.. deprecated:: 1.15.0 + + In order to remain backward compatible with a common usage in + Numeric, basic slicing is also initiated if the selection object is + any non-ndarray and non-tuple sequence (such as a :class:`list`) containing + :class:`slice` objects, the :const:`Ellipsis` object, or the :const:`newaxis` + object, but not for integer arrays or other embedded sequences. .. index:: triple: ndarray; special methods; getitem @@ -196,7 +200,8 @@ basic slicing that returns a :term:`view`). why this occurs. Also recognize that ``x[[1,2,3]]`` will trigger advanced indexing, - whereas ``x[[1,2,slice(None)]]`` will trigger basic slicing. + whereas due to the deprecated Numeric compatibility mentioned above, + ``x[[1,2,slice(None)]]`` will trigger basic slicing. Integer array indexing ^^^^^^^^^^^^^^^^^^^^^^ diff --git a/numpy/core/src/multiarray/mapping.c b/numpy/core/src/multiarray/mapping.c index eca4e98bec0a..0bfc019b38db 100644 --- a/numpy/core/src/multiarray/mapping.c +++ b/numpy/core/src/multiarray/mapping.c @@ -293,8 +293,7 @@ unpack_indices(PyObject *index, PyObject **result, npy_intp result_n) if (commit_to_unpack) { /* propagate errors */ if (tmp_obj == NULL) { - multi_DECREF(result, i); - return -1; + goto fail; } } else { @@ -313,6 +312,16 @@ unpack_indices(PyObject *index, PyObject **result, npy_intp result_n) || PySlice_Check(tmp_obj) || tmp_obj == Py_Ellipsis || tmp_obj == Py_None) { + if (DEPRECATE_FUTUREWARNING( + "Using a non-tuple sequence for multidimensional " + "indexing is deprecated; use `arr[tuple(seq)]` " + "instead of `arr[seq]`. In the future this will be " + "interpreted as an array index, `arr[np.array(seq)]`, " + "which will result either in an error or a different " + "result.") < 0) { + i++; /* since loop update doesn't run */ + goto fail; + } commit_to_unpack = 1; } } @@ -328,6 +337,10 @@ unpack_indices(PyObject *index, PyObject **result, npy_intp result_n) multi_DECREF(result, i); return unpack_scalar(index, result, result_n); } + +fail: + multi_DECREF(result, i); + return -1; } /** diff --git a/numpy/core/tests/test_deprecations.py b/numpy/core/tests/test_deprecations.py index fe0c7cc5f73d..21ea7f1ccad8 100644 --- a/numpy/core/tests/test_deprecations.py +++ b/numpy/core/tests/test_deprecations.py @@ -132,6 +132,22 @@ class _VisibleDeprecationTestCase(_DeprecationTestCase): warning_cls = np.VisibleDeprecationWarning +class TestNonTupleNDIndexDeprecation(object): + def test_basic(self): + a = np.zeros((5, 5)) + with warnings.catch_warnings(): + warnings.filterwarnings('always') + assert_warns(FutureWarning, a.__getitem__, [[0, 1], [0, 1]]) + assert_warns(FutureWarning, a.__getitem__, [slice(None)]) + + warnings.filterwarnings('error') + assert_raises(FutureWarning, a.__getitem__, [[0, 1], [0, 1]]) + assert_raises(FutureWarning, a.__getitem__, [slice(None)]) + + # a a[[0, 1]] always was advanced indexing, so no error/warning + a[[0, 1]] + + class TestRankDeprecation(_DeprecationTestCase): """Test that np.rank is deprecated. The function should simply be removed. The VisibleDeprecationWarning may become unnecessary.