Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[minor] Function selflessly #279

Merged
merged 2 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 6 additions & 30 deletions pyiron_workflow/function.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down
59 changes: 8 additions & 51 deletions tests/unit/test_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,61 +242,18 @@ 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."
)
def test_protected_name(self):
@as_function_node()
def Selfish(self, x):
return x

node.executor = Executor()
n = Selfish()
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"
msg="When we try to build inputs, we should run into the fact that inputs "
"can't overlap with __init__ signature terms"
):
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)
n.inputs

def test_call(self):
node = function_node(no_default, output_labels="output")
Expand Down
Loading