Alternative Take on Factories (#916)

* API change - factories

ctapipe/calib/camera/calibrator.py

@@ -5,9 +5,13 @@
 """
 
 from ctapipe.core import Component
-from ctapipe.calib.camera import CameraR1CalibratorFactory, CameraDL0Reducer, \
-    CameraDL1Calibrator
-from ctapipe.image import ChargeExtractorFactory, WaveformCleanerFactory
+from ctapipe.calib.camera import (
+    CameraR1Calibrator,
+    CameraDL0Reducer,
+    CameraDL1Calibrator,
+)
+from ctapipe.image import ChargeExtractor, WaveformCleaner
+
 
 __all__ = ['CameraCalibrator']
 
@@ -44,8 +48,8 @@ class CameraCalibrator(Component):
     """
     def __init__(self, config=None, tool=None,
                  r1_product=None,
-                 extractor_product=None,
-                 cleaner_product=None,
+                 extractor_product='NeighbourPeakIntegrator',
+                 cleaner_product='NullWaveformCleaner',
                  eventsource=None,
                  **kwargs):
         """
@@ -73,33 +77,30 @@ def __init__(self, config=None, tool=None,
         """
         super().__init__(config=config, tool=tool, **kwargs)
 
-        kwargs_ = dict()
-        if extractor_product:
-            kwargs_['product'] = extractor_product
-        extractor = ChargeExtractorFactory.produce(
+        extractor = ChargeExtractor.from_name(
+            extractor_product,
             config=config,
-            tool=tool,
-            **kwargs_
+            tool=tool
         )
 
-        kwargs_ = dict()
-        if cleaner_product:
-            kwargs_['product'] = cleaner_product
-        cleaner = WaveformCleanerFactory.produce(
+        cleaner = WaveformCleaner.from_name(
+            cleaner_product,
             config=config,
             tool=tool,
-            **kwargs_
         )
 
-        kwargs_ = dict()
         if r1_product:
-            kwargs_['product'] = r1_product
-        self.r1 = CameraR1CalibratorFactory.produce(
-            config=config,
-            tool=tool,
-            eventsource=eventsource,
-            **kwargs_
-        )
+            self.r1 = CameraR1Calibrator.from_name(
+                r1_product,
+                config=config,
+                tool=tool,
+            )
+        else:
+            self.r1 = CameraR1Calibrator.from_eventsource(
+                eventsource,
+                config=config,
+                tool=tool,
+            )
 
         self.dl0 = CameraDL0Reducer(config=config, tool=tool)
         self.dl1 = CameraDL1Calibrator(config=config, tool=tool,

ctapipe/calib/camera/gainselection.py

@@ -5,10 +5,10 @@
 
 import numpy as np
 
-from ...core import Component, Factory, traits
+from ...core import Component, traits
 from ...utils import get_table_dataset
 
-__all__ = ['GainSelectorFactory',
+__all__ = ['GainSelector',
            'ThresholdGainSelector',
            'SimpleGainSelector',
            'pick_gain_channel']
@@ -66,7 +66,6 @@ class GainSelector(Component, metaclass=ABCMeta):
     Base class for algorithms that reduce a 2-gain-channel waveform to a
     single waveform.
     """
-
     @abstractclassmethod
     def select_gains(self, cam_id, multi_gain_waveform):
         """
@@ -172,12 +171,3 @@ def select_gains(self, cam_id, multi_gain_waveform):
         )
 
         return waveform, gain_mask
-
-
-class GainSelectorFactory(Factory):
-    """
-    Factory to obtain a GainSelector
-    """
-    base = GainSelector
-    default = 'ThresholdGainSelector'
-    custom_product_help = 'Gain-channel selection scheme to use.'

ctapipe/calib/camera/r1.py

@@ -9,21 +9,21 @@
 As the R1 calibration is camera specific, each camera (and seperately the MC)
 requires their own calibrator class with inherits from `CameraR1Calibrator`.
 `HessioR1Calibrator` is the calibrator for the MC data obtained from readhess.
-Through the use of `CameraR1CalibratorFactory`, the correct
+Through the use of `CameraR1Calibrator.from_eventsource()`, the correct
 `CameraR1Calibrator` can be obtained based on the origin (MC/Camera format)
 of the data.
 """
 from abc import abstractmethod
 import numpy as np
-from ...core import Component, Factory
+
+from ...core import Component
 from ...core.traits import Unicode
-from ...io import EventSource
 
 __all__ = [
     'NullR1Calibrator',
     'HESSIOR1Calibrator',
     'TargetIOR1Calibrator',
-    'CameraR1CalibratorFactory'
+    'CameraR1Calibrator',
 ]
 
 
@@ -47,7 +47,6 @@ class CameraR1Calibrator(Component):
         Set to None if no Tool to pass.
     kwargs
     """
-
     def __init__(self, config=None, tool=None, **kwargs):
         """
         Parent class for the r1 calibrators. Fills the r1 container.
@@ -110,6 +109,32 @@ def check_r0_exists(self, event, telid):
                 self._r0_empty_warn = True
             return False
 
+    @classmethod
+    def from_eventsource(cls, eventsource, **kwargs):
+        """
+        Obtain the correct `CameraR1Calibrator` according the the `EventSource`
+        of the file.
+
+        Parameters
+        ----------
+        eventsource : ctapipe.io.EventSource
+            Subclass of `ctapipe.io.EventSource`
+        kwargs
+            Named arguments to pass to the `CameraR1Calibrator`
+
+        Returns
+        -------
+        calibrator
+            Subclass of `CameraR1Calibrator`
+        """
+        if (hasattr(eventsource, 'metadata') and
+                eventsource.metadata['is_simulation']):
+            return HESSIOR1Calibrator(**kwargs)
+        elif eventsource.__class__.__name__ == "TargetIOEventSource":
+            return TargetIOR1Calibrator(**kwargs)
+
+        return NullR1Calibrator(**kwargs)
+
 
 class NullR1Calibrator(CameraR1Calibrator):
     """
@@ -165,13 +190,13 @@ class HESSIOR1Calibrator(CameraR1Calibrator):
     """
     CALIB_SCALE is only relevant for MC calibration.
 
-    CALIB_SCALE is the factor needed to transform from mean p.e. units to 
-    units of the single-p.e. peak: Depends on the collection efficiency, 
-    the asymmetry of the single p.e. amplitude  distribution and the 
+    CALIB_SCALE is the factor needed to transform from mean p.e. units to
+    units of the single-p.e. peak: Depends on the collection efficiency,
+    the asymmetry of the single p.e. amplitude  distribution and the
     electronic noise added to the signals. Default value is for GCT.
 
     To correctly calibrate to number of photoelectron, a fresh SPE calibration
-    should be applied using a SPE sim_telarray run with an 
+    should be applied using a SPE sim_telarray run with an
     artificial light source.
     """
     # TODO: Handle calib_scale differently per simlated telescope
@@ -311,67 +336,3 @@ def real_calibrate(self, event):
             r1 = event.r1.tel[self.telid].waveform[0]
             self.calibrator.ApplyEvent(samples[0], fci, self._r1_wf[0])
             event.r1.tel[self.telid].waveform = self._r1_wf
-
-
-class CameraR1CalibratorFactory(Factory):
-    """
-    The R1 calibrator `ctapipe.core.factory.Factory`. This
-    `ctapipe.core.factory.Factory` allows the correct
-    `CameraR1Calibrator` to be obtained for the data investigated.
-
-    Additional filepaths are required by some cameras for R1 calibration. Due
-    to the current inplementation of `ctapipe.core.factory.Factory`, every
-    trait that could
-    possibly be required for a child `ctapipe.core.component.Component` of
-    `CameraR1Calibrator` is
-    included in this `ctapipe.core.factory.Factory`. The
-    `CameraR1Calibrator` specific to a
-    camera type should then define how/if that filepath should be used. The
-    format of the file is not restricted, and the file can be read from inside
-    ctapipe, or can call a different library created by the camera teams for
-    the calibration of their camera.
-    """
-    base = CameraR1Calibrator
-    custom_product_help = ('R1 Calibrator to use. If None then a '
-                           'calibrator will either be selected based on the '
-                           'supplied EventSource, or will default to '
-                           '"NullR1Calibrator".')
-
-    def __init__(self, config=None, tool=None, eventsource=None, **kwargs):
-        """
-        Parameters
-        ----------
-        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.
-        tool : ctapipe.core.Tool
-            Tool executable that is calling this component.
-            Passes the correct logger to the component.
-            Set to None if no Tool to pass.
-        eventsource : ctapipe.io.eventsource.EventSource
-            EventSource that is being used to read the events. The EventSource
-            contains information (such as metadata or inst) which indicates
-            the appropriate R1Calibrator to use.
-        kwargs
-
-        """
-
-        super().__init__(config, tool, **kwargs)
-        if eventsource and not issubclass(type(eventsource), EventSource):
-            raise TypeError(
-                "eventsource must be a ctapipe.io.eventsource.EventSource"
-            )
-        self.eventsource = eventsource
-
-    def _get_product_name(self):
-        try:
-            return super()._get_product_name()
-        except AttributeError:
-            es = self.eventsource
-            if es:
-                if es.metadata['is_simulation']:
-                    return 'HESSIOR1Calibrator'
-                elif es.__class__.__name__ == "TargetIOEventSource":
-                    return 'TargetIOR1Calibrator'
-            return 'NullR1Calibrator'

ctapipe/calib/camera/tests/test_r1.py

@@ -3,10 +3,10 @@
     assert_array_almost_equal
 
 from ctapipe.calib.camera.r1 import (
-    CameraR1CalibratorFactory,
+    CameraR1Calibrator,
     HESSIOR1Calibrator,
     TargetIOR1Calibrator,
-    NullR1Calibrator
+    NullR1Calibrator,
 )
 from ctapipe.io.eventsource import EventSource
 from ctapipe.io.simteleventsource import SimTelEventSource
@@ -42,7 +42,7 @@ def test_targetio_calibrator():
     source_r0 = TargetIOEventSource(input_url=url_r0)
     source_r1 = TargetIOEventSource(input_url=url_r1)
 
-    r1c = CameraR1CalibratorFactory.produce(eventsource=source_r0)
+    r1c = CameraR1Calibrator.from_eventsource(eventsource=source_r0)
 
     event_r0 = source_r0._get_event_by_index(0)
     event_r1 = source_r1._get_event_by_index(0)
@@ -51,7 +51,7 @@ def test_targetio_calibrator():
     assert_array_equal(event_r0.r0.tel[0].waveform,
                        event_r0.r1.tel[0].waveform)
 
-    r1c = CameraR1CalibratorFactory.produce(
+    r1c = CameraR1Calibrator.from_eventsource(
         eventsource=source_r0,
         pedestal_path=pedpath
     )
@@ -77,38 +77,19 @@ def test_check_r0_exists(example_event):
 
 
 def test_factory_from_product():
-    calibrator = CameraR1CalibratorFactory.produce(
-        product="NullR1Calibrator"
-    )
+    calibrator = CameraR1Calibrator.from_name("NullR1Calibrator")
     assert isinstance(calibrator, NullR1Calibrator)
-    calibrator = CameraR1CalibratorFactory.produce(
-        product="HESSIOR1Calibrator"
-    )
+    calibrator = CameraR1Calibrator.from_name("HESSIOR1Calibrator")
     assert isinstance(calibrator, HESSIOR1Calibrator)
 
 
-def test_factory_default():
-    calibrator = CameraR1CalibratorFactory.produce()
-    assert isinstance(calibrator, NullR1Calibrator)
-
-
-def test_factory_from_eventsource():
+def test_factory_for_eventsource():
     dataset = get_dataset_path("gamma_test.simtel.gz")
     eventsource = SimTelEventSource(input_url=dataset)
-    calibrator = CameraR1CalibratorFactory.produce(eventsource=eventsource)
+    calibrator = CameraR1Calibrator.from_eventsource(eventsource=eventsource)
     assert isinstance(calibrator, HESSIOR1Calibrator)
 
 
-def test_factory_from_eventsource_override():
-    dataset = get_dataset_path("gamma_test.simtel.gz")
-    eventsource = SimTelEventSource(input_url=dataset)
-    calibrator = CameraR1CalibratorFactory.produce(
-        eventsource=eventsource,
-        product="NullR1Calibrator"
-    )
-    assert isinstance(calibrator, NullR1Calibrator)
-
-
 class UnknownEventSource(EventSource):
     """
     Simple working EventSource
@@ -125,5 +106,5 @@ def is_compatible(file_path):
 def test_factory_from_unknown_eventsource():
     dataset = get_dataset_path("gamma_test.simtel.gz")
     eventsource = UnknownEventSource(input_url=dataset)
-    calibrator = CameraR1CalibratorFactory.produce(eventsource=eventsource)
+    calibrator = CameraR1Calibrator.from_eventsource(eventsource=eventsource)
     assert isinstance(calibrator, NullR1Calibrator)

ctapipe/core/__init__.py

@@ -1,10 +1,17 @@
 # Licensed under a 3-clause BSD style license - see LICENSE.rst
 
-from .component import Component
+from .component import Component, non_abstract_children
 from .container import Container, Field, Map
-from .factory import Factory
 from .provenance import Provenance
 from .tool import Tool, ToolConfigurationError
 
-__all__ = ['Component', 'Container', 'Tool', 'Field', 'Map', 'Factory',
-           'Provenance', 'ToolConfigurationError']
+__all__ = [
+    'Component',
+    'Container',
+    'Tool',
+    'Field',
+    'Map',
+    'Provenance',
+    'ToolConfigurationError',
+    'non_abstract_children',
+]