get some exception safety, using Cython's built-in exception translation

Uses a hack to get around limitations in Cython, see pcl/indexing.hpp.
See https://groups.google.com/forum/#!topic/cython-users/AAo7MQFLZyw
for explanation.

Also, we no longer need to use the at member when bounds checking is not
needed:

>>> import pcl
>>> p = pcl.PointCloud()
>>> %timeit p.from_array(a)
100000 loops, best of 3: 17.5 µs per loop

This used to be 23.4 µs per loop.

Author: larsmans

Makefile

@@ -1,4 +1,5 @@
-pcl/_pcl.so: pcl/_pcl.pyx setup.py pcl/pcl_defs.pxd pcl/minipcl.cpp
+pcl/_pcl.so: pcl/_pcl.pyx setup.py pcl/pcl_defs.pxd pcl/minipcl.cpp \
+             pcl/indexing.hpp
 	python setup.py build_ext --inplace
 
 test: pcl/_pcl.so tests/test.py

pcl/_pcl.pyx

@@ -174,7 +174,7 @@ cdef class PointCloud:
 
         cdef cpp.PointXYZ *p
         for i in range(npts):
-            p = &self.thisptr.at(i)
+            p = cpp.getptr(self.thisptr, i)
             p.x, p.y, p.z = arr[i, 0], arr[i, 1], arr[i, 2]
 
     @cython.boundscheck(False)
@@ -190,7 +190,7 @@ cdef class PointCloud:
         result = np.empty((n, 3), dtype=np.float32)
 
         for i in range(n):
-            p = &self.thisptr.at(i)
+            p = cpp.getptr(self.thisptr, i)
             result[i, 0] = p.x
             result[i, 1] = p.y
             result[i, 2] = p.z
@@ -206,8 +206,8 @@ cdef class PointCloud:
         self.resize(npts)
         self.thisptr.width = npts
         self.thisptr.height = 1
-        for i,l in enumerate(_list):
-            p = &self.thisptr.at(i)
+        for i, l in enumerate(_list):
+            p = cpp.getptr(self.thisptr, i)
             p.x, p.y, p.z = l
 
     def to_list(self):
@@ -223,11 +223,11 @@ cdef class PointCloud:
         """
         Return a point (3-tuple) at the given row/column
         """
-        cdef cpp.PointXYZ *p = &self.thisptr.at(row, col)
+        cdef cpp.PointXYZ *p = cpp.getptr_at(self.thisptr, row, col)
         return p.x, p.y, p.z
 
     def __getitem__(self, cnp.npy_intp idx):
-        cdef cpp.PointXYZ *p = &self.thisptr.at(idx)
+        cdef cpp.PointXYZ *p = cpp.getptr_at(self.thisptr, idx)
         return p.x, p.y, p.z
 
     def from_file(self, char *f):

pcl/indexing.hpp

@@ -0,0 +1,21 @@
+namespace {
+    // Workaround for a Cython bug in operator[] with templated types and
+    // references. Let's hope the compiler optimizes these functions away.
+    template <typename T>
+    T *getptr(pcl::PointCloud<T> *pc, size_t i)
+    {
+        return &(*pc)[i];
+    }
+
+    template <typename T>
+    T *getptr_at(pcl::PointCloud<T> *pc, size_t i)
+    {
+        return &(pc->at(i));
+    }
+
+    template <typename T>
+    T *getptr_at(pcl::PointCloud<T> *pc, int i, int j)
+    {
+        return &(pc->at(i, j));
+    }
+}

pcl/pcl_defs.pxd

@@ -16,11 +16,17 @@ cdef extern from "pcl/point_cloud.h" namespace "pcl":
         bool is_dense
         void resize(size_t) except +
         size_t size()
-        T& operator[](size_t)
-        T& at(size_t)
-        T& at(int, int)
+        #T& operator[](size_t)
+        #T& at(size_t) except +
+        #T& at(int, int) except +
         shared_ptr[PointCloud[T]] makeShared()
 
+cdef extern from "indexing.hpp":
+    # Use these instead of operator[] or at.
+    PointXYZ *getptr(PointCloud[PointXYZ] *, size_t)
+    PointXYZ *getptr_at(PointCloud[PointXYZ] *, size_t) except +
+    PointXYZ *getptr_at(PointCloud[PointXYZ] *, int, int) except +
+
 cdef extern from "pcl/point_types.h" namespace "pcl":
     cdef struct PointXYZ:
         PointXYZ()

tests/test.py

@@ -208,6 +208,20 @@ def testExtractNeg(self):
         self.assertNotEqual(self.p, p2)
         self.assertEqual(p2.size, self.p.size - 3)
 
+class TestExceptions(unittest.TestCase):
+    def setUp(self):
+        self.p = pcl.PointCloud(np.arange(9, dtype=np.float32).reshape(3, 3))
+
+    def testIndex(self):
+        self.assertRaises(IndexError, self.p.__getitem__, self.p.size)
+        self.assertRaises(Exception, self.p.get_point, self.p.size, 1)
+
+    def testResize(self):
+        # XXX MemoryError isn't actually the prettiest exception for a
+        # negative argument. Don't hesitate to change this test to reflect
+        # better exceptions.
+        self.assertRaises(MemoryError, self.p.resize, -1)
+
 class TestSegmenterNormal(unittest.TestCase):
 
     def setUp(self):