Fixed ClusterVector move, filtering, and improved documentation (#357)

- Default `ClusterVector` move operations simplifying the code and
fixing a bug.
- Fixed inconsistent frame number type (uint64/int32)
- Validate Python masks as one-dimensional, C-contiguous Boolean arrays,
handle empty masks safely, and reserve filtered storage based on the
selected cluster count.
- Align the C++ and Python API documentation with the implementation,
including concise `hitmap` and reduction documentation and a correctly
rendered constructor example.

python/aare/ClusterVector.py

@@ -7,16 +7,33 @@ from .ClusterFinder import _get_class
 
 def ClusterVector(cluster_size=(3,3), dtype = np.int32):
     """
-    Factory function to create a ClusterVector object. Provides a cleaner syntax for
-    the templated ClusterVector in C++.
+    Create an empty ClusterVector for a supported cluster size and pixel dtype.
 
-    .. code-block:: python
+    Parameters
+    ----------
+    cluster_size : tuple[int, int], default=(3, 3)
+        Cluster dimensions in the x and y directions.
+    dtype : numpy.dtype, default=numpy.int32
+        Pixel storage type. The cluster size and dtype combination must have a
+        compiled binding.
 
-        from aare import ClusterVector
-        
-        ClusterVector(cluster_size=(3,3), dtype=np.float64)
+    Returns
+    -------
+    ClusterVector
+        An empty vector with frame number 0 and space reserved for at least
+        1024 clusters.
+
+    Raises
+    ------
+    ValueError
+        If the requested size and dtype combination is unavailable.
+
+    Examples
+    --------
+    >>> import numpy as np
+    >>> from aare import ClusterVector
+    >>> clusters = ClusterVector(cluster_size=(3, 3), dtype=np.float64)
     """
 
     cls = _get_class("ClusterVector", cluster_size, dtype)
     return cls()
-

python/src/bind_Cluster.hpp

@@ -67,8 +67,16 @@ void define_Cluster(py::module &m, const std::string &typestr) {
                 return py::make_tuple(max_sum.sum,
                                       static_cast<int>(max_sum.index));
             },
-            R"(calculates sum of 2x2 subcluster with highest energy and index relative to cluster center 0: top_left, 1: top_right, 2: bottom_left, 3: bottom_right
-            )");
+            R"doc(
+            Return the highest-sum center-adjacent 2x2 subcluster.
+
+            Returns
+            -------
+            tuple
+                ``(sum, index)``, where index is 0 for top-left, 1 for
+                top-right, 2 for bottom-left, or 3 for bottom-right relative to
+                the cluster center.
+            )doc");
 }
 
 template <typename T, uint8_t ClusterSizeX, uint8_t ClusterSizeY,
@@ -80,7 +88,14 @@ void reduce_to_3x3(py::module &m) {
         [](const Cluster<T, ClusterSizeX, ClusterSizeY, CoordType> &cl) {
             return reduce_to_3x3(cl);
         },
-        py::return_value_policy::move, R"(Reduce cluster to 3x3 subcluster)");
+        py::return_value_policy::move, py::arg("cluster"), R"doc(
+        Return the 3x3 block around the cluster's center index.
+
+        Both input dimensions must be at least 3, and at least one must be
+        greater than 3.
+        The input coordinates are preserved and output data is stored in
+        row-major order.
+        )doc");
 }
 
 template <typename T, uint8_t ClusterSizeX, uint8_t ClusterSizeY,
@@ -92,16 +107,12 @@ void reduce_to_2x2(py::module &m) {
         [](const Cluster<T, ClusterSizeX, ClusterSizeY, CoordType> &cl) {
             return reduce_to_2x2(cl);
         },
-        py::return_value_policy::move,
-        R"(
-        Reduce cluster to 2x2 subcluster by taking the 2x2 subcluster with 
-        the highest photon energy.
+        py::return_value_policy::move, py::arg("cluster"), R"doc(
+        Return the highest-sum center-adjacent 2x2 block.
 
-        RETURN: 
-        
-        reduced cluster (cluster is filled in row major ordering starting at the top left. Thus for a max subcluster in the top left corner the photon hit is at the fourth position.)
-        
-        )");
+        The input coordinates are preserved and output data is stored in
+        row-major order.
+        )doc");
 }
 
-#pragma GCC diagnostic pop
+#pragma GCC diagnostic pop

python/src/bind_ClusterVector.hpp

@@ -29,37 +29,76 @@ void define_ClusterVector(py::module &m, const std::string &typestr) {
 
     py::class_<ClusterVector<
         Cluster<Type, ClusterSizeX, ClusterSizeY, CoordType>, void>>(
-        m, class_name.c_str(),
+        m, class_name.c_str(), R"doc(
+        A contiguous, move-only container of fixed-size clusters.
+
+        The class supports the Python buffer protocol, so ``numpy.array`` can
+        either copy its data or create a zero-copy view. A zero-copy view is
+        valid only while the ClusterVector's underlying allocation and size
+        remain unchanged.
+        )doc",
         py::buffer_protocol())
 
-        .def(py::init()) // TODO change!!!
+        .def(py::init(), R"doc(
+        Create an empty ClusterVector with frame number 0 and space reserved
+        for at least 1024 clusters.
+        )doc")
 
         .def(
             "__call__",
-            [](ClusterVector<ClusterType> &self, py::array_t<bool> mask) {
+            [](ClusterVector<ClusterType> &self,
+               py::array_t<bool, py::array::c_style> mask) {
+                if (mask.ndim() != 1) {
+                    throw py::value_error("Mask must be one-dimensional");
+                }
                 return self(make_view_1d(mask));
             },
-            py::arg("mask"), R"(
-            Create a copy of the clustervector and apply a boolean mask to the ClusterVector.
+            py::arg("mask").noconvert(), R"doc(
+            Return a filtered copy of this ClusterVector.
 
             Parameters
             ----------
+            mask : numpy.ndarray
+                One-dimensional, writable, C-contiguous array with dtype
+                ``numpy.bool_`` and one element per cluster.
 
-            mask : 1d boolean numpy array
-                Mask to apply to the ClusterVector. Must be the same length as the number of clusters in the ClusterVector. 
+            Returns
+            -------
+            ClusterVector
+                Selected clusters in their original order. The frame number is
+                preserved.
+            )doc")
 
-            )")
+        .def(
+            "push_back",
+            [](ClusterVector<ClusterType> &self, const ClusterType &cluster) {
+                self.push_back(cluster);
+            },
+            py::arg("cluster"), R"doc(
+            Append one cluster.
 
-        .def("push_back",
-             [](ClusterVector<ClusterType> &self, const ClusterType &cluster) {
-                 self.push_back(cluster);
-             })
+            Notes
+            -----
+            Do not call this method while a zero-copy NumPy view of the
+            ClusterVector exists. Reallocation invalidates the view, while an
+            append without reallocation leaves its shape unchanged.
+            )doc")
 
-        .def("sum",
-             [](ClusterVector<ClusterType> &self) {
-                 auto *vec = new std::vector<Type>(self.sum());
-                 return return_vector(vec);
-             })
+        .def(
+            "sum",
+            [](ClusterVector<ClusterType> &self) {
+                auto *vec = new std::vector<Type>(self.sum());
+                return return_vector(vec);
+            },
+            R"doc(
+            Return the sum of all pixels in each cluster.
+
+            Returns
+            -------
+            numpy.ndarray
+                One value per cluster, in container order and with the cluster
+                pixel dtype.
+            )doc")
         .def(
             "sum_2x2",
             [](ClusterVector<ClusterType> &self) {
@@ -68,24 +107,45 @@ void define_ClusterVector(py::module &m, const std::string &typestr) {
 
                 return return_vector(vec);
             },
-            R"(calculates sum of 2x2 subcluster with highest energy and index relative to cluster center 0: top_left, 1: top_right, 2: bottom_left, 3: bottom_right
-            )")
-        .def_property_readonly("size", &ClusterVector<ClusterType>::size)
-        .def("empty", &ClusterVector<ClusterType>::empty)
-        .def("item_size", &ClusterVector<ClusterType>::item_size)
-        .def_property_readonly("fmt",
-                               [typestr](ClusterVector<ClusterType> &self) {
-                                   return fmt_format<ClusterType>;
-                               })
+            R"doc(
+            Return the highest-sum center-adjacent 2x2 subcluster for each
+            cluster.
+
+            Returns
+            -------
+            numpy.ndarray
+                Structured array with ``sum`` and ``index`` fields. Indices are
+                0 for top-left, 1 for top-right, 2 for bottom-left, and 3 for
+                bottom-right, relative to the cluster center.
+            )doc")
+        .def_property_readonly("size", &ClusterVector<ClusterType>::size,
+                               "Number of stored clusters.")
+        .def("empty", &ClusterVector<ClusterType>::empty,
+             "Return True when no clusters are stored.")
+        .def("item_size", &ClusterVector<ClusterType>::item_size,
+             "Return the size in bytes of one stored cluster, including "
+             "padding.")
+        .def_property_readonly(
+            "fmt",
+            [typestr](ClusterVector<ClusterType> &self) {
+                return fmt_format<ClusterType>;
+            },
+            "PEP 3118 format string for one stored cluster.")
 
         .def_property_readonly("cluster_size_x",
-                               &ClusterVector<ClusterType>::cluster_size_x)
+                               &ClusterVector<ClusterType>::cluster_size_x,
+                               "Cluster size in the x dimension.")
         .def_property_readonly("cluster_size_y",
-                               &ClusterVector<ClusterType>::cluster_size_y)
+                               &ClusterVector<ClusterType>::cluster_size_y,
+                               "Cluster size in the y dimension.")
         .def_property_readonly("capacity",
-                               &ClusterVector<ClusterType>::capacity)
+                               &ClusterVector<ClusterType>::capacity,
+                               "Number of clusters that fit without "
+                               "reallocation.")
         .def_property("frame_number", &ClusterVector<ClusterType>::frame_number,
-                      &ClusterVector<ClusterType>::set_frame_number)
+                      &ClusterVector<ClusterType>::set_frame_number,
+                      "Signed 32-bit frame number; 0 can indicate clusters "
+                      "from multiple frames.")
         .def_buffer(
             [typestr](ClusterVector<ClusterType> &self) -> py::buffer_info {
                 return py::buffer_info(
@@ -99,31 +159,38 @@ void define_ClusterVector(py::module &m, const std::string &typestr) {
             });
 
     // Free functions using ClusterVector
-    m.def("hitmap",
-          [](std::array<size_t, 2> image_size, ClusterVector<ClusterType> &cv) {
-              // Create a numpy array to hold the hitmap
-              // The shape of the array is (image_size[0], image_size[1])
-              // note that the python array is passed as [row, col] which
-              // is the opposite of the clusters [x,y]
-              py::array_t<int32_t> hitmap(image_size);
-              auto r = hitmap.mutable_unchecked<2>();
+    m.def(
+        "hitmap",
+        [](std::array<size_t, 2> image_size, ClusterVector<ClusterType> &cv) {
+            // Create a numpy array to hold the hitmap
+            // The shape of the array is (image_size[0], image_size[1])
+            // note that the python array is passed as [row, col] which
+            // is the opposite of the clusters [x,y]
+            py::array_t<int32_t> hitmap(image_size);
+            auto r = hitmap.mutable_unchecked<2>();
 
-              // Initialize hitmap to 0
-              for (py::ssize_t i = 0; i < r.shape(0); i++)
-                  for (py::ssize_t j = 0; j < r.shape(1); j++)
-                      r(i, j) = 0;
+            // Initialize hitmap to 0
+            for (py::ssize_t i = 0; i < r.shape(0); i++)
+                for (py::ssize_t j = 0; j < r.shape(1); j++)
+                    r(i, j) = 0;
 
-              // Loop over the clusters and increment the hitmap
-              // Skip out of bound clusters
-              for (const auto &cluster : cv) {
-                  auto x = cluster.x;
-                  auto y = cluster.y;
-                  if (x < image_size[1] && y < image_size[0])
-                      r(cluster.y, cluster.x) += 1;
-              }
+            // Loop over the clusters and increment the hitmap
+            // Skip out of bound clusters
+            for (const auto &cluster : cv) {
+                auto x = cluster.x;
+                auto y = cluster.y;
+                if (x < image_size[1] && y < image_size[0])
+                    r(cluster.y, cluster.x) += 1;
+            }
 
-              return hitmap;
-          });
+            return hitmap;
+        },
+        py::arg("image_size"), py::arg("clusters"), R"doc(
+        Count cluster centers into an ``int32`` image whose shape is given by
+        ``image_size`` as ``(rows, columns)``. Element ``[y, x]`` contains the
+        number of cluster centers at that coordinate. Out-of-bounds centers are
+        ignored.
+        )doc");
 }
 
 template <typename Type, uint8_t ClusterSizeX, uint8_t ClusterSizeY,
@@ -136,15 +203,12 @@ void define_2x2_reduction(py::module &m) {
             return new ClusterVector<Cluster<Type, 2, 2, CoordType>>(
                 reduce_to_2x2(cv));
         },
-        R"(
-        Reduce cluster to 2x2 subcluster by taking the 2x2 subcluster with 
-        the highest photon energy.
-
-        Parameters
-
-        cv : ClusterVector (clusters are filled in row-major ordering starting at the top left. Thus for a max subcluster in the top left corner the photon hit is at the fourth position.)
-
-        )",
+        R"doc(
+        Reduce every cluster to its highest-sum center-adjacent 2x2 block.
+        Returns a new ClusterVector; cluster order, coordinates, and frame
+        number are preserved. Input pixel data is interpreted in row-major
+        order.
+        )doc",
         py::arg("clustervector"));
 }
 
@@ -159,13 +223,13 @@ void define_3x3_reduction(py::module &m) {
             return new ClusterVector<Cluster<Type, 3, 3, CoordType>>(
                 reduce_to_3x3(cv));
         },
-        R"(
-        Reduce cluster to 3x3 subcluster 
-
-        Parameters
-        
-        cv : ClusterVector   
-        )",
+        R"doc(
+        Reduce every cluster to the 3x3 block around its center index.
+        Both input dimensions must be at least 3, and at least one must be
+        greater than 3.
+        Returns a new ClusterVector; cluster order, coordinates, and frame
+        number are preserved.
+        )doc",
         py::arg("clustervector"));
 }
 

python/src/module.cpp

@@ -128,9 +128,6 @@ PYBIND11_MODULE(_aare, m) {
     DEFINE_BINDINGS_CLUSTERFINDER(double, 9, 9, uint16_t, d);
     DEFINE_BINDINGS_CLUSTERFINDER(float, 9, 9, uint16_t, f);
 
-    define_3x3_reduction<int, 3, 3, uint16_t>(m);
-    define_3x3_reduction<double, 3, 3, uint16_t>(m);
-    define_3x3_reduction<float, 3, 3, uint16_t>(m);
     define_3x3_reduction<int, 5, 5, uint16_t>(m);
     define_3x3_reduction<double, 5, 5, uint16_t>(m);
     define_3x3_reduction<float, 5, 5, uint16_t>(m);
@@ -141,9 +138,6 @@ PYBIND11_MODULE(_aare, m) {
     define_3x3_reduction<double, 9, 9, uint16_t>(m);
     define_3x3_reduction<float, 9, 9, uint16_t>(m);
 
-    reduce_to_3x3<int, 3, 3, uint16_t>(m);
-    reduce_to_3x3<double, 3, 3, uint16_t>(m);
-    reduce_to_3x3<float, 3, 3, uint16_t>(m);
     reduce_to_3x3<int, 5, 5, uint16_t>(m);
     reduce_to_3x3<double, 5, 5, uint16_t>(m);
     reduce_to_3x3<float, 5, 5, uint16_t>(m);

python/tests/test_ClusterVector.py

@@ -83,12 +83,15 @@ def test_make_a_hitmap_from_cluster_vector():
 
 def test_2x2_reduction(): 
     cv = ClusterVector((3,3))
+    cv.frame_number = -135
 
     cv.push_back(_aare.Cluster3x3i(5, 5, np.array([1, 1, 1, 2, 3, 1, 2, 2, 1], dtype=np.int32)))
     cv.push_back(_aare.Cluster3x3i(5, 5, np.array([2, 2, 1, 2, 3, 1, 1, 1, 1], dtype=np.int32)))
 
-    reduced_cv = np.array(_aare.reduce_to_2x2(cv), copy=False) 
+    reduced = _aare.reduce_to_2x2(cv)
+    reduced_cv = np.array(reduced, copy=False)
 
+    assert reduced.frame_number == cv.frame_number
     assert reduced_cv.size == 2
     assert reduced_cv[0]["x"] == 5
     assert reduced_cv[0]["y"] == 5
@@ -100,14 +103,17 @@ def test_2x2_reduction():
     
 def test_3x3_reduction(): 
     cv = _aare.ClusterVector_Cluster5x5d()
+    cv.frame_number = 246
     
     cv.push_back(_aare.Cluster5x5d(5,5,np.array([1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 2.0, 1.0, 1.0, 1.0, 2.0, 2.0, 3.0,
                                    1.0, 1.0, 1.0, 2.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0], dtype=np.double)))
     cv.push_back(_aare.Cluster5x5d(5,5,np.array([1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 2.0, 1.0, 1.0, 1.0, 2.0, 2.0, 3.0,
                                    1.0, 1.0, 1.0, 2.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0], dtype=np.double)))
     
-    reduced_cv = np.array(_aare.reduce_to_3x3(cv), copy=False)  
+    reduced = _aare.reduce_to_3x3(cv)
+    reduced_cv = np.array(reduced, copy=False)
 
+    assert reduced.frame_number == cv.frame_number
     assert reduced_cv.size == 2
     assert reduced_cv[0]["x"] == 5
     assert reduced_cv[0]["y"] == 5
@@ -130,3 +136,26 @@ def test_masking():
     assert cv_masked_array[0]["x"] == 1
     assert cv_masked_array[0]["y"] == 2
     assert (cv_masked_array[0]["data"] == np.ones((3,3),dtype=np.int32)).all()
+
+
+def test_masking_requires_c_contiguous_array():
+    cv = _aare.ClusterVector_Cluster3x3i()
+    cv.push_back(_aare.Cluster3x3i(1, 2, np.ones(9, dtype=np.int32)))
+    cv.push_back(_aare.Cluster3x3i(3, 4, np.ones(9, dtype=np.int32)))
+
+    mask = np.array([True, False, True, False], dtype=bool)[::2]
+    assert not mask.flags.c_contiguous
+
+    with pytest.raises(TypeError):
+        cv(mask)
+
+
+def test_masking_requires_one_dimension():
+    cv = _aare.ClusterVector_Cluster3x3i()
+    cv.push_back(_aare.Cluster3x3i(1, 2, np.ones(9, dtype=np.int32)))
+    cv.push_back(_aare.Cluster3x3i(3, 4, np.ones(9, dtype=np.int32)))
+
+    mask = np.array([[True, False]], dtype=bool)
+
+    with pytest.raises(ValueError, match="one-dimensional"):
+        cv(mask)