Releaseable point cloud

Instead of transforming the point cloud directly to a numpy buffer, we
should instead return a intermediate layer, since in the underlying C++
API point cloud exposes various functions such as width and height.

Making buffer releasable also created some technical debt:
assertNotReleased is needed since accessing the underlying a C++ object
after it has been released causes a crash.

Ignored flake8's D402, since it mistakenly thought that the paranthesis
in the docstring was the signature of the function.

.flake8-packaged-files

@@ -11,4 +11,5 @@ ignore =
 ### Temporarily disabled docstring errors:
     D105, # missing docstring in magic method
     I401, # Missing exception(s) in Raises section
+    D402, # First line should not be the function’s “signature”
 show-source = True

modules/zivid/__init__.py

@@ -14,6 +14,7 @@
 )
 import zivid.environment
 import zivid.firmware
+from zivid.point_cloud import PointCloud
 from zivid.frame import Frame  # noqa: F401 'zivid.frame.Frame' imported but unused
 from zivid.frame_info import (
     FrameInfo,  # noqa: F401 'zivid.frame_info.FrameInfo' imported but unused

modules/zivid/frame.py

@@ -1,11 +1,11 @@
 """Contains the Frame class."""
 from pathlib import Path
-import numpy
 import _zivid
 
 import zivid._settings_converter as _settings_converter
 import zivid._camera_state_converter as _camera_state_converter
 import zivid._frame_info_converter as _frame_info_converter
+from zivid.point_cloud import PointCloud
 
 
 class Frame:  # pylint: disable=too-few-public-methods
@@ -36,10 +36,10 @@ def get_point_cloud(self):
         """Copy the point cloud to the CPU and returns it.
 
         Returns:
-            A numpy array
+            a point cloud instance
 
         """
-        return numpy.array(self.__impl.get_point_cloud())
+        return PointCloud(self.__impl.get_point_cloud())
 
     def save(self, file_path):
         """Save the frame to file. The file type is determined from the file extension.

modules/zivid/point_cloud.py

@@ -0,0 +1,66 @@
+"""Contains the PointCloud class."""
+import numpy
+
+import _zivid
+
+
+class PointCloud:
+    """A point cloud."""
+
+    def __init__(self, internal_point_cloud):
+        """Create a point cloud from an internal point cloud.
+
+        Args:
+            internal_point_cloud: a internal point cloud
+
+        """
+        if not isinstance(internal_point_cloud, _zivid.PointCloud):
+            raise ValueError(
+                "Unsupported type for argument internal_point_cloud. Got {}, expected {}".format(
+                    type(internal_point_cloud), type(_zivid.PointCloud)
+                )
+            )
+        self.__impl = internal_point_cloud
+
+    def to_array(self):
+        """Convert point cloud to numpy array.
+
+        Returns:
+            a numpy array
+
+        """
+        self.__impl.assert_not_released()
+        return numpy.array(self.__impl)
+
+    @property
+    def height(self):
+        """Return height (number of rows) of point cloud.
+
+        Returns:
+            a positive integer
+
+        """
+        return self.__impl.height()
+
+    @property
+    def width(self):
+        """Return width (number of columns) of point cloud.
+
+        Returns:
+            a positive integer
+
+        """
+        return self.__impl.width()
+
+    def release(self):
+        """Release the underlying resources."""
+        self.__impl.release()
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, exception_type, exception_value, traceback):
+        self.release()
+
+    def __del__(self):
+        self.release()

src/CMakeLists.txt

@@ -5,7 +5,7 @@ set(SOURCES
     Environment.cpp
     Firmware.cpp
     HDR.cpp
-    PointCloud.cpp
+    ReleasablePointCloud.cpp
     ReleasableCamera.cpp
     ReleasableFrame.cpp
     SingletonApplication.cpp

src/PointCloud.cpp → src/ReleasablePointCloud.cpp

@@ -1,4 +1,4 @@
-#include <ZividPython/PointCloud.h>
+#include <ZividPython/ReleasablePointCloud.h>
 
 #include <pybind11/pybind11.h>
 
@@ -18,7 +18,7 @@ namespace
     };
 #pragma pack(pop)
 
-    py::buffer_info makeBufferInfo(Zivid::PointCloud &pointCloud)
+    py::buffer_info makeBufferInfo(ZividPython::ReleasablePointCloud &pointCloud)
     {
         const auto data = pointCloud.dataPtr();
 
@@ -45,10 +45,13 @@ namespace
 
 namespace ZividPython
 {
-    void wrapClass(pybind11::class_<Zivid::PointCloud> pyClass)
+    void wrapClass(pybind11::class_<ReleasablePointCloud> pyClass)
     {
         PYBIND11_NUMPY_DTYPE(DataType, x, y, z, contrast, b, g, r, a);
 
-        pyClass.def(py::init<>()).def_buffer(makeBufferInfo);
+        pyClass.def(py::init<>())
+            .def_buffer(makeBufferInfo)
+            .def("width", &ReleasablePointCloud::width)
+            .def("height", &ReleasablePointCloud::height);
     }
 } // namespace ZividPython

src/Wrapper.cpp

@@ -7,9 +7,9 @@
 #include <ZividPython/Environment.h>
 #include <ZividPython/Firmware.h>
 #include <ZividPython/HDR.h>
-#include <ZividPython/PointCloud.h>
 #include <ZividPython/ReleasableCamera.h>
 #include <ZividPython/ReleasableFrame.h>
+#include <ZividPython/ReleasablePointCloud.h>
 #include <ZividPython/SingletonApplication.h>
 #include <ZividPython/Version.h>
 #include <ZividPython/Wrapper.h>
@@ -30,7 +30,7 @@ ZIVID_PYTHON_MODULE // NOLINT
     ZIVID_PYTHON_WRAP_CLASS_AS_RELEASABLE(module, Frame);
     ZIVID_PYTHON_WRAP_CLASS(module, CameraRevision);
 
-    ZIVID_PYTHON_WRAP_CLASS_BUFFER(module, PointCloud);
+    ZIVID_PYTHON_WRAP_CLASS_BUFFER_AS_RELEASABLE(module, PointCloud);
 
     ZIVID_PYTHON_WRAP_NAMESPACE_AS_SUBMODULE(module, Environment);
     ZIVID_PYTHON_WRAP_NAMESPACE_AS_SUBMODULE(module, Firmware);

src/include/ZividPython/Releasable.h

@@ -93,6 +93,13 @@ namespace ZividPython
             m_impl.reset();
         }
 
+        // This function is required to verify that the buffer has not already
+        // released in certain situations. Ideally it should not exist.
+        void assertNotReleased()
+        {
+            std::ignore = impl();
+        }
+
     private:
         std::optional<T> m_impl{ std::make_optional<T>() };
     };

src/include/ZividPython/ReleasableFrame.h

@@ -2,6 +2,7 @@
 
 #include <Zivid/Frame.h>
 #include <ZividPython/Releasable.h>
+#include <ZividPython/ReleasablePointCloud.h>
 #include <ZividPython/Wrappers.h>
 
 namespace ZividPython
@@ -13,7 +14,7 @@ namespace ZividPython
 
         ZIVID_PYTHON_FORWARD_1_ARGS(save, const std::string &, fileName)
         ZIVID_PYTHON_FORWARD_1_ARGS(load, const std::string &, fileName)
-        ZIVID_PYTHON_FORWARD_0_ARGS(getPointCloud)
+        ZIVID_PYTHON_FORWARD_0_ARGS_WRAP_RETURN(ReleasablePointCloud, getPointCloud)
         ZIVID_PYTHON_FORWARD_0_ARGS(settings)
         ZIVID_PYTHON_FORWARD_0_ARGS(state)
         ZIVID_PYTHON_FORWARD_0_ARGS(info)

src/include/ZividPython/ReleasablePointCloud.h

@@ -0,0 +1,20 @@
+#pragma once
+
+#include <Zivid/PointCloud.h>
+#include <ZividPython/Releasable.h>
+#include <ZividPython/Wrappers.h>
+
+namespace ZividPython
+{
+    class ReleasablePointCloud : public Releasable<Zivid::PointCloud>
+    {
+    public:
+        using Releasable<Zivid::PointCloud>::Releasable;
+
+        ZIVID_PYTHON_FORWARD_0_ARGS(width)
+        ZIVID_PYTHON_FORWARD_0_ARGS(height)
+        ZIVID_PYTHON_FORWARD_0_ARGS(dataPtr)
+    };
+
+    void wrapClass(pybind11::class_<ReleasablePointCloud> pyClass);
+} // namespace ZividPython

src/include/ZividPython/Wrappers.h

@@ -30,7 +30,7 @@ namespace ZividPython
 
         if constexpr(WrapType::releasable == wrapType)
         {
-            pyClass.def("release", &Source::release);
+            pyClass.def("release", &Source::release).def("assert_not_released", &Source::assertNotReleased);
         }
         else if constexpr(WrapType::singleton == wrapType)
         {
@@ -63,10 +63,10 @@ namespace ZividPython
     ZividPython::wrapClass<ZividPython::Singleton##name, ZividPython::WrapType::singleton>(                            \
         dest, static_cast<void (*)(pybind11::class_<ZividPython::Singleton##name>)>(ZividPython::wrapClass), #name);
 
-#define ZIVID_PYTHON_WRAP_CLASS_BUFFER(dest, name)                                                                     \
-    ZividPython::wrapClass<Zivid::name, ZividPython::WrapType::normal>(                                                \
+#define ZIVID_PYTHON_WRAP_CLASS_BUFFER_AS_RELEASABLE(dest, name)                                                       \
+    ZividPython::wrapClass<ZividPython::Releasable##name, ZividPython::WrapType::releasable>(                          \
         dest,                                                                                                          \
-        static_cast<void (*)(pybind11::class_<Zivid::name>)>(ZividPython::wrapClass),                                  \
+        static_cast<void (*)(pybind11::class_<ZividPython::Releasable##name>)>(ZividPython::wrapClass),                \
         #name,                                                                                                         \
         pybind11::buffer_protocol())
 

test/conftest.py

@@ -35,6 +35,12 @@ def frame_fixture(application, sample_data_file):  # pylint: disable=unused-argu
         yield frame
 
 
+@pytest.fixture(name="point_cloud")
+def point_cloud_fixture(frame):
+    with frame.get_point_cloud() as point_cloud:
+        yield point_cloud
+
+
 @pytest.fixture(name="random_settings")
 def random_settings_fixture():
     import datetime

test/test_frame.py

@@ -13,10 +13,10 @@ def test_illegal_init(application):  # pylint: disable=unused-argument
 
 
 def test_get_point_cloud(frame):
-    import numpy
+    import zivid
 
     point_cloud = frame.get_point_cloud()
-    assert isinstance(point_cloud, numpy.ndarray)
+    assert isinstance(point_cloud, zivid.PointCloud)
 
 
 def test_release(frame):

test/test_point_cloud.py

@@ -1,17 +1,71 @@
-def test_point_cloud_to_array(frame):
+def test_point_cloud_to_array(point_cloud):
     import numpy as np
 
-    point_cloud = frame.get_point_cloud()
-    np_array = np.array(point_cloud)
+    np_array = point_cloud.to_array()
     assert np_array is not None
     assert isinstance(np_array, np.ndarray)
 
 
-def test_to_rgb_image(frame):
+def test_to_rgb_image(point_cloud):
     import numpy as np
 
-    point_cloud = frame.get_point_cloud()
-    image = point_cloud[["r", "g", "b"]]
-    image = np.asarray([point_cloud["r"], point_cloud["g"], point_cloud["b"]])
+    np_array = point_cloud.to_array()
+    image = np_array[["r", "g", "b"]]
+    image = np.asarray([np_array["r"], np_array["g"], np_array["b"]])
     image = np.moveaxis(image, [0, 1, 2], [2, 0, 1])
     image = image.astype(np.uint8)
+
+
+def test_height(point_cloud):
+    height = point_cloud.height
+
+    assert height is not None
+    assert isinstance(height, int)
+
+
+def test_width(point_cloud):
+    width = point_cloud.width
+
+    assert width is not None
+    assert isinstance(width, int)
+
+
+def test_height_context_manager(frame):
+    import pytest
+
+    with frame.get_point_cloud() as point_cloud:
+        point_cloud.height  # pylint: disable=pointless-statement
+    with pytest.raises(RuntimeError):
+        point_cloud.height  # pylint: disable=pointless-statement
+
+
+def test_width_context_manager(frame):
+    import pytest
+
+    with frame.get_point_cloud() as point_cloud:
+        point_cloud.width  # pylint: disable=pointless-statement
+    with pytest.raises(RuntimeError):
+        point_cloud.width  # pylint: disable=pointless-statement
+
+
+def test_to_array_context_manager(frame):
+    import pytest
+
+    with frame.get_point_cloud() as point_cloud:
+        point_cloud.to_array()
+    with pytest.raises(RuntimeError):
+        point_cloud.to_array()
+
+
+def test_illegal_init(application):  # pylint: disable=unused-argument
+    import pytest
+    import zivid
+
+    with pytest.raises(TypeError):
+        zivid.PointCloud()  # pylint: disable=no-value-for-parameter
+
+    with pytest.raises(ValueError):
+        zivid.PointCloud("Should fail.")
+
+    with pytest.raises(ValueError):
+        zivid.PointCloud(123)