From 8c5cd1b900e3d02dce58439a7bbbb429efa3fee4 Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Mon, 10 Jul 2023 10:01:41 +0200
Subject: [PATCH 01/15] Add an interface to turn objects read-only

---
 pyiron_base/interfaces/lockable.py | 222 +++++++++++++++++++++++++++++
 1 file changed, 222 insertions(+)
 create mode 100644 pyiron_base/interfaces/lockable.py

diff --git a/pyiron_base/interfaces/lockable.py b/pyiron_base/interfaces/lockable.py
new file mode 100644
index 000000000..330f279d6
--- /dev/null
+++ b/pyiron_base/interfaces/lockable.py
@@ -0,0 +1,222 @@
+"""
+A small mixin to lock attribute and method access at runtime.
+
+Sometimes we wish to restrict users of pyiron from changing certain things past certain stages of object lifetime, e.g.
+the input of jobs should only be changed before it is run, but still need to be able to change them internally.  This
+can be implemented with :class:`~Lockable` and the decorator :func:`~sentinel`.  It should be thought of as a well
+defined escape hatch that is rarely necessary.  Users should never be expected to unlock an object ever again after it
+has been locked by them or pyiron.
+
+The context manager functionality is implemented in a separate class rather than directly on Lockable to conserve dunder
+name space real estate and let subclasses be context managers on their own.
+"""
+
+from abc import ABC
+from functools import wraps
+
+from pyiron_base.interfaces.has_groups import HasGroups
+
+class Locked(Exception):
+    pass
+
+def sentinel(meth):
+    @wraps(meth)
+    def f(self, *args, **kwargs):
+        if self.read_only:
+            raise Locked("Object is currently locked!  Use unlocked() if you know what you are doing.")
+        else:
+            return meth(self, *args, **kwargs)
+    return f
+
+class _UnlockContext:
+    __slots__ = ("owner",)
+
+    def __init__(self, owner):
+        self.owner = owner
+
+    def __enter__(self):
+        self.owner.read_only = False
+        return self.owner
+
+    def __exit__(self, *_):
+        self.owner.lock()
+        return False # never handle exceptions
+
+def _iterate_lockable_subs(lockable_groups):
+    if not isinstance(lockable_groups, HasGroups):
+        return
+
+    subs = lockable_groups.list_all()
+    for n in subs["nodes"]:
+        node = lockable_groups[n]
+        if isinstance(node, Lockable):
+            yield node
+
+    for g in subs["groups"]:
+        group = lockable_groups[g]
+        yield group
+        yield from _iterate_lockable_subs(g)
+
+class Lockable:
+    """
+    A small mixin to lock attribute and method access at runtime.
+
+    The mixin maintains an :attr:`~.read_only` and offers a context manager to temporarily unset it.  It does **not**
+    restrict access to any attributes or methods on its own.  Instead sub classes are expected to mark methods they wish
+    protected with :func:`.sentinel`.  Wrapped methods will then raise :exc:`.Locked` if :attr:`~.read_only` is set.
+
+    If the subclass also implements :class:`.HasGroups`, locking it will iterate over all nodes and (recursively)
+    groups and lock them if possible and vice-versa for unlocking.
+
+    Once an object has been locked it should generally not be expected to be (permanently) unlocked again, especially
+    not explicitely by the user.
+
+    Subclasses need to initialize this class by calling the inherited `__init__`, if explicitely overriding it.  When
+    not explicitely overriding it (as in the examples below), take care that either the other super classes call
+    `super().__init__` or place this class before them in the inheritance order.
+
+    Let's start with a simple example; a list that can be locked
+
+    >>> class LockList(Lockable, list):
+    ...   __setitem__ = sentinel(list.__setitem__)
+    ...   clear = sentinel(list.clear)
+    >>> l = LockList([1,2,3])
+    >>> l
+    [1, 2, 3]
+
+    Deriving adds the read only flag
+
+    >>> l.read_only
+    False
+
+    While it is not set, we may mutate the object
+
+    >>> l[2] = 4
+    >>> l[2]
+    4
+
+    Once it is locked, the wrapped methods will raise errors
+
+    >>> l.lock()
+    >>> l.read_only
+    True
+    >>> l[1]
+    2
+    >>> l[1] = 4
+    Traceback (most recent call last):
+        ...
+    lockable.Locked: Object is currently locked!  Use unlocked() if you know what you are doing.
+
+    You can lock an object multiple times to no effect
+
+    >>> l.lock()
+
+    From now on every modification should be done with the :meth:`~.unlocked()` context manager.  It returns the
+    unlocked object itself.
+
+    >>> with l.unlocked():
+    ...   l[1] = 4
+    >>> l[1]
+    4
+    >>> with l.unlocked() as lopen:
+    ...   print(l is lopen)
+    ...   l[1] = 4
+    True
+
+    :func:`~.sentinel` can be used for methods, item and attribute access.
+
+    >>> l.clear()
+    Traceback (most recent call last):
+        ...
+    lockable.Locked: Object is currently locked!  Use unlocked() if you know what you are doing.
+    >>> with l.unlocked():
+    >>>    l.clear()
+    >>> l
+    []
+
+    When used together with :class:`.HasGroups`, objects will be locked recursively.
+
+    >>> class LockGroupDict(Lockable, dict, HasGroups):
+    ...   __setitem__ = sentinel(dict.__setitem__)
+    ...
+    ...   def _list_groups(self):
+    ...     return [k for k, v in self.items() if isinstance(v, LockGroupDict)]
+    ...
+    ...   def _list_nodes(self):
+    ...     return [k for k, v in self.items() if not isinstance(v, LockGroupDict)]
+
+    >>> d = LockGroupDict(a=dict(c=1, d=2), b=LockGroupDict(c=1, d=2))
+    >>> d.lock()
+
+    Since the first item is a plain dict, it can still be mutated.
+
+    >>> type(d['a'])
+    dict
+    >>> d['a']['c'] = 23
+    >>> d['a']['c']
+    23
+
+    Where as the second will be locked from now on
+
+    >>> type(d['b'])
+    LockGroupDict
+    >>> d['b']['c'] = 23
+    Traceback (most recent call last):
+        ...
+    lockable.Locked: Object is currently locked!  Call unlock() if you know what you are doing.
+    >>> d['b']['c']
+    1
+
+    but we can unlock it as usual
+
+    >>> with d.unlocked() as dopen:
+    ...   dopen['b']['d'] = 23
+    >>> d['b']['d']
+    23
+    """
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        object.__setattr__(self, "_read_only", False)
+
+    @property
+    def read_only(self) -> bool:
+        """
+        bool: False if the object can currently be written to
+
+        Setting this value will trigger :meth:`._on_lock` and :meth:`._on_unlock` if it changes.
+        """
+        return object.__getattribute__(self, "_read_only")
+
+    @read_only.setter
+    def read_only(self, value: bool):
+        changed = self._read_only != value
+        self._read_only = value
+        if changed:
+            if value:
+                self._on_lock()
+            else:
+                self._on_unlock()
+
+    def _on_lock(self):
+        if isinstance(self, HasGroups):
+            for it in _iterate_lockable_subs(self):
+                it.lock()
+
+    def _on_unlock(self):
+        for it in _iterate_lockable_subs(self):
+            it.read_only = False
+
+    def lock(self):
+        """
+        Set :attr:`~.read_only`.
+        """
+        self.read_only = True
+
+    def unlocked(self) -> _UnlockContext:
+        """
+        Unlock the object temporarily.
+
+        Context manager returns this object again.
+        """
+        return _UnlockContext(self)

From 1b3849e6fd7e169101342b769a7a8e266838831f Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Mon, 10 Jul 2023 12:23:56 +0200
Subject: [PATCH 02/15] More docstrings

---
 pyiron_base/interfaces/lockable.py | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/pyiron_base/interfaces/lockable.py b/pyiron_base/interfaces/lockable.py
index 330f279d6..dfad46806 100644
--- a/pyiron_base/interfaces/lockable.py
+++ b/pyiron_base/interfaces/lockable.py
@@ -20,6 +20,17 @@ class Locked(Exception):
     pass
 
 def sentinel(meth):
+    """
+    Wrap a method to fail if `read_only` is `True` on the owning object.
+
+    Use together with :class:`Lockable`.
+
+    Args:
+        meth (method): method to call if `read_only` is `False`.
+
+    Returns:
+        wrapped method
+    """
     @wraps(meth)
     def f(self, *args, **kwargs):
         if self.read_only:
@@ -29,6 +40,11 @@ def f(self, *args, **kwargs):
     return f
 
 class _UnlockContext:
+    """
+    Context manager that unlocks and relocks a :class:`Lockable`.
+
+    This is an implementation detail of :class:`Lockable`.
+    """
     __slots__ = ("owner",)
 
     def __init__(self, owner):
@@ -43,6 +59,11 @@ def __exit__(self, *_):
         return False # never handle exceptions
 
 def _iterate_lockable_subs(lockable_groups):
+    """
+    Yield sub nodes and groups that are lockable.  Recurse into groups.
+
+    If the given object is not a :class:`HasGroups` yield nothing.
+    """
     if not isinstance(lockable_groups, HasGroups):
         return
 
@@ -199,9 +220,8 @@ def read_only(self, value: bool):
                 self._on_unlock()
 
     def _on_lock(self):
-        if isinstance(self, HasGroups):
-            for it in _iterate_lockable_subs(self):
-                it.lock()
+        for it in _iterate_lockable_subs(self):
+            it.lock()
 
     def _on_unlock(self):
         for it in _iterate_lockable_subs(self):

From 4dbc3859b71ff26989377cb64b9f69efa8803e64 Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Mon, 10 Jul 2023 12:30:15 +0200
Subject: [PATCH 03/15] Remove ABC

---
 pyiron_base/interfaces/lockable.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pyiron_base/interfaces/lockable.py b/pyiron_base/interfaces/lockable.py
index dfad46806..13993a3ef 100644
--- a/pyiron_base/interfaces/lockable.py
+++ b/pyiron_base/interfaces/lockable.py
@@ -11,7 +11,6 @@
 name space real estate and let subclasses be context managers on their own.
 """
 
-from abc import ABC
 from functools import wraps
 
 from pyiron_base.interfaces.has_groups import HasGroups

From 63e5bf5aa4662752d34a7514b840e5b4b4fc2a05 Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Mon, 10 Jul 2023 13:00:32 +0200
Subject: [PATCH 04/15] Use lockable in Server

---
 .../jobs/job/extension/server/generic.py      | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/pyiron_base/jobs/job/extension/server/generic.py b/pyiron_base/jobs/job/extension/server/generic.py
index 2c763942f..4cc214412 100644
--- a/pyiron_base/jobs/job/extension/server/generic.py
+++ b/pyiron_base/jobs/job/extension/server/generic.py
@@ -12,6 +12,7 @@
 from typing import Union
 
 from pyiron_base.state import state
+from pyiron_base.interfaces.lockable import Lockable, sentinel
 from pyiron_base.jobs.job.extension.server.runmode import Runmode
 
 __author__ = "Jan Janssen"
@@ -26,7 +27,7 @@
 __date__ = "Sep 1, 2017"
 
 
-class Server:  # add the option to return the job id and the hold id to the server object
+class Server(Lockable):  # add the option to return the job id and the hold id to the server object
     """
     Generic Server object to handle the execution environment for the job
 
@@ -117,6 +118,7 @@ def __init__(
         self._structure_id = None
         self._accept_crash = False
         self.additional_arguments = {}
+        super().__init__()
 
     @property
     def send_to_db(self):
@@ -129,6 +131,7 @@ def send_to_db(self):
         return self._send_to_db
 
     @send_to_db.setter
+    @sentinel
     def send_to_db(self, send):
         """
         Set the boolean option to decide which jobs should be store in the external/public database
@@ -143,6 +146,7 @@ def accept_crash(self):
         return self._accept_crash
 
     @accept_crash.setter
+    @sentinel
     def accept_crash(self, accept):
         self._accept_crash = accept
 
@@ -157,6 +161,7 @@ def structure_id(self):
         return self._structure_id
 
     @structure_id.setter
+    @sentinel
     def structure_id(self, structure_id):
         """
         Set the structure ID to be linked to an external/public database
@@ -177,6 +182,7 @@ def queue(self):
         return self._active_queue
 
     @queue.setter
+    @sentinel
     def queue(self, new_scheduler):
         """
         Set a queue for the current simulation, by choosing one of the options
@@ -235,6 +241,7 @@ def queue_id(self):
         return self._queue_id
 
     @queue_id.setter
+    @sentinel
     def queue_id(self, qid):
         """
         Set the queue ID
@@ -255,6 +262,7 @@ def threads(self):
         return self._threads
 
     @threads.setter
+    @sentinel
     def threads(self, number_of_threads):
         """
         The number of threads selected for the current simulation
@@ -275,6 +283,7 @@ def gpus(self):
         return self._gpus
 
     @gpus.setter
+    @sentinel
     def gpus(self, number_of_gpus):
         """
         Total number of GPUs to use for this calculation.
@@ -295,6 +304,7 @@ def cores(self):
         return self._cores
 
     @cores.setter
+    @sentinel
     def cores(self, new_cores):
         """
         The number of cores selected for the current simulation
@@ -334,6 +344,7 @@ def run_time(self):
         return self._run_time
 
     @run_time.setter
+    @sentinel
     def run_time(self, new_run_time):
         """
         The run time in seconds selected for the current simulation
@@ -365,6 +376,7 @@ def memory_limit(self):
         return self._memory_limit
 
     @memory_limit.setter
+    @sentinel
     def memory_limit(self, limit):
         if state.queue_adapter is not None and self._active_queue is not None:
             memory_max = state.queue_adapter.check_queue_parameters(
@@ -393,6 +405,7 @@ def run_mode(self):
         return self._run_mode
 
     @run_mode.setter
+    @sentinel
     def run_mode(self, new_mode):
         """
         Set the run mode of the job
@@ -420,6 +433,7 @@ def new_hdf(self):
         return self._new_hdf
 
     @new_hdf.setter
+    @sentinel
     def new_hdf(self, new_hdf_bool):
         """
         New_hdf5 defines whether a subjob should be stored in the same HDF5 file or in a new one.
@@ -467,6 +481,7 @@ def executor(self) -> Union[Executor, None]:
         return self._executor
 
     @executor.setter
+    @sentinel
     def executor(self, exe: Union[Executor, None]):
         """
         Executor to execute the job object this server object is attached to in the background.
@@ -496,6 +511,8 @@ def future(self) -> Union[Future, None]:
         """
         return self._future
 
+    # We don't wrap future in sentinel, to allow it later to be dropped to
+    # None, once execution is finished
     @future.setter
     def future(self, future_obj: Future):
         """

From cc0965de39ded22bf7ba958d8506281333612200 Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Tue, 11 Jul 2023 00:41:03 +0200
Subject: [PATCH 05/15] Fix/Add doctests; fix Server init

---
 pyiron_base/interfaces/lockable.py            | 34 ++++++++++++++++---
 .../jobs/job/extension/server/generic.py      |  2 +-
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/pyiron_base/interfaces/lockable.py b/pyiron_base/interfaces/lockable.py
index 13993a3ef..f8a2fdafc 100644
--- a/pyiron_base/interfaces/lockable.py
+++ b/pyiron_base/interfaces/lockable.py
@@ -93,7 +93,8 @@ class Lockable:
 
     Subclasses need to initialize this class by calling the inherited `__init__`, if explicitely overriding it.  When
     not explicitely overriding it (as in the examples below), take care that either the other super classes call
-    `super().__init__` or place this class before them in the inheritance order.
+    `super().__init__` or place this class before them in the inheritance order.  Also be sure to initialize it before
+    using methods and properties decorated with :func:`.sentinel`.
 
     Let's start with a simple example; a list that can be locked
 
@@ -150,7 +151,7 @@ class Lockable:
         ...
     lockable.Locked: Object is currently locked!  Use unlocked() if you know what you are doing.
     >>> with l.unlocked():
-    >>>    l.clear()
+    ...   l.clear()
     >>> l
     []
 
@@ -171,7 +172,7 @@ class Lockable:
     Since the first item is a plain dict, it can still be mutated.
 
     >>> type(d['a'])
-    dict
+    <class 'dict'>
     >>> d['a']['c'] = 23
     >>> d['a']['c']
     23
@@ -179,11 +180,11 @@ class Lockable:
     Where as the second will be locked from now on
 
     >>> type(d['b'])
-    LockGroupDict
+    <class 'lockable.LockGroupDict'>
     >>> d['b']['c'] = 23
     Traceback (most recent call last):
         ...
-    lockable.Locked: Object is currently locked!  Call unlock() if you know what you are doing.
+    lockable.Locked: Object is currently locked!  Use unlocked() if you know what you are doing.
     >>> d['b']['c']
     1
 
@@ -193,6 +194,29 @@ class Lockable:
     ...   dopen['b']['d'] = 23
     >>> d['b']['d']
     23
+
+    To use this class with properties, simply decorate the setter
+
+    >>> class MyLock(Lockable):
+    ...   def __init__(self, foo):
+    ...     super().__init__()
+    ...     self._foo = foo
+    ...   @property
+    ...   def foo(self):
+    ...     return self._foo
+    ...   @foo.setter
+    ...   @sentinel
+    ...   def foo(self, value):
+    ...     self._foo = value
+    >>> ml = MyLock(42)
+    >>> ml.foo
+    42
+    >>> ml.foo = 23
+    >>> ml.lock()
+    >>> ml.foo = 42
+    Traceback (most recent call last):
+        ...
+    lockable.Locked: Object is currently locked!  Use unlocked() if you know what you are doing.
     """
 
     def __init__(self, *args, **kwargs):
diff --git a/pyiron_base/jobs/job/extension/server/generic.py b/pyiron_base/jobs/job/extension/server/generic.py
index 4cc214412..4138128fe 100644
--- a/pyiron_base/jobs/job/extension/server/generic.py
+++ b/pyiron_base/jobs/job/extension/server/generic.py
@@ -95,6 +95,7 @@ def __init__(
         run_mode="modal",
         new_hdf=True,
     ):
+        super().__init__()
         self._cores = cores
         self._threads = threads
         self._active_queue = None
@@ -118,7 +119,6 @@ def __init__(
         self._structure_id = None
         self._accept_crash = False
         self.additional_arguments = {}
-        super().__init__()
 
     @property
     def send_to_db(self):

From 0bc8e4bb7bad2987ab37a1dfccc02574b3cd699a Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Fri, 21 Jul 2023 17:13:07 +0200
Subject: [PATCH 06/15] Allow to issue warning; special case __setattr__

Sub classes that use sentinel on __setattr__ need to special case when
the attribute to set is `read_only` or `_read_only`.  Failure to do so
would result in being unable to unlock the object.  `sentinel` now
handles this.

Additionally adds a `lock_method`.  Default is `error`, but can be set
to emit a warning and allow modification instead.
---
 pyiron_base/interfaces/lockable.py | 72 ++++++++++++++++++++++++++----
 1 file changed, 63 insertions(+), 9 deletions(-)

diff --git a/pyiron_base/interfaces/lockable.py b/pyiron_base/interfaces/lockable.py
index f8a2fdafc..f8e8ea8dc 100644
--- a/pyiron_base/interfaces/lockable.py
+++ b/pyiron_base/interfaces/lockable.py
@@ -9,9 +9,15 @@
 
 The context manager functionality is implemented in a separate class rather than directly on Lockable to conserve dunder
 name space real estate and let subclasses be context managers on their own.
+
+Through out the code inside methods of `Lockable` will use `object.__setattr__`
+and `object.__getattribute__` to avoid any overloading attribute access that
+sibling classes may bring in.
 """
 
+from typing import Optional, Literal
 from functools import wraps
+import warnings
 
 from pyiron_base.interfaces.has_groups import HasGroups
 
@@ -30,12 +36,35 @@ def sentinel(meth):
     Returns:
         wrapped method
     """
-    @wraps(meth)
-    def f(self, *args, **kwargs):
-        if self.read_only:
+    def dispatch_or_error(self, *args, **kwargs):
+        try:
+            method = object.__getattribute__(self, "_lock_method")
+        except AttributeError:
+            method = None
+        if method not in ("error", "warning"):
+            method = "error"
+        if self.read_only and method == "error":
             raise Locked("Object is currently locked!  Use unlocked() if you know what you are doing.")
-        else:
-            return meth(self, *args, **kwargs)
+        elif self.read_only and method == "warning":
+            warnings.warn(
+                    f"{meth.__name__} called on {type(self)}, but object is locked!"
+            )
+        return meth(self, *args, **kwargs)
+
+    # if sentinel is applied to __setattr__ we must ensure that `read_only`
+    # stays available, otherwise we can't unlock again later
+    if meth.__name__ == "__setattr__":
+        @wraps(meth)
+        def f(self, *args, **kwargs):
+            if len(args) > 0:
+                target = args[0]
+            else:
+                target = kwargs["name"]
+            if target in ("read_only", "_read_only"):
+                return meth(self, *args, **kwargs)
+            return dispatch_or_error(self, *args, **kwargs)
+    else:
+        f = wraps(meth)(dispatch_or_error)
     return f
 
 class _UnlockContext:
@@ -96,6 +125,9 @@ class Lockable:
     `super().__init__` or place this class before them in the inheritance order.  Also be sure to initialize it before
     using methods and properties decorated with :func:`.sentinel`.
 
+    Subclasses may override :meth:`_on_lock` and :meth:`_on_unlock` if they wish to customize locking/unlocking
+    behaviour, provided that they call super() in their overloads.
+
     Let's start with a simple example; a list that can be locked
 
     >>> class LockList(Lockable, list):
@@ -217,11 +249,31 @@ class Lockable:
     Traceback (most recent call last):
         ...
     lockable.Locked: Object is currently locked!  Use unlocked() if you know what you are doing.
+
+    It's possible to change the errors raised into a warning and allow
+    modification by passing `lock_method` to :meth:`~.Lockable.__init__` or
+    `method` to :meth:`~.lock`.
+
+    >>> mw = LockList(lock_method="warning")
+    >>> mw.append(0)
+    >>> mw.lock()
+    >>> mw[0] = 1 # will print the warning
+    >>> mw[0]
+    1
+
+    >>> mw = LockList()
+    >>> mw.append(0)
+    >>> mw.lock(method='warning')
+    >>> mw[0] = 1 # will print the warning
+    >>> mw[0]
+    1
+
     """
 
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
+    def __init__(self, *args, lock_method: str = "error", **kwargs):
         object.__setattr__(self, "_read_only", False)
+        object.__setattr__(self, "_lock_method", lock_method)
+        super().__init__(*args, **kwargs)
 
     @property
     def read_only(self) -> bool:
@@ -235,8 +287,8 @@ def read_only(self) -> bool:
     @read_only.setter
     def read_only(self, value: bool):
         changed = self._read_only != value
-        self._read_only = value
         if changed:
+            self._read_only = value
             if value:
                 self._on_lock()
             else:
@@ -250,10 +302,12 @@ def _on_unlock(self):
         for it in _iterate_lockable_subs(self):
             it.read_only = False
 
-    def lock(self):
+    def lock(self, method: Optional[Literal["error", "warning"]] = None):
         """
         Set :attr:`~.read_only`.
         """
+        if method is not None:
+            object.__setattr__(self, "_lock_method", method)
         self.read_only = True
 
     def unlocked(self) -> _UnlockContext:

From e26c89a544682dafae9222fa8ff026ec26a5329c Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Fri, 21 Jul 2023 17:16:07 +0200
Subject: [PATCH 07/15] Use Lockable in DataContainer

---
 pyiron_base/storage/datacontainer.py | 48 ++++++++++++----------------
 tests/generic/test_datacontainer.py  |  3 +-
 2 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/pyiron_base/storage/datacontainer.py b/pyiron_base/storage/datacontainer.py
index 1a93ae95b..8823eb5ce 100644
--- a/pyiron_base/storage/datacontainer.py
+++ b/pyiron_base/storage/datacontainer.py
@@ -17,6 +17,7 @@
 from pyiron_base.storage.hdfstub import HDFStub
 from pyiron_base.interfaces.has_groups import HasGroups
 from pyiron_base.interfaces.has_hdf import HasHDF
+from pyiron_base.interfaces.lockable import Lockable, sentinel
 
 __author__ = "Marvin Poul"
 __copyright__ = (
@@ -46,7 +47,7 @@ def _normalize(key):
     return key
 
 
-class DataContainer(MutableMapping, HasGroups, HasHDF):
+class DataContainer(MutableMapping, Lockable, HasGroups, HasHDF):
     """
     Mutable sequence with optional keys.
 
@@ -252,7 +253,6 @@ def __new__(cls, *args, **kwargs):
         object.__setattr__(instance, "_store", [])
         object.__setattr__(instance, "_indices", {})
         object.__setattr__(instance, "table_name", None)
-        object.__setattr__(instance, "_read_only", False)
         object.__setattr__(instance, "_lazy", False)
 
         return instance
@@ -269,6 +269,7 @@ def __init__(self, init=None, table_name=None, lazy=False, wrap_blacklist=()):
             wrap_blacklist (tuple of types): any values in `init` that are instances of the given types are *not*
                                              wrapped in :class:`.DataContainer`
         """
+        super().__init__(lock_method="warning")
         self.table_name = table_name
         self._lazy = lazy
         if init is not None:
@@ -317,9 +318,8 @@ def __getitem__(self, key):
         else:
             raise ValueError("{} is not a valid key, must be str or int".format(key))
 
+    @sentinel
     def __setitem__(self, key, val):
-        if self.read_only:
-            self._read_only_error()
 
         key = _normalize(key)
 
@@ -347,9 +347,8 @@ def __setitem__(self, key, val):
         else:
             raise ValueError("{} is not a valid key, must be str or int".format(key))
 
+    @sentinel
     def __delitem__(self, key):
-        if self.read_only:
-            self._read_only_error()
 
         key = _normalize(key)
 
@@ -387,6 +386,7 @@ def __getattr__(self, name):
     def _is_class_var(cls, name):
         return any(name in c.__dict__ for c in cls.__mro__)
 
+    @sentinel
     def __setattr__(self, name, val):
         # Search instance variables (self.__dict___) and class variables
         # (self.__class__.__dict__ + iterating over mro to find variables on
@@ -398,6 +398,7 @@ def __setattr__(self, name, val):
         else:
             self[name] = val
 
+    @sentinel
     def __delattr__(self, name):
         # see __setattr__
         if name in self.__dict__ or self._is_class_var(name):
@@ -429,28 +430,6 @@ def __repr__(self):
         else:
             return name + "([" + ", ".join("{!r}".format(v) for v in self._store) + "])"
 
-    @property
-    def read_only(self):
-        """
-        bool: if set, raise warning when attempts are made to modify the container
-        """
-        return self._read_only
-
-    @read_only.setter
-    def read_only(self, val):
-        # can't mark a read-only list as writeable
-        if self._read_only and not val:
-            self._read_only_error()
-        else:
-            self._read_only = bool(val)
-
-    @classmethod
-    def _read_only_error(cls):
-        warnings.warn(
-            "The input in {} changed, while the state of the job was already "
-            "finished.".format(cls.__name__)
-        )
-
     def to_builtin(self, stringify=False):
         """
         Convert the container back to builtin dict's and list's recursively.
@@ -596,6 +575,7 @@ def _wrap_val(cls, val, blacklist):
         else:
             return val
 
+    @sentinel
     def update(self, init, wrap=False, blacklist=(), **kwargs):
         """
         Add all elements or key-value pairs from init to this container.  If wrap is
@@ -639,6 +619,7 @@ def update(self, init, wrap=False, blacklist=(), **kwargs):
         else:
             super().update(init, **kwargs)
 
+    @sentinel
     def append(self, val):
         """
         Add new value to the container without a key.
@@ -648,6 +629,7 @@ def append(self, val):
         """
         self._store.append(val)
 
+    @sentinel
     def extend(self, vals):
         """
         Append vals to the end of this DataContainer.
@@ -659,6 +641,7 @@ def extend(self, vals):
         for v in vals:
             self.append(v)
 
+    @sentinel
     def insert(self, index, val, key=None):
         """
         Add a new element to the container at the specified position, with an optional
@@ -679,6 +662,7 @@ def insert(self, index, val, key=None):
 
         self._store.insert(index, val)
 
+    @sentinel
     def mark(self, index, key):
         """
         Add a key to an existing item at index.  If key already exists, it is
@@ -705,6 +689,7 @@ def mark(self, index, key):
 
         self._indices[key] = index
 
+    @sentinel
     def clear(self):
         """
         Remove all items from DataContainer.
@@ -712,6 +697,7 @@ def clear(self):
         self._store.clear()
         self._indices.clear()
 
+    @sentinel
     def create_group(self, name):
         """
         Add a new empty subcontainer under the given key.
@@ -889,6 +875,7 @@ def groups(self):
     def _list_groups(self):
         return list(self.groups())
 
+    @sentinel
     def read(self, file_name, wrap=True):
         """
         Parse file as dictionary and add its keys to this container.
@@ -932,6 +919,11 @@ def _force_load(self, recursive=True):
             if recursive and isinstance(v, DataContainer):
                 v._force_load()
 
+    # Lockable overload
+    def _on_unlock(self):
+        warnings.warn("Unlock previously locked object!")
+        super()._on_unlock()
+
     def __init_subclass__(cls):
         # called whenever a subclass of DataContainer is defined, then register all subclasses with the same function
         # that the DataContainer is registered
diff --git a/tests/generic/test_datacontainer.py b/tests/generic/test_datacontainer.py
index 2056ff879..cf8de9a9d 100644
--- a/tests/generic/test_datacontainer.py
+++ b/tests/generic/test_datacontainer.py
@@ -313,7 +313,8 @@ def test_del_item(self):
 
     def test_del_attr(self):
         class SubDataContainer(DataContainer):
-            def __init__(self):
+            def __init__(self, *args, **kwargs):
+                super().__init__(*args, **kwargs)
                 object.__setattr__(self, "attr", 42)
         s = SubDataContainer()
         del s.attr

From e0a957233df0ac7ea9904d7fa01c05fbb9e54851 Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Fri, 21 Jul 2023 17:30:14 +0200
Subject: [PATCH 08/15] Lock server together with input

This requires a few supers where subclasses were previously not diligent about it
---
 pyiron_base/jobs/job/generic.py               | 2 +-
 pyiron_base/jobs/master/generic.py            | 1 +
 pyiron_base/jobs/master/interactivewrapper.py | 1 +
 pyiron_base/jobs/script.py                    | 1 +
 4 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/pyiron_base/jobs/job/generic.py b/pyiron_base/jobs/job/generic.py
index 82c836ca2..cc28e9fa1 100644
--- a/pyiron_base/jobs/job/generic.py
+++ b/pyiron_base/jobs/job/generic.py
@@ -1264,7 +1264,7 @@ def set_input_to_read_only(self):
         This function enforces read-only mode for the input classes, but it has to be implemented in the individual
         classes.
         """
-        pass
+        self.server.lock()
 
     def _run_if_busy(self):
         """
diff --git a/pyiron_base/jobs/master/generic.py b/pyiron_base/jobs/master/generic.py
index 69a3f9916..381fc9e1e 100644
--- a/pyiron_base/jobs/master/generic.py
+++ b/pyiron_base/jobs/master/generic.py
@@ -143,6 +143,7 @@ def job_object_dict(self):
 
     @wraps(GenericJob.set_input_to_read_only)
     def set_input_to_read_only(self):
+        super().set_input_to_read_only()
         self._input.read_only = True
 
     def first_child_name(self):
diff --git a/pyiron_base/jobs/master/interactivewrapper.py b/pyiron_base/jobs/master/interactivewrapper.py
index a02b775ca..61ea211fc 100644
--- a/pyiron_base/jobs/master/interactivewrapper.py
+++ b/pyiron_base/jobs/master/interactivewrapper.py
@@ -59,6 +59,7 @@ def ref_job(self, ref_job):
         self.append(ref_job)
 
     def set_input_to_read_only(self):
+        super().set_input_to_read_only()
         self.input.read_only = True
 
     set_input_to_read_only.__doc__ = GenericMaster.set_input_to_read_only.__doc__
diff --git a/pyiron_base/jobs/script.py b/pyiron_base/jobs/script.py
index 2b6777b57..7df709cf1 100644
--- a/pyiron_base/jobs/script.py
+++ b/pyiron_base/jobs/script.py
@@ -271,6 +271,7 @@ def set_input_to_read_only(self):
         This function enforces read-only mode for the input classes, but it has to be implement in the individual
         classes.
         """
+        super().set_input_to_read_only()
         self.input.read_only = True
 
     def to_hdf(self, hdf=None, group_name=None):

From eb637adadf0212e6754814e030d7d40376b24b12 Mon Sep 17 00:00:00 2001
From: pyiron-runner <pyiron@mpie.de>
Date: Thu, 5 Oct 2023 12:35:56 +0000
Subject: [PATCH 09/15] Format black

---
 pyiron_base/interfaces/lockable.py              | 17 ++++++++++++++---
 .../jobs/job/extension/server/generic.py        |  4 +++-
 pyiron_base/storage/datacontainer.py            |  2 --
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/pyiron_base/interfaces/lockable.py b/pyiron_base/interfaces/lockable.py
index f8e8ea8dc..ceb1d85f9 100644
--- a/pyiron_base/interfaces/lockable.py
+++ b/pyiron_base/interfaces/lockable.py
@@ -21,9 +21,11 @@
 
 from pyiron_base.interfaces.has_groups import HasGroups
 
+
 class Locked(Exception):
     pass
 
+
 def sentinel(meth):
     """
     Wrap a method to fail if `read_only` is `True` on the owning object.
@@ -36,6 +38,7 @@ def sentinel(meth):
     Returns:
         wrapped method
     """
+
     def dispatch_or_error(self, *args, **kwargs):
         try:
             method = object.__getattribute__(self, "_lock_method")
@@ -44,16 +47,19 @@ def dispatch_or_error(self, *args, **kwargs):
         if method not in ("error", "warning"):
             method = "error"
         if self.read_only and method == "error":
-            raise Locked("Object is currently locked!  Use unlocked() if you know what you are doing.")
+            raise Locked(
+                "Object is currently locked!  Use unlocked() if you know what you are doing."
+            )
         elif self.read_only and method == "warning":
             warnings.warn(
-                    f"{meth.__name__} called on {type(self)}, but object is locked!"
+                f"{meth.__name__} called on {type(self)}, but object is locked!"
             )
         return meth(self, *args, **kwargs)
 
     # if sentinel is applied to __setattr__ we must ensure that `read_only`
     # stays available, otherwise we can't unlock again later
     if meth.__name__ == "__setattr__":
+
         @wraps(meth)
         def f(self, *args, **kwargs):
             if len(args) > 0:
@@ -63,16 +69,19 @@ def f(self, *args, **kwargs):
             if target in ("read_only", "_read_only"):
                 return meth(self, *args, **kwargs)
             return dispatch_or_error(self, *args, **kwargs)
+
     else:
         f = wraps(meth)(dispatch_or_error)
     return f
 
+
 class _UnlockContext:
     """
     Context manager that unlocks and relocks a :class:`Lockable`.
 
     This is an implementation detail of :class:`Lockable`.
     """
+
     __slots__ = ("owner",)
 
     def __init__(self, owner):
@@ -84,7 +93,8 @@ def __enter__(self):
 
     def __exit__(self, *_):
         self.owner.lock()
-        return False # never handle exceptions
+        return False  # never handle exceptions
+
 
 def _iterate_lockable_subs(lockable_groups):
     """
@@ -106,6 +116,7 @@ def _iterate_lockable_subs(lockable_groups):
         yield group
         yield from _iterate_lockable_subs(g)
 
+
 class Lockable:
     """
     A small mixin to lock attribute and method access at runtime.
diff --git a/pyiron_base/jobs/job/extension/server/generic.py b/pyiron_base/jobs/job/extension/server/generic.py
index bdf5a4219..66f94e105 100644
--- a/pyiron_base/jobs/job/extension/server/generic.py
+++ b/pyiron_base/jobs/job/extension/server/generic.py
@@ -28,7 +28,9 @@
 __date__ = "Sep 1, 2017"
 
 
-class Server(Lockable):  # add the option to return the job id and the hold id to the server object
+class Server(
+    Lockable
+):  # add the option to return the job id and the hold id to the server object
     """
     Generic Server object to handle the execution environment for the job
 
diff --git a/pyiron_base/storage/datacontainer.py b/pyiron_base/storage/datacontainer.py
index 8823eb5ce..1f6e2bc6a 100644
--- a/pyiron_base/storage/datacontainer.py
+++ b/pyiron_base/storage/datacontainer.py
@@ -320,7 +320,6 @@ def __getitem__(self, key):
 
     @sentinel
     def __setitem__(self, key, val):
-
         key = _normalize(key)
 
         if isinstance(key, tuple):
@@ -349,7 +348,6 @@ def __setitem__(self, key, val):
 
     @sentinel
     def __delitem__(self, key):
-
         key = _normalize(key)
 
         if isinstance(key, tuple):

From 1aa8f6ae83317ec7911661f1f80da4d92ef244f3 Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Thu, 5 Oct 2023 15:48:05 +0200
Subject: [PATCH 10/15] Add LockedWarning and update docstring

---
 pyiron_base/interfaces/lockable.py | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/pyiron_base/interfaces/lockable.py b/pyiron_base/interfaces/lockable.py
index ceb1d85f9..e59687bd1 100644
--- a/pyiron_base/interfaces/lockable.py
+++ b/pyiron_base/interfaces/lockable.py
@@ -25,6 +25,8 @@
 class Locked(Exception):
     pass
 
+class LockedWarning(UserWarning):
+    pass
 
 def sentinel(meth):
     """
@@ -52,7 +54,8 @@ def dispatch_or_error(self, *args, **kwargs):
             )
         elif self.read_only and method == "warning":
             warnings.warn(
-                f"{meth.__name__} called on {type(self)}, but object is locked!"
+                f"{meth.__name__} called on {type(self)}, but object is locked!",
+                category=LockedWarning
             )
         return meth(self, *args, **kwargs)
 
@@ -137,7 +140,7 @@ class Lockable:
     using methods and properties decorated with :func:`.sentinel`.
 
     Subclasses may override :meth:`_on_lock` and :meth:`_on_unlock` if they wish to customize locking/unlocking
-    behaviour, provided that they call super() in their overloads.
+    behaviour, provided that they call `super()` in their overloads.
 
     Let's start with a simple example; a list that can be locked
 
@@ -316,7 +319,19 @@ def _on_unlock(self):
     def lock(self, method: Optional[Literal["error", "warning"]] = None):
         """
         Set :attr:`~.read_only`.
+
+        Objects may be safely locked multiple times without further effect.
+
+        Args:
+            method (str, either "error" or "warning"): if "error" raise an :class:`.Locked` exception if modification is
+                    attempted; if "warning" raise a :class:`.LockedWarning` warning; default is "error" or the value
+                    passed to the constructor.
+
+        Raises:
+            ValueError: if `method` is not an allowed value
         """
+        if method not in ["error", "warning"]:
+            raise ValueError(f"Unrecognized lock method {method}!")
         if method is not None:
             object.__setattr__(self, "_lock_method", method)
         self.read_only = True

From f9a0fd2a9869403d20d1f073940aafd0871e6be3 Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Thu, 5 Oct 2023 15:48:48 +0200
Subject: [PATCH 11/15] Allow to pass lock method to data container

---
 pyiron_base/storage/datacontainer.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pyiron_base/storage/datacontainer.py b/pyiron_base/storage/datacontainer.py
index 1f6e2bc6a..8430a73f1 100644
--- a/pyiron_base/storage/datacontainer.py
+++ b/pyiron_base/storage/datacontainer.py
@@ -257,7 +257,7 @@ def __new__(cls, *args, **kwargs):
 
         return instance
 
-    def __init__(self, init=None, table_name=None, lazy=False, wrap_blacklist=()):
+    def __init__(self, init=None, table_name=None, lazy=False, wrap_blacklist=(), lock_method="warning"):
         """
         Create new container.
 
@@ -269,7 +269,7 @@ def __init__(self, init=None, table_name=None, lazy=False, wrap_blacklist=()):
             wrap_blacklist (tuple of types): any values in `init` that are instances of the given types are *not*
                                              wrapped in :class:`.DataContainer`
         """
-        super().__init__(lock_method="warning")
+        super().__init__(lock_method=lock_method)
         self.table_name = table_name
         self._lazy = lazy
         if init is not None:

From 7d0522b91b2ae745e316c91aaad7366c3ef0fcd3 Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Thu, 5 Oct 2023 16:47:25 +0200
Subject: [PATCH 12/15] Allow None as lock method

---
 pyiron_base/interfaces/lockable.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pyiron_base/interfaces/lockable.py b/pyiron_base/interfaces/lockable.py
index e59687bd1..b8e7adcf5 100644
--- a/pyiron_base/interfaces/lockable.py
+++ b/pyiron_base/interfaces/lockable.py
@@ -330,7 +330,7 @@ def lock(self, method: Optional[Literal["error", "warning"]] = None):
         Raises:
             ValueError: if `method` is not an allowed value
         """
-        if method not in ["error", "warning"]:
+        if method not in ["error", "warning", None]:
             raise ValueError(f"Unrecognized lock method {method}!")
         if method is not None:
             object.__setattr__(self, "_lock_method", method)

From c46f1bd924cabb8c80ec74359b1eafa69cda81e5 Mon Sep 17 00:00:00 2001
From: pyiron-runner <pyiron@mpie.de>
Date: Mon, 9 Oct 2023 13:53:44 +0000
Subject: [PATCH 13/15] Format black

---
 pyiron_base/interfaces/lockable.py   | 4 +++-
 pyiron_base/storage/datacontainer.py | 9 ++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/pyiron_base/interfaces/lockable.py b/pyiron_base/interfaces/lockable.py
index b8e7adcf5..84a8e2878 100644
--- a/pyiron_base/interfaces/lockable.py
+++ b/pyiron_base/interfaces/lockable.py
@@ -25,9 +25,11 @@
 class Locked(Exception):
     pass
 
+
 class LockedWarning(UserWarning):
     pass
 
+
 def sentinel(meth):
     """
     Wrap a method to fail if `read_only` is `True` on the owning object.
@@ -55,7 +57,7 @@ def dispatch_or_error(self, *args, **kwargs):
         elif self.read_only and method == "warning":
             warnings.warn(
                 f"{meth.__name__} called on {type(self)}, but object is locked!",
-                category=LockedWarning
+                category=LockedWarning,
             )
         return meth(self, *args, **kwargs)
 
diff --git a/pyiron_base/storage/datacontainer.py b/pyiron_base/storage/datacontainer.py
index 8430a73f1..89f359310 100644
--- a/pyiron_base/storage/datacontainer.py
+++ b/pyiron_base/storage/datacontainer.py
@@ -257,7 +257,14 @@ def __new__(cls, *args, **kwargs):
 
         return instance
 
-    def __init__(self, init=None, table_name=None, lazy=False, wrap_blacklist=(), lock_method="warning"):
+    def __init__(
+        self,
+        init=None,
+        table_name=None,
+        lazy=False,
+        wrap_blacklist=(),
+        lock_method="warning",
+    ):
         """
         Create new container.
 

From 28a6ea3d4b945936b0ac8df90ee7519f27b5c567 Mon Sep 17 00:00:00 2001
From: Marvin Poul <poul@mpie.de>
Date: Fri, 15 Dec 2023 09:21:31 +0100
Subject: [PATCH 14/15] Docstring fixes

---
 pyiron_base/interfaces/lockable.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/pyiron_base/interfaces/lockable.py b/pyiron_base/interfaces/lockable.py
index 84a8e2878..133c33c6a 100644
--- a/pyiron_base/interfaces/lockable.py
+++ b/pyiron_base/interfaces/lockable.py
@@ -238,8 +238,8 @@ class Lockable:
 
     but we can unlock it as usual
 
-    >>> with d.unlocked() as dopen:
-    ...   dopen['b']['d'] = 23
+    >>> with d.unlocked():
+    ...   d['b']['d'] = 23
     >>> d['b']['d']
     23
 
@@ -342,6 +342,10 @@ def unlocked(self) -> _UnlockContext:
         """
         Unlock the object temporarily.
 
-        Context manager returns this object again.
+        Context manager returns this object again and relocks it after the `with` statement finished.
+        
+        .. note:: `lock()` vs. `unlocked()`
+        
+            There is a small asymmetry between these two methods.  :meth:`.lock` can only be done once (meaningfully), while :meth:`.unlocked` is a context manager and can be called multiple times.
         """
         return _UnlockContext(self)

From b4b48840ccb1134e239a4db2491365ad8cc03630 Mon Sep 17 00:00:00 2001
From: pyiron-runner <pyiron@mpie.de>
Date: Fri, 15 Dec 2023 15:52:52 +0000
Subject: [PATCH 15/15] Format black

---
 pyiron_base/interfaces/lockable.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pyiron_base/interfaces/lockable.py b/pyiron_base/interfaces/lockable.py
index 133c33c6a..4116b63ef 100644
--- a/pyiron_base/interfaces/lockable.py
+++ b/pyiron_base/interfaces/lockable.py
@@ -343,9 +343,9 @@ def unlocked(self) -> _UnlockContext:
         Unlock the object temporarily.
 
         Context manager returns this object again and relocks it after the `with` statement finished.
-        
+
         .. note:: `lock()` vs. `unlocked()`
-        
+
             There is a small asymmetry between these two methods.  :meth:`.lock` can only be done once (meaningfully), while :meth:`.unlocked` is a context manager and can be called multiple times.
         """
         return _UnlockContext(self)