Merge 2c0a1e6 into 0716fbc

concert/experiments/addons.py

@@ -1,10 +1,14 @@
 """Add-ons for acquisitions are standalone extensions which can be applied to them. They operate on
 the acquired data, e.g. write images to disk, do tomographic reconstruction etc.
 """
+import logging
 from concert.coroutines.filters import queue
 from concert.coroutines.sinks import Accumulate
 
 
+LOG = logging.getLogger(__name__)
+
+
 class Addon(object):
 
     """A base addon class. An addon can be attached, i.e. its functionality is applied to the
@@ -19,21 +23,32 @@ class Addon(object):
 
     def __init__(self, acquisitions):
         self.acquisitions = acquisitions
+        self._attached = False
         self.attach()
 
     def attach(self):
-        """attach adds the addon to an experiment. This means all the necessary operations which
-        provide the addon functionality should be implemented in this method. This mostly means
-        appending consumers to acquisitions.
-        """
-        pass
+        """Attach the addon to all acquisitions."""
+        if self._attached:
+            LOG.debug('Cannot attach an already attached Addon')
+        else:
+            self._attach()
+            self._attached = True
 
     def detach(self):
-        """Unattach removes the addon from an experiment. This means all the necessary operations
-        which provide the addon functionality should be undone by this method. This mostly means
-        removing consumers from acquisitions.
-        """
-        pass
+        """Detach the addon from all acquisitions."""
+        if self._attached:
+            self._detach()
+            self._attached = False
+        else:
+            LOG.debug('Cannot detach an unattached Addon')
+
+    def _attach(self):
+        """Attach implementation."""
+        raise NotImplementedError
+
+    def _detach(self):
+        """Detach implementation."""
+        raise NotImplementedError
 
 
 class Consumer(Addon):
@@ -54,13 +69,13 @@ def __init__(self, acquisitions, consumer):
         self.consumer = consumer
         super(Consumer, self).__init__(acquisitions)
 
-    def attach(self):
-        """attach all acquisitions."""
+    def _attach(self):
+        """Attach all acquisitions."""
         for acq in self.acquisitions:
             acq.consumers.append(self.consumer)
 
-    def detach(self):
-        """Unattach all acquisitions."""
+    def _detach(self):
+        """Detach all acquisitions."""
         for acq in self.acquisitions:
             acq.consumers.remove(self.consumer)
 
@@ -89,17 +104,17 @@ def __init__(self, acquisitions, shapes=None, dtype=None):
         self.items = {}
         super(Accumulator, self).__init__(acquisitions)
 
-    def attach(self):
-        """attach all acquisitions."""
+    def _attach(self):
+        """Attach all acquisitions."""
         shapes = (None,) * len(self.acquisitions) if self._shapes is None else self._shapes
 
         for i, acq in enumerate(self.acquisitions):
             self._accumulators[acq] = Accumulate(shape=shapes[i], dtype=self._dtype)
             self.items[acq] = self._accumulators[acq].items
             acq.consumers.append(self._accumulators[acq])
 
-    def detach(self):
-        """Unattach all acquisitions."""
+    def _detach(self):
+        """Detach all acquisitions."""
         self.items = {}
         for acq in self.acquisitions:
             acq.consumers.remove(self._accumulators[acq])
@@ -130,15 +145,15 @@ def __init__(self, acquisitions, walker, async=True):
         self._writers = {}
         super(ImageWriter, self).__init__(acquisitions)
 
-    def attach(self):
-        """attach all acquisitions."""
+    def _attach(self):
+        """Attach all acquisitions."""
         for acq in self.acquisitions:
             block = True if acq == self.acquisitions[-1] else False
             self._writers[acq] = self._write_sequence(acq, block)
             acq.consumers.append(self._writers[acq])
 
-    def detach(self):
-        """Unattach all acquisitions."""
+    def _detach(self):
+        """Detach all acquisitions."""
         for acq in self.acquisitions:
             acq.consumers.remove(self._writers[acq])
             del self._writers[acq]

concert/tests/integration/test_experiments.py

@@ -10,12 +10,24 @@
 from concert.experiments.base import Acquisition, Experiment, ExperimentError
 from concert.experiments.imaging import (tomo_angular_step, tomo_max_speed,
                                          tomo_projections_number, frames)
-from concert.experiments.addons import Consumer, ImageWriter, Accumulator
+from concert.experiments.addons import Addon, Consumer, ImageWriter, Accumulator
 from concert.devices.cameras.dummy import Camera
 from concert.tests import TestCase, suppressed_logging, assert_almost_equal, VisitChecker
 from concert.storage import DummyWalker
 
 
+class DummyAddon(Addon):
+    def __init__(self):
+        self.attached_num_times = 0
+        super(DummyAddon, self).__init__([])
+
+    def _attach(self):
+        self.attached_num_times += 1
+
+    def _detach(self):
+        self.attached_num_times -= 1
+
+
 @suppressed_logging
 def test_tomo_angular_step():
     truth = np.arctan(2.0 / 100) * q.rad
@@ -215,3 +227,18 @@ def test_accumulation(self):
             for consumer in acq.consumers:
                 self.assertFalse(isinstance(consumer, Accumulate))
         self.assertEquals(acc.items, {})
+
+    def test_attach_num_times(self):
+        """An attached addon cannot be attached the second time."""
+        addon = DummyAddon()
+        # Does nothing because it's attached during construction
+        addon.attach()
+        self.assertEqual(1, addon.attached_num_times)
+
+        # Detach
+        addon.detach()
+        self.assertEqual(0, addon.attached_num_times)
+
+        # Second time, cannot be called
+        addon.detach()
+        self.assertEqual(0, addon.attached_num_times)