From 5abb43393489bd017110eb814d71c0c39a57ef52 Mon Sep 17 00:00:00 2001
From: richardjgowers <richardjgowers@gmail.com>
Date: Thu, 12 Jul 2018 12:13:21 -0500
Subject: [PATCH] added readonly_attributes() context manager to
 ParallelAnalysisBase

ParallelAnalysisBase.run() now applies a lock so that attributes can
no longer be set during `_single_frame()`.  This is to prevent parallel
processes trying to write to the class (impossible anyway).
---
 AUTHORS                    |  2 ++
 CHANGELOG                  |  3 ++-
 docs/conf.py               |  2 +-
 pmda/parallel.py           | 52 +++++++++++++++++++++++++++++---------
 pmda/test/test_parallel.py | 19 ++++++++++++++
 5 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 80c01ed7..d283ee33 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -21,3 +21,5 @@ Chronological list of authors
 
  2018
   - Shujie Fan
+  - Richard J Gowers
+
diff --git a/CHANGELOG b/CHANGELOG
index ced557cf..21572956 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -13,13 +13,14 @@ The rules for this file:
   * release numbers follow "Semantic Versioning" http://semver.org
 
 ------------------------------------------------------------------------------
-xx/xx/18 VOD555
+xx/xx/18 VOD555, richardjgowers
 
   * 0.2.0
 
 Enhancements
   * add add timing for _conclude and _prepare (Issue #49)
   * add parallel particle-particle RDF calculation module pmda.rdf (Issue #41)
+  * add readonly_attributes context manager to ParallelAnalysisBase
 
 
 06/07/18 orbeckst
diff --git a/docs/conf.py b/docs/conf.py
index 6f2c7464..d9fb6398 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -57,7 +57,7 @@
 
 # General information about the project.
 project = u'PMDA'
-author = u'Max Linke, Shujie Fan, Oliver Beckstein'
+author = u'Max Linke, Shujie Fan, Richard J. Gowers, Oliver Beckstein'
 copyright = u'2018, ' + author
 
 
diff --git a/pmda/parallel.py b/pmda/parallel.py
index cb0f88c1..ef930018 100644
--- a/pmda/parallel.py
+++ b/pmda/parallel.py
@@ -15,6 +15,7 @@
 
 """
 from __future__ import absolute_import, division
+from contextlib import contextmanager
 from six.moves import range
 
 import MDAnalysis as mda
@@ -154,6 +155,32 @@ def __init__(self, universe, atomgroups):
         self._traj = universe.trajectory.filename
         self._indices = [ag.indices for ag in atomgroups]
 
+    @contextmanager
+    def readonly_attributes(self):
+        """Set the attributes of this class to be read only
+
+        Useful to avoid the class being modified when passing it around.
+
+        To be used as a context manager::
+
+          with analysis.readonly_attributes():
+              some_function(analysis)
+
+        """
+        self._attr_lock = True
+        yield
+        self._attr_lock = False
+
+    def __setattr__(self, key, val):
+        # guards to stop people assigning to self when they shouldn't
+        # if locked, the only attribute you can modify is _attr_lock
+        # if self._attr_lock isn't set, default to unlocked
+        if key == '_attr_lock' or not getattr(self, '_attr_lock', False):
+            super(ParallelAnalysisBase, self).__setattr__(key, val)
+        else:
+            # raise HalError("I'm sorry Dave, I'm afraid I can't do that")
+            raise AttributeError("Can't set attribute at this time")
+
     def _conclude(self):
         """Finalise the results you've gathered.
 
@@ -259,18 +286,19 @@ def run(self,
                 self._prepare()
             time_prepare = prepare.elapsed
             blocks = []
-            for b in range(n_blocks):
-                task = delayed(
-                    self._dask_helper, pure=False)(
-                        b * bsize * step + start,
-                        min(stop, (b + 1) * bsize * step + start),
-                        step,
-                        self._indices,
-                        self._top,
-                        self._traj, )
-                blocks.append(task)
-            blocks = delayed(blocks)
-            res = blocks.compute(**scheduler_kwargs)
+            with self.readonly_attributes():
+                for b in range(n_blocks):
+                    task = delayed(
+                        self._dask_helper, pure=False)(
+                            b * bsize * step + start,
+                            min(stop, (b + 1) * bsize * step + start),
+                            step,
+                            self._indices,
+                            self._top,
+                            self._traj, )
+                    blocks.append(task)
+                blocks = delayed(blocks)
+                res = blocks.compute(**scheduler_kwargs)
             self._results = np.asarray([el[0] for el in res])
             with timeit() as conclude:
                 self._conclude()
diff --git a/pmda/test/test_parallel.py b/pmda/test/test_parallel.py
index eb4d80c3..15863e58 100644
--- a/pmda/test/test_parallel.py
+++ b/pmda/test/test_parallel.py
@@ -111,3 +111,22 @@ def test_nblocks(analysis, n_blocks):
 def test_guess_nblocks(analysis):
     analysis.run(n_jobs=-1)
     assert len(analysis._results) == joblib.cpu_count()
+
+
+def test_attrlock():
+    u = mda.Universe(PSF, DCD)
+    pab = parallel.ParallelAnalysisBase(u, (u.atoms,))
+
+    # Should initially be allowed to set attributes
+    pab.thing1 = 24
+    assert pab.thing1 == 24
+    # Apply lock
+    with pab.readonly_attributes():
+        # Reading should still work
+        assert pab.thing1 == 24
+        # Setting should fail
+        with pytest.raises(AttributeError):
+            pab.thing2 = 100
+    # Outside of lock context setting should again work
+    pab.thing2 = 100
+    assert pab.thing2 == 100