REF: Further refactoring following code review

Author: effigies

pydra/engine/specs.py

@@ -161,53 +161,48 @@ def check_fields_input_spec(self):
             field_is_mandatory = bool(field.metadata.get("mandatory"))
             field_is_unset = getattr(self, field.name) is attr.NOTHING
 
+            if field_is_unset and not field_is_mandatory:
+                continue
+
             # Collect alternative fields associated with this field.
             alternative_fields = {
                 name: getattr(self, name) is not attr.NOTHING
                 for name in field.metadata.get("xor", [])
                 if name != field.name
             }
+            alternatives_are_set = any(alternative_fields.values())
 
-            # Collect required fields associated with this field.
-            required_fields = {
-                name: getattr(self, name) is not attr.NOTHING
-                for name in field.metadata.get("requires", [])
-                if name != field.name
-            }
-
-            # Raise error if field is mandatory and unset
-            # or no suitable alternative is provided.
+            # Raise error if no field in mandatory alternative group is set.
             if field_is_unset:
-                if field_is_mandatory:
-                    if alternative_fields:
-                        if any(alternative_fields.values()):
-                            # Alternative fields found, skip other checks.
-                            continue
-                        else:
-                            raise AttributeError(
-                                f"{field.name} is mandatory and unset, "
-                                "but no value provided by "
-                                f"{list(alternative_fields.keys())}."
-                            )
-                    else:
-                        raise AttributeError(
-                            f"{field.name} is mandatory, but no value provided."
-                        )
-                else:
-                    # Field is not set, check the next one.
+                if alternatives_are_set:
                     continue
+                message = f"{field.name} is mandatory and unset."
+                if alternative_fields:
+                    raise AttributeError(
+                        message[:-1]
+                        + f", but no alternative provided by {list(alternative_fields)}."
+                    )
+                else:
+                    raise AttributeError(message)
 
             # Raise error if multiple alternatives are set.
-            if alternative_fields and any(alternative_fields.values()):
+            elif alternatives_are_set:
                 set_alternative_fields = [
                     name for name, is_set in alternative_fields.items() if is_set
                 ]
                 raise AttributeError(
                     f"{field.name} is mutually exclusive with {set_alternative_fields}"
                 )
 
+            # Collect required fields associated with this field.
+            required_fields = {
+                name: getattr(self, name) is not attr.NOTHING
+                for name in field.metadata.get("requires", [])
+                if name != field.name
+            }
+
             # Raise error if any required field is unset.
-            if required_fields and not all(required_fields.values()):
+            if not all(required_fields.values()):
                 unset_required_fields = [
                     name for name, is_set in required_fields.items() if not is_set
                 ]

pydra/engine/tests/test_shelltask_inputspec.py

@@ -2089,7 +2089,7 @@ def test_shell_cmd_inputs_di(tmpdir, use_validator):
     )
     assert (
         shelly.cmdline == f"DenoiseImage -i {tmpdir.join('a_file.ext')} -s 1 -p 1 -r 2 "
-        f"-o [{str(shelly.output_dir / 'a_file_out.ext')}, {str(shelly.output_dir / 'a_file_noise.ext')}]"
+                          f"-o [{str(shelly.output_dir / 'a_file_out.ext')}, {str(shelly.output_dir / 'a_file_noise.ext')}]"
     )
 
     # input file name and help_short
@@ -2205,7 +2205,7 @@ def test_task_inputs_mandatory_with_xOR_zero_mandatory_raises_error():
     with pytest.raises(Exception) as excinfo:
         task.inputs.check_fields_input_spec()
     assert "input_1 is mandatory" in str(excinfo.value)
-    assert "no value provided by ['input_2', 'input_3']" in str(excinfo.value)
+    assert "no alternative provided by ['input_2', 'input_3']" in str(excinfo.value)
     assert excinfo.type is AttributeError