From 60adb1ac9fe688184973a02e8bce552d70d087f4 Mon Sep 17 00:00:00 2001 From: Charles Harris Date: Mon, 7 Mar 2011 18:24:35 -0700 Subject: BUG: Preliminary fix for ticket #1757. --- numpy/core/src/multiarray/ctors.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'numpy/core') diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c index 4fb6bc781..a98116afe 100644 --- a/numpy/core/src/multiarray/ctors.c +++ b/numpy/core/src/multiarray/ctors.c @@ -822,22 +822,38 @@ discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, npy_intp dtmp[NPY_MAXDIMS]; int j, maxndim_m1 = *maxndim - 1; - e = PySequence_GetItem(s, 0); + if ((e = PySequence_GetItem(s, 0)) == NULL) { + /* not a list */ + *maxndim = 0; + PyErr_Clear(); + return 0; + } r = discover_dimensions(e, &maxndim_m1, d + 1, check_it, stop_at_string, stop_at_tuple, out_is_object); Py_DECREF(e); + if (r < 0) { + return r; + } /* For the dimension truncation check below */ *maxndim = maxndim_m1 + 1; for (i = 1; i < n; ++i) { /* Get the dimensions of the first item */ - e = PySequence_GetItem(s, i); + if ((e = PySequence_GetItem(s, 0)) == NULL) { + /* not a list */ + *maxndim = 0; + PyErr_Clear(); + return 0; + } r = discover_dimensions(e, &maxndim_m1, dtmp, check_it, stop_at_string, stop_at_tuple, out_is_object); Py_DECREF(e); + if (r < 0) { + return r; + } /* Reduce max_ndim_m1 to just items which match */ for (j = 0; j < maxndim_m1; ++j) { -- cgit v1.2.1 From 9e845494355a7bb6c6548f0f48fa83485999a460 Mon Sep 17 00:00:00 2001 From: Charles Harris Date: Mon, 7 Mar 2011 22:29:29 -0700 Subject: Make a failed sequence access error set the object creation flag. --- numpy/core/src/multiarray/ctors.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'numpy/core') diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c index a98116afe..f41ab1a4a 100644 --- a/numpy/core/src/multiarray/ctors.c +++ b/numpy/core/src/multiarray/ctors.c @@ -825,6 +825,7 @@ discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, if ((e = PySequence_GetItem(s, 0)) == NULL) { /* not a list */ *maxndim = 0; + *out_is_object = 1; PyErr_Clear(); return 0; } @@ -844,6 +845,7 @@ discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, if ((e = PySequence_GetItem(s, 0)) == NULL) { /* not a list */ *maxndim = 0; + *out_is_object = 1; PyErr_Clear(); return 0; } -- cgit v1.2.1 From c3f9989fe078fec44671172680e39e100d2a843f Mon Sep 17 00:00:00 2001 From: Charles Harris Date: Mon, 7 Mar 2011 22:42:40 -0700 Subject: ENH: Make comment more informative. --- numpy/core/src/multiarray/ctors.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'numpy/core') diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c index f41ab1a4a..e0d698781 100644 --- a/numpy/core/src/multiarray/ctors.c +++ b/numpy/core/src/multiarray/ctors.c @@ -823,7 +823,13 @@ discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, int j, maxndim_m1 = *maxndim - 1; if ((e = PySequence_GetItem(s, 0)) == NULL) { - /* not a list */ + /* + * This should probably be an error but we suppress + * it to maintain backwards compatibility with pandas + * and possibly other existing applications. The making + * of the object type instead of a straght return might + * cause other errors not to occur. + */ *maxndim = 0; *out_is_object = 1; PyErr_Clear(); @@ -843,7 +849,7 @@ discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, for (i = 1; i < n; ++i) { /* Get the dimensions of the first item */ if ((e = PySequence_GetItem(s, 0)) == NULL) { - /* not a list */ + /* see comment above */ *maxndim = 0; *out_is_object = 1; PyErr_Clear(); -- cgit v1.2.1 From 90e4cf055c8b469634cdb6c84e0a8843c2212e78 Mon Sep 17 00:00:00 2001 From: Charles Harris Date: Tue, 8 Mar 2011 09:11:21 -0700 Subject: TST: Add test for robustness with non-sequence detected as sequence. --- numpy/core/tests/test_multiarray.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'numpy/core') diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index f1165999e..edb64cd61 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -302,6 +302,20 @@ class TestCreation(TestCase): msg = 'String conversion for %s' % type assert_equal(array(nstr, dtype=type), result, err_msg=msg) + def test_non_sequence_sequence(self): + """Should not segfault.""" + class x(object): + def __len__(self): + return 1 + + def __getitem__(self): + raise ValueError('hi there') + + a = np.array([x()]) + assert_(a.shape == (1,)) + assert_(a.dtype == np.dtype(object)) + + class TestStructured(TestCase): def test_subarray_field_access(self): a = np.zeros((3, 5), dtype=[('a', ('i4', (2, 2)))]) -- cgit v1.2.1 From cef500872547ba3f06f42a8669c7c72382451822 Mon Sep 17 00:00:00 2001 From: Charles Harris Date: Tue, 8 Mar 2011 10:21:25 -0700 Subject: DOC: Improve the content of a comment. --- numpy/core/src/multiarray/ctors.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'numpy/core') diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c index e0d698781..e2af0dab4 100644 --- a/numpy/core/src/multiarray/ctors.c +++ b/numpy/core/src/multiarray/ctors.c @@ -824,11 +824,13 @@ discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, if ((e = PySequence_GetItem(s, 0)) == NULL) { /* - * This should probably be an error but we suppress - * it to maintain backwards compatibility with pandas - * and possibly other existing applications. The making - * of the object type instead of a straght return might - * cause other errors not to occur. + * PySequence_Check looks for the presence of the __getitem__ + * attribute to detect a sequence. Consequently, dictionaries + * and other objects that implement that method may show up + * here and the PySequence_GetItem call can fail in those + * cases. Consequently we truncate the dimensions and set the + * object creation flag. Raising an error is another option but + * it might break backward compatibility. */ *maxndim = 0; *out_is_object = 1; -- cgit v1.2.1 From 891bc2e8cebc3259f83033eceb2287c9f8eaa555 Mon Sep 17 00:00:00 2001 From: Charles Harris Date: Tue, 8 Mar 2011 17:44:12 -0700 Subject: BUG: Fix cut and paste error. --- numpy/core/src/multiarray/ctors.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) (limited to 'numpy/core') diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c index e2af0dab4..cb88690a4 100644 --- a/numpy/core/src/multiarray/ctors.c +++ b/numpy/core/src/multiarray/ctors.c @@ -647,6 +647,7 @@ discover_itemsize(PyObject *s, int nd, int *itemsize) * Take an arbitrary object and discover how many dimensions it * has, filling in the dimensions as we go. */ +#include static int discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, int stop_at_string, int stop_at_tuple, @@ -688,8 +689,8 @@ discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, PyInstance_Check(s) || #endif PySequence_Length(s) < 0) { - PyErr_Clear(); *maxndim = 0; + PyErr_Clear(); return 0; } @@ -824,13 +825,17 @@ discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, if ((e = PySequence_GetItem(s, 0)) == NULL) { /* - * PySequence_Check looks for the presence of the __getitem__ - * attribute to detect a sequence. Consequently, dictionaries - * and other objects that implement that method may show up - * here and the PySequence_GetItem call can fail in those - * cases. Consequently we truncate the dimensions and set the - * object creation flag. Raising an error is another option but - * it might break backward compatibility. + * PySequence_Check detects whether an old type object is a + * sequence by the presence of the __getitem__ attribute, and + * for new type objects that aren't dictionaries by the + * presence of the __len__ attribute as well. In either case it + * is possible to have an object that tests as a sequence but + * doesn't behave as a sequence and consequently, the + * PySequence_GetItem call can fail. When that happens we + * truncate the dimensions and set the object creation flag. + * Raising an error is another option but it might break + * backward compatibility. We could probably be a bit stricter + * here for Python 3k because there are no old type objects. */ *maxndim = 0; *out_is_object = 1; @@ -847,10 +852,9 @@ discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, /* For the dimension truncation check below */ *maxndim = maxndim_m1 + 1; - for (i = 1; i < n; ++i) { /* Get the dimensions of the first item */ - if ((e = PySequence_GetItem(s, 0)) == NULL) { + if ((e = PySequence_GetItem(s, i)) == NULL) { /* see comment above */ *maxndim = 0; *out_is_object = 1; -- cgit v1.2.1 From ea403b5c993f5c897480d8bb273501fc30be55f8 Mon Sep 17 00:00:00 2001 From: Charles Harris Date: Tue, 8 Mar 2011 17:56:00 -0700 Subject: DOC: Make clear that class in test breaks the sequence protocol. --- numpy/core/tests/test_multiarray.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'numpy/core') diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index edb64cd61..0c2d55ae6 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -303,15 +303,21 @@ class TestCreation(TestCase): assert_equal(array(nstr, dtype=type), result, err_msg=msg) def test_non_sequence_sequence(self): - """Should not segfault.""" - class x(object): + """Should not segfault. + + Class Foo breaks the sequence protocol for new style classes, i.e., + those derived from object. At some point we may raise a warning in + this case. + + """ + class Foo(object): def __len__(self): return 1 def __getitem__(self): raise ValueError('hi there') - a = np.array([x()]) + a = np.array([Foo()]) assert_(a.shape == (1,)) assert_(a.dtype == np.dtype(object)) -- cgit v1.2.1 From 3e135e222e006cb039f41a0d1af2656d59c2dbb1 Mon Sep 17 00:00:00 2001 From: Charles Harris Date: Tue, 8 Mar 2011 18:12:16 -0700 Subject: STY: Remove uneeded stdio.h include. --- numpy/core/src/multiarray/ctors.c | 1 - 1 file changed, 1 deletion(-) (limited to 'numpy/core') diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c index cb88690a4..1da1027ad 100644 --- a/numpy/core/src/multiarray/ctors.c +++ b/numpy/core/src/multiarray/ctors.c @@ -647,7 +647,6 @@ discover_itemsize(PyObject *s, int nd, int *itemsize) * Take an arbitrary object and discover how many dimensions it * has, filling in the dimensions as we go. */ -#include static int discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, int stop_at_string, int stop_at_tuple, -- cgit v1.2.1 From 4aaa4b7e500514a3bc37b515445592d17dce65b1 Mon Sep 17 00:00:00 2001 From: Charles Harris Date: Tue, 8 Mar 2011 20:09:38 -0700 Subject: ENH: Make discover_dimensions pass thru all errors in list/mapping processing except KeyError. In the latter case an object is forced. --- numpy/core/src/multiarray/ctors.c | 41 +++++++++++++++++++++---------------- numpy/core/tests/test_multiarray.py | 23 ++++++++++++++------- 2 files changed, 39 insertions(+), 25 deletions(-) (limited to 'numpy/core') diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c index 1da1027ad..94602a5c9 100644 --- a/numpy/core/src/multiarray/ctors.c +++ b/numpy/core/src/multiarray/ctors.c @@ -830,16 +830,20 @@ discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, * presence of the __len__ attribute as well. In either case it * is possible to have an object that tests as a sequence but * doesn't behave as a sequence and consequently, the - * PySequence_GetItem call can fail. When that happens we - * truncate the dimensions and set the object creation flag. - * Raising an error is another option but it might break - * backward compatibility. We could probably be a bit stricter - * here for Python 3k because there are no old type objects. + * PySequence_GetItem call can fail. When that happens and the + * object looks like a dictionary, we truncate the dimensions + * and set the object creation flag, otherwise we pass the + * error back up the call chain. */ - *maxndim = 0; - *out_is_object = 1; - PyErr_Clear(); - return 0; + if (PyErr_ExceptionMatches(PyExc_KeyError)) { + PyErr_Clear(); + *maxndim = 0; + *out_is_object = 1; + return 0; + } + else { + return -1; + } } r = discover_dimensions(e, &maxndim_m1, d + 1, check_it, stop_at_string, stop_at_tuple, @@ -855,10 +859,15 @@ discover_dimensions(PyObject *s, int *maxndim, npy_intp *d, int check_it, /* Get the dimensions of the first item */ if ((e = PySequence_GetItem(s, i)) == NULL) { /* see comment above */ - *maxndim = 0; - *out_is_object = 1; - PyErr_Clear(); - return 0; + if (PyErr_ExceptionMatches(PyExc_KeyError)) { + PyErr_Clear(); + *maxndim = 0; + *out_is_object = 1; + return 0; + } + else { + return -1; + } } r = discover_dimensions(e, &maxndim_m1, dtmp, check_it, stop_at_string, stop_at_tuple, @@ -1540,13 +1549,9 @@ PyArray_GetArrayParamsFromObject(PyObject *op, stop_at_string, stop_at_tuple, &is_object) < 0) { Py_DECREF(*out_dtype); - if (PyErr_Occurred() && - PyErr_GivenExceptionMatches(PyErr_Occurred(), - PyExc_MemoryError)) { + if (PyErr_Occurred()) { return -1; } - /* Say it's an OBJECT scalar if there's an error */ - PyErr_Clear(); *out_dtype = PyArray_DescrFromType(NPY_OBJECT); if (*out_dtype == NULL) { return -1; diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index 0c2d55ae6..2f3575649 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -305,21 +305,30 @@ class TestCreation(TestCase): def test_non_sequence_sequence(self): """Should not segfault. - Class Foo breaks the sequence protocol for new style classes, i.e., - those derived from object. At some point we may raise a warning in - this case. + Class Fail breaks the sequence protocol for new style classes, i.e., + those derived from object. Class Map is a mapping type indicated by + raising a ValueError. At some point we may raise a warning instead + of an error in the Fail case. """ - class Foo(object): + class Fail(object): def __len__(self): return 1 - def __getitem__(self): - raise ValueError('hi there') + def __getitem__(self, index): + raise ValueError() - a = np.array([Foo()]) + class Map(object): + def __len__(self): + return 1 + + def __getitem__(self, index): + raise KeyError() + + a = np.array([Map()]) assert_(a.shape == (1,)) assert_(a.dtype == np.dtype(object)) + assert_raises(ValueError, np.array, [Fail()]) class TestStructured(TestCase): -- cgit v1.2.1