From e6b1745f33ae2e5da977327bd72637d034d94d27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20K=2E=20Sepp=C3=A4nen?= Date: Sun, 20 Sep 2015 18:13:06 +0300 Subject: [PATCH 1/8] Return error from array_view for numpy scalars Unless ND==0, in which case return success but don't clobber the m_shape and m_strides members. --- src/numpy_cpp.h | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/src/numpy_cpp.h b/src/numpy_cpp.h index 10ae68c328a5..cc9d7b9a7c04 100644 --- a/src/numpy_cpp.h +++ b/src/numpy_cpp.h @@ -443,13 +443,17 @@ class array_view : public detail::array_view_accessors m_data = NULL; m_shape = zeros; m_strides = zeros; - } else if (PyArray_NDIM(tmp) != ND) { - PyErr_Format(PyExc_ValueError, - "Expected %d-dimensional array, got %d", - ND, - PyArray_NDIM(tmp)); - Py_DECREF(tmp); - return 0; + if (PyArray_NDIM(tmp) == 0 && ND == 0) { + return 1; + } + } + if (PyArray_NDIM(tmp) != ND) { + PyErr_Format(PyExc_ValueError, + "Expected %d-dimensional array, got %d", + ND, + PyArray_NDIM(tmp)); + Py_DECREF(tmp); + return 0; } /* Copy some of the data to the view object for faster access */ From 724cb9b2d17731c390839ef058c37b57412f4cd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20K=2E=20Sepp=C3=A4nen?= Date: Sun, 20 Sep 2015 18:14:18 +0300 Subject: [PATCH 2/8] Catch errors in the 1d branch of Py_affine_transform --- src/_path_wrapper.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/_path_wrapper.cpp b/src/_path_wrapper.cpp index a62893077c86..cd80e636dc3d 100644 --- a/src/_path_wrapper.cpp +++ b/src/_path_wrapper.cpp @@ -439,11 +439,16 @@ static PyObject *Py_affine_transform(PyObject *self, PyObject *args, PyObject *k CALL_CPP("affine_transform", (affine_transform_2d(vertices, trans, result))); return result.pyobj(); } catch (py::exception) { - numpy::array_view vertices(vertices_obj); - npy_intp dims[] = { vertices.dim(0) }; - numpy::array_view result(dims); - CALL_CPP("affine_transform", (affine_transform_1d(vertices, trans, result))); - return result.pyobj(); + PyErr_Clear(); + try { + numpy::array_view vertices(vertices_obj); + npy_intp dims[] = { vertices.dim(0) }; + numpy::array_view result(dims); + CALL_CPP("affine_transform", (affine_transform_1d(vertices, trans, result))); + return result.pyobj(); + } catch (py::exception) { + return NULL; + } } } From 2f29bdfd1de59c695f5bbeedcd722e5c086452ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20K=2E=20Sepp=C3=A4nen?= Date: Sun, 20 Sep 2015 18:37:36 +0300 Subject: [PATCH 3/8] Accept dimension mismatch for empty arrays --- src/numpy_cpp.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/numpy_cpp.h b/src/numpy_cpp.h index cc9d7b9a7c04..71c8606f1e59 100644 --- a/src/numpy_cpp.h +++ b/src/numpy_cpp.h @@ -447,6 +447,10 @@ class array_view : public detail::array_view_accessors return 1; } } + if (PyArray_NDIM(tmp) > 0 && PyArray_DIM(tmp, 0) == 0) { + // accept dimension mismatch for empty arrays + return 1; + } if (PyArray_NDIM(tmp) != ND) { PyErr_Format(PyExc_ValueError, "Expected %d-dimensional array, got %d", From c0e09d5e96b0293f4296f4a36b896f00a16a9612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20K=2E=20Sepp=C3=A4nen?= Date: Mon, 21 Sep 2015 21:09:28 +0300 Subject: [PATCH 4/8] Assign m_arr = tmp in more cases --- src/numpy_cpp.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/numpy_cpp.h b/src/numpy_cpp.h index 71c8606f1e59..d17e70504859 100644 --- a/src/numpy_cpp.h +++ b/src/numpy_cpp.h @@ -444,11 +444,14 @@ class array_view : public detail::array_view_accessors m_shape = zeros; m_strides = zeros; if (PyArray_NDIM(tmp) == 0 && ND == 0) { + m_arr = tmp; return 1; } } if (PyArray_NDIM(tmp) > 0 && PyArray_DIM(tmp, 0) == 0) { // accept dimension mismatch for empty arrays + Py_XDECREF(m_arr); + m_arr = tmp; return 1; } if (PyArray_NDIM(tmp) != ND) { From 536d175995bbef9fc9576b7055cd4d3afd8d581a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20K=2E=20Sepp=C3=A4nen?= Date: Sun, 27 Sep 2015 09:24:39 +0300 Subject: [PATCH 5/8] Don't allow shape mismatches for empty arrays Instead change callers to have empty arrays of the right shape. --- lib/matplotlib/cbook.py | 15 +++++++++++++++ lib/matplotlib/collections.py | 6 ++---- lib/matplotlib/path.py | 5 +++-- lib/matplotlib/tests/test_cbook.py | 8 ++++++++ lib/matplotlib/transforms.py | 4 +++- src/numpy_cpp.h | 6 ------ 6 files changed, 31 insertions(+), 13 deletions(-) diff --git a/lib/matplotlib/cbook.py b/lib/matplotlib/cbook.py index bc7e4df568b4..6451a908baa6 100644 --- a/lib/matplotlib/cbook.py +++ b/lib/matplotlib/cbook.py @@ -2244,6 +2244,21 @@ def _reshape_2D(X): return X +def ensure_3d(arr): + """ + Return a version of arr with ndim==3, with extra dimensions added + at the end of arr.shape as needed. + """ + arr = np.asanyarray(arr) + if arr.ndim == 1: + arr = arr[:, None, None] + elif arr.ndim == 2: + arr = arr[:, :, None] + elif arr.ndim > 3 or arr.ndim < 1: + raise ValueError("cannot convert arr to 3-dimensional") + return arr + + def violin_stats(X, method, points=100): ''' Returns a list of dictionaries of data which can be used to draw a series diff --git a/lib/matplotlib/collections.py b/lib/matplotlib/collections.py index dc48215dee43..7ca2b4619dde 100644 --- a/lib/matplotlib/collections.py +++ b/lib/matplotlib/collections.py @@ -80,9 +80,7 @@ class Collection(artist.Artist, cm.ScalarMappable): # _offsets must be a Nx2 array! _offsets.shape = (0, 2) _transOffset = transforms.IdentityTransform() - _transforms = [] - - + _transforms = np.empty((0, 3, 3)) def __init__(self, edgecolors=None, @@ -1515,7 +1513,7 @@ def __init__(self, widths, heights, angles, units='points', **kwargs): self._angles = np.asarray(angles).ravel() * (np.pi / 180.0) self._units = units self.set_transform(transforms.IdentityTransform()) - self._transforms = [] + self._transforms = np.empty((0, 3, 3)) self._paths = [mpath.Path.unit_circle()] def _set_transforms(self): diff --git a/lib/matplotlib/path.py b/lib/matplotlib/path.py index 3c5a291edf1c..ca80f4cbb6b7 100644 --- a/lib/matplotlib/path.py +++ b/lib/matplotlib/path.py @@ -24,7 +24,7 @@ from numpy import ma from matplotlib import _path -from matplotlib.cbook import simple_linear_interpolation, maxdict +from matplotlib.cbook import simple_linear_interpolation, maxdict, ensure_3d from matplotlib import rcParams @@ -988,7 +988,8 @@ def get_path_collection_extents( if len(paths) == 0: raise ValueError("No paths provided") return Bbox.from_extents(*_path.get_path_collection_extents( - master_transform, paths, transforms, offsets, offset_transform)) + master_transform, paths, ensure_3d(transforms), + offsets, offset_transform)) def get_paths_extents(paths, transforms=[]): diff --git a/lib/matplotlib/tests/test_cbook.py b/lib/matplotlib/tests/test_cbook.py index 2b916b08566f..b244e761cf8f 100644 --- a/lib/matplotlib/tests/test_cbook.py +++ b/lib/matplotlib/tests/test_cbook.py @@ -376,3 +376,11 @@ def test_step_fails(): np.arange(12)) assert_raises(ValueError, cbook._step_validation, np.arange(12), np.arange(3)) + + +def test_ensure_3d(): + assert_array_equal([[[1]], [[2]], [[3]]], + cbook.ensure_3d([1, 2, 3])) + assert_array_equal([[[1], [2]], [[3], [4]]], + cbook.ensure_3d([[1, 2], [3, 4]])) + assert_raises(ValueError, cbook.ensure_3d, [[[[1]]]]) diff --git a/lib/matplotlib/transforms.py b/lib/matplotlib/transforms.py index 0df6ea4c4b12..17ad25e2fad7 100644 --- a/lib/matplotlib/transforms.py +++ b/lib/matplotlib/transforms.py @@ -48,6 +48,7 @@ from sets import Set as set from .path import Path +from .cbook import ensure_3d DEBUG = False # we need this later, but this is very expensive to set up @@ -666,7 +667,8 @@ def count_overlaps(self, bboxes): bboxes is a sequence of :class:`BboxBase` objects """ - return count_bboxes_overlapping_bbox(self, [np.array(x) for x in bboxes]) + return count_bboxes_overlapping_bbox( + self, ensure_3d([np.array(x) for x in bboxes])) def expanded(self, sw, sh): """ diff --git a/src/numpy_cpp.h b/src/numpy_cpp.h index d17e70504859..c7cb14a74804 100644 --- a/src/numpy_cpp.h +++ b/src/numpy_cpp.h @@ -448,12 +448,6 @@ class array_view : public detail::array_view_accessors return 1; } } - if (PyArray_NDIM(tmp) > 0 && PyArray_DIM(tmp, 0) == 0) { - // accept dimension mismatch for empty arrays - Py_XDECREF(m_arr); - m_arr = tmp; - return 1; - } if (PyArray_NDIM(tmp) != ND) { PyErr_Format(PyExc_ValueError, "Expected %d-dimensional array, got %d", From 39658589d9bf352c05af9d9f05bf71fc17d10684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20K=2E=20Sepp=C3=A4nen?= Date: Sun, 27 Sep 2015 11:22:37 +0300 Subject: [PATCH 6/8] Add test --- lib/matplotlib/tests/test_transforms.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/matplotlib/tests/test_transforms.py b/lib/matplotlib/tests/test_transforms.py index 6d953de030ec..5190ddc873fe 100644 --- a/lib/matplotlib/tests/test_transforms.py +++ b/lib/matplotlib/tests/test_transforms.py @@ -561,6 +561,21 @@ def test_nonsingular(): assert_array_equal(out, zero_expansion) +def test_invalid_arguments(): + t = mtrans.Affine2D() + # There are two different exceptions, since the wrong number of + # dimensions is caught when constructing an array_view, and that + # raises a ValueError, and a wrong shape with a possible number + # of dimensions is caught by our CALL_CPP macro, which always + # raises the less precise RuntimeError. + assert_raises(ValueError, t.transform, 1) + assert_raises(ValueError, t.transform, [[[1]]]) + assert_raises(RuntimeError, t.transform, []) + assert_raises(RuntimeError, t.transform, [1]) + assert_raises(RuntimeError, t.transform, [[1]]) + assert_raises(RuntimeError, t.transform, [[1, 2, 3]]) + + if __name__ == '__main__': import nose nose.runmodule(argv=['-s', '--with-doctest'], exit=False) From 2f5a633b1ac469f9c958a065896d832d94db0883 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20K=2E=20Sepp=C3=A4nen?= Date: Mon, 28 Sep 2015 18:24:58 +0300 Subject: [PATCH 7/8] Use atleast_3d instead of ensure_3d It's not exactly the same, but we really use it just for empty arrays, so it shouldn't actually matter. --- lib/matplotlib/cbook.py | 15 --------------- lib/matplotlib/path.py | 4 ++-- lib/matplotlib/tests/test_cbook.py | 8 -------- lib/matplotlib/transforms.py | 3 +-- 4 files changed, 3 insertions(+), 27 deletions(-) diff --git a/lib/matplotlib/cbook.py b/lib/matplotlib/cbook.py index 6451a908baa6..bc7e4df568b4 100644 --- a/lib/matplotlib/cbook.py +++ b/lib/matplotlib/cbook.py @@ -2244,21 +2244,6 @@ def _reshape_2D(X): return X -def ensure_3d(arr): - """ - Return a version of arr with ndim==3, with extra dimensions added - at the end of arr.shape as needed. - """ - arr = np.asanyarray(arr) - if arr.ndim == 1: - arr = arr[:, None, None] - elif arr.ndim == 2: - arr = arr[:, :, None] - elif arr.ndim > 3 or arr.ndim < 1: - raise ValueError("cannot convert arr to 3-dimensional") - return arr - - def violin_stats(X, method, points=100): ''' Returns a list of dictionaries of data which can be used to draw a series diff --git a/lib/matplotlib/path.py b/lib/matplotlib/path.py index ca80f4cbb6b7..2e0d86456167 100644 --- a/lib/matplotlib/path.py +++ b/lib/matplotlib/path.py @@ -24,7 +24,7 @@ from numpy import ma from matplotlib import _path -from matplotlib.cbook import simple_linear_interpolation, maxdict, ensure_3d +from matplotlib.cbook import simple_linear_interpolation, maxdict from matplotlib import rcParams @@ -988,7 +988,7 @@ def get_path_collection_extents( if len(paths) == 0: raise ValueError("No paths provided") return Bbox.from_extents(*_path.get_path_collection_extents( - master_transform, paths, ensure_3d(transforms), + master_transform, paths, np.atleast_3d(transforms), offsets, offset_transform)) diff --git a/lib/matplotlib/tests/test_cbook.py b/lib/matplotlib/tests/test_cbook.py index b244e761cf8f..2b916b08566f 100644 --- a/lib/matplotlib/tests/test_cbook.py +++ b/lib/matplotlib/tests/test_cbook.py @@ -376,11 +376,3 @@ def test_step_fails(): np.arange(12)) assert_raises(ValueError, cbook._step_validation, np.arange(12), np.arange(3)) - - -def test_ensure_3d(): - assert_array_equal([[[1]], [[2]], [[3]]], - cbook.ensure_3d([1, 2, 3])) - assert_array_equal([[[1], [2]], [[3], [4]]], - cbook.ensure_3d([[1, 2], [3, 4]])) - assert_raises(ValueError, cbook.ensure_3d, [[[[1]]]]) diff --git a/lib/matplotlib/transforms.py b/lib/matplotlib/transforms.py index 17ad25e2fad7..54981f7f01ba 100644 --- a/lib/matplotlib/transforms.py +++ b/lib/matplotlib/transforms.py @@ -48,7 +48,6 @@ from sets import Set as set from .path import Path -from .cbook import ensure_3d DEBUG = False # we need this later, but this is very expensive to set up @@ -668,7 +667,7 @@ def count_overlaps(self, bboxes): bboxes is a sequence of :class:`BboxBase` objects """ return count_bboxes_overlapping_bbox( - self, ensure_3d([np.array(x) for x in bboxes])) + self, np.atleast_3d([np.array(x) for x in bboxes])) def expanded(self, sw, sh): """ From 123deb5b3f95c7a70e140fdc26961f88ca8c5545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jouni=20K=2E=20Sepp=C3=A4nen?= Date: Thu, 1 Oct 2015 20:31:22 +0300 Subject: [PATCH 8/8] Add a comment about _transforms --- lib/matplotlib/collections.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/matplotlib/collections.py b/lib/matplotlib/collections.py index 7ca2b4619dde..58e4d955eeb8 100644 --- a/lib/matplotlib/collections.py +++ b/lib/matplotlib/collections.py @@ -80,6 +80,12 @@ class Collection(artist.Artist, cm.ScalarMappable): # _offsets must be a Nx2 array! _offsets.shape = (0, 2) _transOffset = transforms.IdentityTransform() + #: Either a list of 3x3 arrays or an Nx3x3 array of transforms, suitable + #: for the `all_transforms` argument to + #: :meth:`~matplotlib.backend_bases.RendererBase.draw_path_collection`; + #: each 3x3 array is used to initialize an + #: :class:`~matplotlib.transforms.Affine2D` object. + #: Each kind of collection defines this based on its arguments. _transforms = np.empty((0, 3, 3)) def __init__(self,