From 0d80db3caa01d63483fa5dc48272e24c6409719f Mon Sep 17 00:00:00 2001
From: Maximilian Linhoff <maximilian.linhoff@cta-observatory.org>
Date: Mon, 12 May 2025 11:36:18 +0200
Subject: [PATCH] Remove one level of indirection from DVR classes

---
 src/ctapipe/image/reducer.py            | 70 ++++++++-----------------
 src/ctapipe/image/tests/test_reducer.py |  8 +--
 2 files changed, 27 insertions(+), 51 deletions(-)

diff --git a/src/ctapipe/image/reducer.py b/src/ctapipe/image/reducer.py
index 535aca63355..fe4829da31d 100644
--- a/src/ctapipe/image/reducer.py
+++ b/src/ctapipe/image/reducer.py
@@ -2,7 +2,7 @@
 Algorithms for the data volume reduction.
 """
 
-from abc import abstractmethod
+from abc import ABCMeta, abstractmethod
 
 import numpy as np
 
@@ -18,10 +18,14 @@
 from ctapipe.image.cleaning import dilate
 from ctapipe.image.extractor import ImageExtractor
 
-__all__ = ["DataVolumeReducer", "NullDataVolumeReducer", "TailCutsDataVolumeReducer"]
+__all__ = [
+    "DataVolumeReducer",
+    "NullDataVolumeReducer",
+    "TailCutsDataVolumeReducer",
+]
 
 
-class DataVolumeReducer(TelescopeComponent):
+class DataVolumeReducer(TelescopeComponent, metaclass=ABCMeta):
     """
     Base component for data volume reducers.
     """
@@ -41,39 +45,12 @@ def __init__(self, subarray, config=None, parent=None, **kwargs):
         self.subarray = subarray
         super().__init__(config=config, parent=parent, subarray=subarray, **kwargs)
 
-    def __call__(self, waveforms, tel_id=None, selected_gain_channel=None):
-        """
-        Call the relevant functions to perform data volume reduction on the
-        waveforms.
-
-        Parameters
-        ----------
-        waveforms: ndarray
-            Waveforms stored in a numpy array of shape
-            (n_pix, n_samples).
-        tel_id: int
-            The telescope id. Required for the 'image_extractor' and
-            'camera.geometry' in 'TailCutsDataVolumeReducer'.
-        selected_gain_channel: ndarray
-            The channel selected in the gain selection, per pixel. Required for
-            the 'image_extractor' in 'TailCutsDataVolumeReducer'.
-            extraction.
-
-        Returns
-        -------
-        mask: array
-            Mask of selected pixels.
-        """
-        mask = self.select_pixels(
-            waveforms, tel_id=tel_id, selected_gain_channel=selected_gain_channel
-        )
-        return mask
-
     @abstractmethod
-    def select_pixels(self, waveforms, tel_id=None, selected_gain_channel=None):
+    def __call__(
+        self, waveforms, tel_id: int, selected_gain_channel=None
+    ) -> np.ndarray[bool]:
         """
-        Abstract method to be defined by a DataVolumeReducer subclass.
-        Call the relevant functions for the required pixel selection.
+        Select pixels of which to keep the waveform data.
 
         Parameters
         ----------
@@ -99,7 +76,7 @@ class NullDataVolumeReducer(DataVolumeReducer):
     Perform no data volume reduction
     """
 
-    def select_pixels(self, waveforms, tel_id=None, selected_gain_channel=None):
+    def __call__(self, waveforms, tel_id: int, selected_gain_channel=None):
         n_pixels = waveforms.shape[-2]
         return np.ones(n_pixels, dtype=bool)
 
@@ -122,6 +99,16 @@ class TailCutsDataVolumeReducer(DataVolumeReducer):
     do_boundary_dilation: BoolTelescopeParameter
         If set to 'False', the iteration steps in 2) are skipped and
         normal TailcutCleaning is used.
+
+    Parameters
+    ----------
+    subarray: ctapipe.instrument.SubarrayDescription
+        Description of the subarray
+    config: traitlets.loader.Config
+        Configuration specified by config file or cmdline arguments.
+        Used to set traitlet values.
+        Set to None if no configuration to pass.
+    kwargs
     """
 
     image_extractor_type = TelescopeParameter(
@@ -149,17 +136,6 @@ def __init__(
         image_extractor=None,
         **kwargs,
     ):
-        """
-        Parameters
-        ----------
-        subarray: ctapipe.instrument.SubarrayDescription
-            Description of the subarray
-        config: traitlets.loader.Config
-            Configuration specified by config file or cmdline arguments.
-            Used to set traitlet values.
-            Set to None if no configuration to pass.
-        kwargs
-        """
         super().__init__(config=config, parent=parent, subarray=subarray, **kwargs)
 
         if cleaner is None:
@@ -178,7 +154,7 @@ def __init__(
             self.image_extractor_type = [("type", "*", name)]
             self.image_extractors[name] = image_extractor
 
-    def select_pixels(self, waveforms, tel_id=None, selected_gain_channel=None):
+    def __call__(self, waveforms, tel_id, selected_gain_channel=None):
         camera_geom = self.subarray.tel[tel_id].camera.geometry
         # Pulse-integrate waveforms
         extractor = self.image_extractors[self.image_extractor_type.tel[tel_id]]
diff --git a/src/ctapipe/image/tests/test_reducer.py b/src/ctapipe/image/tests/test_reducer.py
index 49cd06776be..4e2dfddfa30 100644
--- a/src/ctapipe/image/tests/test_reducer.py
+++ b/src/ctapipe/image/tests/test_reducer.py
@@ -33,10 +33,10 @@ def test_null_data_volume_reducer(subarray_lst):
     rng = np.random.default_rng(0)
     waveforms = rng.uniform(0, 1, (1, 2048, 96))
     reducer = NullDataVolumeReducer(subarray=subarray)
-    reduced_waveforms_mask = reducer(waveforms)
-    reduced_waveforms = waveforms.copy()
-    reduced_waveforms[:, ~reduced_waveforms_mask] = 0
-    assert_array_equal(waveforms, reduced_waveforms)
+    reduced_waveforms_mask = reducer(waveforms, tel_id=1)
+
+    # null reducer keeps all pixels
+    assert_array_equal(reduced_waveforms_mask, True)
 
 
 def test_tailcuts_data_volume_reducer(subarray_lst):