From 088e20e272389395fb3fd24fed144ed19bae8cdb Mon Sep 17 00:00:00 2001 From: gfyoung Date: Fri, 11 Dec 2015 04:24:16 +0000 Subject: [PATCH] DEP: Stricter arg checking for array ordering The bug traces to the PyArray_OrderConverter method in conversion_utils.c, where no errors are thrown if the ORDER parameter passed in is not of the string data-type or has a string value of length greater than one. This commit causes a DeprecationWarning to be raised, which will later be turned into a TypeError or another type of error in a future release. Closes gh-6598. --- doc/release/1.11.0-notes.rst | 8 ++++++ numpy/add_newdocs.py | 12 +++++--- numpy/core/src/multiarray/conversion_utils.c | 21 ++++++++++++++ numpy/core/tests/test_deprecations.py | 30 ++++++++++++++++++++ numpy/lib/function_base.py | 10 ++++--- numpy/matrixlib/defmatrix.py | 14 +++++---- 6 files changed, 81 insertions(+), 14 deletions(-) diff --git a/doc/release/1.11.0-notes.rst b/doc/release/1.11.0-notes.rst index b3ddae604f27..7c078eed972f 100644 --- a/doc/release/1.11.0-notes.rst +++ b/doc/release/1.11.0-notes.rst @@ -133,3 +133,11 @@ c_contiguous arrays at some future time. A work around that is backward compatible is to use `a.T.view(...).T` instead. A parameter will also be added to the view method to explicitly ask for Fortran order views, but that will not be backward compatible. + +Invalid arguments for array ordering +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +It is currently possible to pass in arguments for the ```order``` +parameter in methods like ```array.flatten``` or ```array.ravel``` +that were not one of the following: 'C', 'F', 'A', 'K' (note that +all of these possible values are unicode- and case-insensitive). +Such behaviour will not be allowed in future releases. diff --git a/numpy/add_newdocs.py b/numpy/add_newdocs.py index c140360891e5..01ef24a5bf16 100644 --- a/numpy/add_newdocs.py +++ b/numpy/add_newdocs.py @@ -3567,10 +3567,14 @@ def luf(lamdaexpr, *args, **kwargs): Parameters ---------- - order : {'C', 'F', 'A'}, optional - Whether to flatten in row-major (C-style) or - column-major (Fortran-style) order or preserve the - C/Fortran ordering from `a`. The default is 'C'. + order : {'C', 'F', 'A', 'K'}, optional + 'C' means to flatten in row-major (C-style) order. + 'F' means to flatten in column-major (Fortran- + style) order. 'A' means to flatten in column-major + order if `a` is Fortran *contiguous* in memory, + row-major order otherwise. 'K' means to flatten + `a` in the order the elements occur in memory. + The default is 'C'. Returns ------- diff --git a/numpy/core/src/multiarray/conversion_utils.c b/numpy/core/src/multiarray/conversion_utils.c index 88064c1d6e2e..d7a61787591c 100644 --- a/numpy/core/src/multiarray/conversion_utils.c +++ b/numpy/core/src/multiarray/conversion_utils.c @@ -540,6 +540,15 @@ PyArray_OrderConverter(PyObject *object, NPY_ORDER *val) return ret; } else if (!PyBytes_Check(object) || PyBytes_GET_SIZE(object) < 1) { + /* 2015-12-14, 1.11 */ + int ret = DEPRECATE("Non-string object detected for " + "the array ordering. Please pass " + "in 'C', 'F', 'A', or 'K' instead"); + + if (ret < 0) { + return -1; + } + if (PyObject_IsTrue(object)) { *val = NPY_FORTRANORDER; } @@ -553,6 +562,18 @@ PyArray_OrderConverter(PyObject *object, NPY_ORDER *val) } else { str = PyBytes_AS_STRING(object); + if (strlen(str) != 1) { + /* 2015-12-14, 1.11 */ + int ret = DEPRECATE("Non length-one string passed " + "in for the array ordering. " + "Please pass in 'C', 'F', 'A', " + "or 'K' instead"); + + if (ret < 0) { + return -1; + } + } + if (str[0] == 'C' || str[0] == 'c') { *val = NPY_CORDER; } diff --git a/numpy/core/tests/test_deprecations.py b/numpy/core/tests/test_deprecations.py index f6dc3d842219..65ddc1e77387 100644 --- a/numpy/core/tests/test_deprecations.py +++ b/numpy/core/tests/test_deprecations.py @@ -400,6 +400,36 @@ def test_fortran_contiguous(self): self.assert_deprecated(np.ones((2,2)).T.view, args=(np.int8,)) +class TestInvalidOrderParameterInputForFlattenArrayDeprecation(_DeprecationTestCase): + """Invalid arguments to the ORDER parameter in array.flatten() should not be + allowed and should raise an error. However, in the interests of not breaking + code that may inadvertently pass invalid arguments to this parameter, a + DeprecationWarning will be issued instead for the time being to give developers + time to refactor relevant code. + """ + + def test_flatten_array_non_string_arg(self): + x = np.zeros((3, 5)) + self.message = ("Non-string object detected for " + "the array ordering. Please pass " + "in 'C', 'F', 'A', or 'K' instead") + self.assert_deprecated(x.flatten, args=(np.pi,)) + + def test_flatten_array_invalid_string_arg(self): + # Tests that a DeprecationWarning is raised + # when a string of length greater than one + # starting with "C", "F", "A", or "K" (case- + # and unicode-insensitive) is passed in for + # the ORDER parameter. Otherwise, a TypeError + # will be raised! + + x = np.zeros((3, 5)) + self.message = ("Non length-one string passed " + "in for the array ordering. Please " + "pass in 'C', 'F', 'A', or 'K' instead") + self.assert_deprecated(x.flatten, args=("FACK",)) + + class TestTestDeprecated(object): def test_assert_deprecated(self): test_case_instance = _DeprecationTestCase() diff --git a/numpy/lib/function_base.py b/numpy/lib/function_base.py index 3298789eeb6e..541ad079cae4 100644 --- a/numpy/lib/function_base.py +++ b/numpy/lib/function_base.py @@ -3966,6 +3966,7 @@ def delete(arr, obj, axis=None): arr = asarray(arr) ndim = arr.ndim + arrorder = 'F' if arr.flags.fnc else 'C' if axis is None: if ndim != 1: arr = arr.ravel() @@ -4003,7 +4004,7 @@ def delete(arr, obj, axis=None): stop = xr[0] + 1 newshape[axis] -= numtodel - new = empty(newshape, arr.dtype, arr.flags.fnc) + new = empty(newshape, arr.dtype, arrorder) # copy initial chunk if start == 0: pass @@ -4054,7 +4055,7 @@ def delete(arr, obj, axis=None): if (obj < 0): obj += N newshape[axis] -= 1 - new = empty(newshape, arr.dtype, arr.flags.fnc) + new = empty(newshape, arr.dtype, arrorder) slobj[axis] = slice(None, obj) new[slobj] = arr[slobj] slobj[axis] = slice(obj, None) @@ -4197,6 +4198,7 @@ def insert(arr, obj, values, axis=None): arr = asarray(arr) ndim = arr.ndim + arrorder = 'F' if arr.flags.fnc else 'C' if axis is None: if ndim != 1: arr = arr.ravel() @@ -4265,7 +4267,7 @@ def insert(arr, obj, values, axis=None): values = np.rollaxis(values, 0, (axis % values.ndim) + 1) numnew = values.shape[axis] newshape[axis] += numnew - new = empty(newshape, arr.dtype, arr.flags.fnc) + new = empty(newshape, arr.dtype, arrorder) slobj[axis] = slice(None, index) new[slobj] = arr[slobj] slobj[axis] = slice(index, index+numnew) @@ -4298,7 +4300,7 @@ def insert(arr, obj, values, axis=None): old_mask = ones(newshape[axis], dtype=bool) old_mask[indices] = False - new = empty(newshape, arr.dtype, arr.flags.fnc) + new = empty(newshape, arr.dtype, arrorder) slobj2 = [slice(None)]*ndim slobj[axis] = indices slobj2[axis] = old_mask diff --git a/numpy/matrixlib/defmatrix.py b/numpy/matrixlib/defmatrix.py index ffd4578ba142..170db87c8d51 100644 --- a/numpy/matrixlib/defmatrix.py +++ b/numpy/matrixlib/defmatrix.py @@ -277,9 +277,9 @@ def __new__(subtype, data, dtype=None, copy=True): elif ndim == 1: shape = (1, shape[0]) - order = False + order = 'C' if (ndim == 2) and arr.flags.fortran: - order = True + order = 'F' if not (order or arr.flags.contiguous): arr = arr.copy() @@ -519,10 +519,12 @@ def flatten(self, order='C'): Parameters ---------- - order : {'C', 'F', 'A'}, optional - Whether to flatten in C (row-major), Fortran (column-major) order, - or preserve the C/Fortran ordering from `m`. - The default is 'C'. + order : {'C', 'F', 'A', 'K'}, optional + 'C' means to flatten in row-major (C-style) order. 'F' means to + flatten in column-major (Fortran-style) order. 'A' means to + flatten in column-major order if `m` is Fortran *contiguous* in + memory, row-major order otherwise. 'K' means to flatten `m` in + the order the elements occur in memory. The default is 'C'. Returns -------