Skip to content

Commit

Permalink
Reparent AmbiguousOutputError onto AttributeError (#595)
Browse files Browse the repository at this point in the history
* Reparent AmbiguousOutputError onto AttributeError

Just conceptually errors being raised from `__getattr__` ought to be this way anyhow, not `ValueError`. But more importantly, raising a `ValueError` instead of an `AttributeError` meant that the builtin `hasattr` method couldn't properly return `False` when the lookup failed due to not having an unambiguous `.channel` attribute to which to delegate the lookup.

Signed-off-by: liamhuber <[email protected]>

* Lint

Signed-off-by: liamhuber <[email protected]>

---------

Signed-off-by: liamhuber <[email protected]>
  • Loading branch information
liamhuber authored Feb 17, 2025
1 parent d55bb2a commit bd13cc3
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 6 deletions.
14 changes: 9 additions & 5 deletions pyiron_workflow/mixin/single_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
)


class AmbiguousOutputError(ValueError):
class AmbiguousOutputError(AttributeError):
"""Raised when searching for exactly one output, but multiple are found."""


Expand Down Expand Up @@ -59,11 +59,15 @@ def channel(self) -> OutputDataWithInjection:
def __getattr__(self, item):
try:
return super().__getattr__(item)
except AttributeError as e1:
try:
except AttributeError as e:
if len(self.outputs) == 1:
return getattr(self.channel, item)
except Exception as e2:
raise e2 from e1
else:
raise AmbiguousOutputError(
f"Tried to access {item} on {self.label}, but failed. Delegating "
f"access to `.channel` was impossible because there is more than "
f"one output channel"
) from e

def __getitem__(self, item):
return self.channel.__getitem__(item)
Expand Down
1 change: 0 additions & 1 deletion tests/integration/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ def test_executors(self):
# executorlib < 0.1 had an Executor with optional backend parameter (defaulting to SingleNodeExecutor)
executors.append(Workflow.create.executorlib.Executor)


wf = Workflow("executed")
wf.a = Workflow.create.standard.UserInput(42) # Regular
wf.b = wf.a + 1 # Injected
Expand Down
43 changes: 43 additions & 0 deletions tests/unit/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,22 @@ def to_dict(self):
pass


class TwOutputs(ANode):
"""To de-abstract the class"""

def _setup_node(self) -> None:
super()._setup_node()
self._outputs = OutputsWithInjection(
OutputDataWithInjection("y", self, type_hint=int),
OutputDataWithInjection("z", self, type_hint=int),
)

def process_run_result(self, run_output):
self.outputs.y.value = run_output
self.outputs.z.value = run_output + 1
return self.outputs.y.value, self.outputs.z.value


class TestNode(unittest.TestCase):
def setUp(self):
self.n1 = ANode(label="start", x=0)
Expand Down Expand Up @@ -394,6 +410,33 @@ def test_single_value(self):
):
node.channel # noqa: B018

def test_injection_hasattr(self):
x = 2
node = ANode(label="n")
node.set_input_values(x=x)
node.run()

self.assertEqual(
node.value,
add_one(x),
msg="With a single output, we expect to access the channel attribute",
)

two_outputs = TwOutputs(label="to")
two_outputs.set_input_values(x=x)
two_outputs.run()

with self.assertRaises(
AmbiguousOutputError,
msg="With two output channels, we should not be able to isolate a well defined value attribute because there is no single `channel` to look on",
):
getattr(two_outputs, "value") # noqa: B009

self.assertFalse(
hasattr(two_outputs, "value"),
msg="As a corollary to the last assertion, hasattr should fail",
)

def test_storage(self):
self.assertIs(
self.n1.outputs.y.value, NOT_DATA, msg="Sanity check on initial state"
Expand Down

0 comments on commit bd13cc3

Please sign in to comment.