Enforce Operation Requirements (#496)

* Enforce condition/operation dichotomy more strictly

* Enforce operation duplication rules more stringently

* Test ``FlowProject.operation`` function/name requirements

* Add changelog entry

* Remove line break in changelog

* Update error message.

Co-authored-by: Carl Simon Adorf <[email protected]>

* Update changelog.

Co-authored-by: Carl Simon Adorf <[email protected]>

Co-authored-by: Bradley Dice <[email protected]>
Co-authored-by: Carl Simon Adorf <[email protected]>

changelog.txt

@@ -26,6 +26,7 @@ Changed
 Fixed
 +++++
 
+- Strictly enforce that operation functions cannot be used as condition functions (and vice-versa) and prevent the registration of two operations with the same name (#496).
 - Changed default value of ``status_parallelization`` to none, to avoid bugs in user code caused by thread parallelism and overhead in process parallelism (#486).
 - Memory directives are converted to an integer number of gigabytes or megabytes in submission scripts (#482, #484).
 

flow/project.py

@@ -24,7 +24,7 @@
 from copy import deepcopy
 from enum import IntFlag
 from hashlib import md5, sha1
-from itertools import count, groupby, islice
+from itertools import chain, count, groupby, islice
 from multiprocessing import Event, Pool, TimeoutError, cpu_count
 from multiprocessing.pool import ThreadPool
 
@@ -4032,11 +4032,24 @@ def hello(job):
         if isinstance(func, str):
             return lambda op: cls.operation(op, name=func)
 
+        if func in chain(
+            *cls._OPERATION_PRECONDITIONS.values(),
+            *cls._OPERATION_POSTCONDITIONS.values(),
+        ):
+            raise ValueError("A condition function cannot be used as an operation.")
+
         if name is None:
             name = func.__name__
 
-        if (name, func) in cls._OPERATION_FUNCTIONS:
-            raise ValueError(f"An operation with name '{name}' is already registered.")
+        for registered_name, registered_func in cls._OPERATION_FUNCTIONS:
+            if name == registered_name:
+                raise ValueError(
+                    f"An operation with name '{name}' is already registered."
+                )
+            if func is registered_func:
+                raise ValueError(
+                    "An operation with this function is already registered."
+                )
         if name in cls._GROUP_NAMES:
             raise ValueError(f"A group with name '{name}' is already registered.")
 

tests/test_project.py

@@ -265,12 +265,55 @@ class A(FlowProject):
         with pytest.raises(ValueError):
 
             @A.operation
-            @A.operation
+            @A.operation("foo")
             def op1(job):
                 pass
 
         return
 
+    def test_repeat_operation_name(self):
+        class A(FlowProject):
+            pass
+
+        @A.operation
+        def op1(job):
+            pass
+
+        with pytest.raises(ValueError):
+
+            @A.operation("op1")
+            def op2(job):
+                pass
+
+    def test_condition_as_operation(self):
+        class A(FlowProject):
+            pass
+
+        def precondition(job):
+            pass
+
+        @A.pre(precondition)
+        @A.operation
+        def op1(job):
+            pass
+
+        with pytest.raises(ValueError):
+            precondition = A.operation(precondition)
+
+    def test_operation_as_condition(self):
+        class A(FlowProject):
+            pass
+
+        @A.operation
+        def attempted_precondition(job):
+            pass
+
+        with pytest.raises(ValueError):
+
+            @A.pre(attempted_precondition)
+            def op1(job):
+                pass
+
     def test_repeat_operation_definition_with_inheritance(self):
         class A(FlowProject):
             pass