From ba74e48066ab8a786586b3b3fe6eab2bf8c1fd01 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Fri, 5 Apr 2024 13:38:57 -0700 Subject: [PATCH 1/2] Don't give `Function` special treatment for `self` --- pyiron_workflow/function.py | 36 ++++-------------------- tests/unit/test_function.py | 56 ------------------------------------- 2 files changed, 6 insertions(+), 86 deletions(-) diff --git a/pyiron_workflow/function.py b/pyiron_workflow/function.py index 2a1ca4ac..fc1e1f9f 100644 --- a/pyiron_workflow/function.py +++ b/pyiron_workflow/function.py @@ -428,19 +428,8 @@ def preview_input_channels(cls) -> dict[str, tuple[Any, Any]]: """ type_hints = cls._type_hints() scraped: dict[str, tuple[Any, Any]] = {} - for ii, (label, value) in enumerate(cls._input_args().items()): - is_self = False - if label == "self": # `self` is reserved for the node object - if ii == 0: - is_self = True - else: - warnings.warn( - "`self` is used as an argument but not in the first" - " position, so it is treated as a normal function" - " argument. If it is to be treated as the node object," - " use it as a first argument" - ) - elif label in cls._init_keywords(): + for label, param in cls._input_args().items(): + if label in cls._init_keywords(): # We allow users to parse arbitrary kwargs as channel initialization # So don't let them choose bad channel names raise ValueError( @@ -450,20 +439,14 @@ def preview_input_channels(cls) -> dict[str, tuple[Any, Any]]: try: type_hint = type_hints[label] - if is_self: - warnings.warn("type hint for self ignored") except KeyError: type_hint = None - default = NOT_DATA # The standard default in DataChannel - if value.default is not inspect.Parameter.empty: - if is_self: - warnings.warn("default value for self ignored") - else: - default = value.default + default = ( + NOT_DATA if param.default is inspect.Parameter.empty else param.default + ) - if not is_self: - scraped[label] = (type_hint, default) + scraped[label] = (type_hint, default) return scraped @classmethod @@ -498,13 +481,6 @@ def on_run(self): @property def run_args(self) -> dict: kwargs = self.inputs.to_value_dict() - if "self" in self._input_args(): - if self.executor: - raise ValueError( - f"Function node {self.label} uses the `self` argument, but this " - f"can't yet be run with executors" - ) - kwargs["self"] = self return kwargs def process_run_result(self, function_output: Any | tuple) -> Any | tuple: diff --git a/tests/unit/test_function.py b/tests/unit/test_function.py index 998001ea..d3a31906 100644 --- a/tests/unit/test_function.py +++ b/tests/unit/test_function.py @@ -242,62 +242,6 @@ def test_statuses(self): self.assertFalse(n.running) self.assertTrue(n.failed) - def test_with_self(self): - def with_self(self, x: float) -> float: - # Note: Adding internal state to the node like this goes against the best - # practice of keeping nodes "functional". Following python's paradigm of - # giving users lots of power, we want to guarantee that this behaviour is - # _possible_. - if "some_counter" in self._user_data: - self._user_data["some_counter"] += 1 - else: - self._user_data["some_counter"] = 1 - return x + 0.1 - - node = function_node(with_self, output_labels="output") - self.assertTrue( - "x" in node.inputs.labels, - msg=f"Expected to find function input 'x' in the node input but got " - f"{node.inputs.labels}" - ) - self.assertFalse( - "self" in node.inputs.labels, - msg="Expected 'self' to be filtered out of node input, but found it in the " - "input labels" - ) - node.inputs.x = 1.0 - node.run() - self.assertEqual( - node.outputs.output.value, - 1.1, - msg="Basic node functionality appears to have failed" - ) - self.assertEqual( - node._user_data["some_counter"], - 1, - msg="node_function should be able to modify attributes on the node " - "object." - ) - - node.executor = Executor() - with self.assertRaises( - ValueError, - msg="We haven't implemented any way to update a function node's `self` when" - "it runs on an executor, so trying to do so should fail hard" - ): - node.run() - node.executor_shutdown() # Shouldn't get this far, but if we do shutdown - node.executor = None - - def with_messed_self(x: float, self) -> float: - return x + 0.1 - - with warnings.catch_warnings(record=True) as warning_list: - node = function_node(with_messed_self) - self.assertTrue("self" in node.inputs.labels) - - self.assertEqual(len(warning_list), 1) - def test_call(self): node = function_node(no_default, output_labels="output") From 61eac44116687614a5093da3ca1fcdc48ce65052 Mon Sep 17 00:00:00 2001 From: liamhuber Date: Fri, 5 Apr 2024 13:56:08 -0700 Subject: [PATCH 2/2] Add a test for protected names --- tests/unit/test_function.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit/test_function.py b/tests/unit/test_function.py index d3a31906..ebaa8383 100644 --- a/tests/unit/test_function.py +++ b/tests/unit/test_function.py @@ -242,6 +242,19 @@ def test_statuses(self): self.assertFalse(n.running) self.assertTrue(n.failed) + def test_protected_name(self): + @as_function_node() + def Selfish(self, x): + return x + + n = Selfish() + with self.assertRaises( + ValueError, + msg="When we try to build inputs, we should run into the fact that inputs " + "can't overlap with __init__ signature terms" + ): + n.inputs + def test_call(self): node = function_node(no_default, output_labels="output")