Fix exception handling when constructing C-level PathGenerator.

The PathGenerator constructor does not set a Python-level exception on
failure.  Switch to use a PyArg_ParseTuple-style converter to get proper
error handling.

(Also, CALL_CPP already converts py::exception to Python-level
exceptions, so it doesn't need to be wrapped in a C++-level try...
catch.)

Author: anntzer

lib/matplotlib/tests/test_agg.py

@@ -244,3 +244,10 @@ def test_pil_kwargs_tiff():
     im = Image.open(buf)
     tags = {TiffTags.TAGS_V2[k].name: v for k, v in im.tag_v2.items()}
     assert tags["ImageDescription"] == "test image"
+
+
+def test_draw_path_collection_error_handling():
+    fig, ax = plt.subplots()
+    ax.scatter([1], [1]).set_paths(path.Path([(0, 1), (2, 3)]))
+    with pytest.raises(TypeError):
+        fig.canvas.draw()

src/_backend_agg_wrapper.cpp

@@ -320,7 +320,7 @@ PyRendererAgg_draw_path_collection(PyRendererAgg *self, PyObject *args, PyObject
 {
     GCAgg gc;
     agg::trans_affine master_transform;
-    PyObject *pathobj;
+    py::PathGenerator paths;
     numpy::array_view<const double, 3> transforms;
     numpy::array_view<const double, 2> offsets;
     agg::trans_affine offset_trans;
@@ -333,12 +333,13 @@ PyRendererAgg_draw_path_collection(PyRendererAgg *self, PyObject *args, PyObject
     PyObject *offset_position; // offset position is no longer used
 
     if (!PyArg_ParseTuple(args,
-                          "O&O&OO&O&O&O&O&O&O&O&OO:draw_path_collection",
+                          "O&O&O&O&O&O&O&O&O&O&O&OO:draw_path_collection",
                           &convert_gcagg,
                           &gc,
                           &convert_trans_affine,
                           &master_transform,
-                          &pathobj,
+                          &convert_pathgen,
+                          &paths,
                           &convert_transforms,
                           &transforms,
                           &convert_points,
@@ -360,27 +361,18 @@ PyRendererAgg_draw_path_collection(PyRendererAgg *self, PyObject *args, PyObject
         return NULL;
     }
 
-    try
-    {
-        py::PathGenerator path(pathobj);
-
-        CALL_CPP("draw_path_collection",
-                 (self->x->draw_path_collection(gc,
-                                                master_transform,
-                                                path,
-                                                transforms,
-                                                offsets,
-                                                offset_trans,
-                                                facecolors,
-                                                edgecolors,
-                                                linewidths,
-                                                dashes,
-                                                antialiaseds)));
-    }
-    catch (const py::exception &)
-    {
-        return NULL;
-    }
+    CALL_CPP("draw_path_collection",
+             (self->x->draw_path_collection(gc,
+                                            master_transform,
+                                            paths,
+                                            transforms,
+                                            offsets,
+                                            offset_trans,
+                                            facecolors,
+                                            edgecolors,
+                                            linewidths,
+                                            dashes,
+                                            antialiaseds)));
 
     Py_RETURN_NONE;
 }

src/_path_wrapper.cpp

@@ -269,17 +269,18 @@ const char *Py_get_path_collection_extents__doc__ =
 static PyObject *Py_get_path_collection_extents(PyObject *self, PyObject *args, PyObject *kwds)
 {
     agg::trans_affine master_transform;
-    PyObject *pathsobj;
+    py::PathGenerator paths;
     numpy::array_view<const double, 3> transforms;
     numpy::array_view<const double, 2> offsets;
     agg::trans_affine offset_trans;
     extent_limits e;
 
     if (!PyArg_ParseTuple(args,
-                          "O&OO&O&O&:get_path_collection_extents",
+                          "O&O&O&O&O&:get_path_collection_extents",
                           &convert_trans_affine,
                           &master_transform,
-                          &pathsobj,
+                          &convert_pathgen,
+                          &paths,
                           &convert_transforms,
                           &transforms,
                           &convert_points,
@@ -289,18 +290,9 @@ static PyObject *Py_get_path_collection_extents(PyObject *self, PyObject *args,
         return NULL;
     }
 
-    try
-    {
-        py::PathGenerator paths(pathsobj);
-
-        CALL_CPP("get_path_collection_extents",
-                 (get_path_collection_extents(
-                     master_transform, paths, transforms, offsets, offset_trans, e)));
-    }
-    catch (const py::exception &)
-    {
-        return NULL;
-    }
+    CALL_CPP("get_path_collection_extents",
+             (get_path_collection_extents(
+                 master_transform, paths, transforms, offsets, offset_trans, e)));
 
     npy_intp dims[] = { 2, 2 };
     numpy::array_view<double, 2> extents(dims);
@@ -327,7 +319,7 @@ static PyObject *Py_point_in_path_collection(PyObject *self, PyObject *args, PyO
 {
     double x, y, radius;
     agg::trans_affine master_transform;
-    PyObject *pathsobj;
+    py::PathGenerator paths;
     numpy::array_view<const double, 3> transforms;
     numpy::array_view<const double, 2> offsets;
     agg::trans_affine offset_trans;
@@ -336,13 +328,14 @@ static PyObject *Py_point_in_path_collection(PyObject *self, PyObject *args, PyO
     std::vector<int> result;
 
     if (!PyArg_ParseTuple(args,
-                          "dddO&OO&O&O&O&O:point_in_path_collection",
+                          "dddO&O&O&O&O&O&O:point_in_path_collection",
                           &x,
                           &y,
                           &radius,
                           &convert_trans_affine,
                           &master_transform,
-                          &pathsobj,
+                          &convert_pathgen,
+                          &paths,
                           &convert_transforms,
                           &transforms,
                           &convert_points,
@@ -355,26 +348,17 @@ static PyObject *Py_point_in_path_collection(PyObject *self, PyObject *args, PyO
         return NULL;
     }
 
-    try
-    {
-        py::PathGenerator paths(pathsobj);
-
-        CALL_CPP("point_in_path_collection",
-                 (point_in_path_collection(x,
-                                           y,
-                                           radius,
-                                           master_transform,
-                                           paths,
-                                           transforms,
-                                           offsets,
-                                           offset_trans,
-                                           filled,
-                                           result)));
-    }
-    catch (const py::exception &)
-    {
-        return NULL;
-    }
+    CALL_CPP("point_in_path_collection",
+             (point_in_path_collection(x,
+                                       y,
+                                       radius,
+                                       master_transform,
+                                       paths,
+                                       transforms,
+                                       offsets,
+                                       offset_trans,
+                                       filled,
+                                       result)));
 
     npy_intp dims[] = {(npy_intp)result.size() };
     numpy::array_view<int, 1> pyresult(dims);

src/py_adaptors.h

@@ -194,12 +194,7 @@ class PathGenerator
   public:
     typedef PathIterator path_iterator;
 
-    PathGenerator(PyObject *obj) : m_paths(NULL), m_npaths(0)
-    {
-        if (!set(obj)) {
-            throw py::exception();
-        }
-    }
+    PathGenerator() : m_paths(NULL), m_npaths(0) {}
 
     ~PathGenerator()
     {

src/py_converters.cpp

@@ -415,6 +415,16 @@ int convert_path(PyObject *obj, void *pathp)
     return status;
 }
 
+int convert_pathgen(PyObject *obj, void *pathgenp)
+{
+    py::PathGenerator *paths = (py::PathGenerator *)pathgenp;
+    if (!paths->set(obj)) {
+        PyErr_SetString(PyExc_TypeError, "Not an iterable of paths");
+        return 0;
+    }
+    return 1;
+}
+
 int convert_clippath(PyObject *clippath_tuple, void *clippathp)
 {
     ClipPath *clippath = (ClipPath *)clippathp;

src/py_converters.h

@@ -32,6 +32,7 @@ int convert_dashes(PyObject *dashobj, void *gcp);
 int convert_dashes_vector(PyObject *obj, void *dashesp);
 int convert_trans_affine(PyObject *obj, void *transp);
 int convert_path(PyObject *obj, void *pathp);
+int convert_pathgen(PyObject *obj, void *pathgenp);
 int convert_clippath(PyObject *clippath_tuple, void *clippathp);
 int convert_snap(PyObject *obj, void *snapp);
 int convert_sketch_params(PyObject *obj, void *sketchp);