[minor] Function selflessly

[liamhuber]

Currently, we jump through some hoops to let users pass self as a special argument for Function nodes. This didn't show up in the node input, and under the hood the node instance got passed in for that argument. You could do things like this:

from pyiron_workflow import Workflow

@Workflow.wrap_as.function_node()
def Selfish(self, x):
    self.some_attribute = 5
    return x

n = Selfish()
print(n.inputs.labels)
>>> ["x"]

n(x=42)
print(n.some_attribute)
>>> 5

Now we don't, and self is treated just like any other Function node argument, which means it bumps right into the protection we have in place to stop people from using terms already in the Function.__init__ signature (this protection allows input to be set as **kwargs)

from pyiron_workflow import Workflow

@Workflow.wrap.as_function_node()
def Selfish(self, x):
    self.some_attribute = 5
    return x

n = Selfish()
print(n.inputs.labels)
>>> ValueError: The Input channel name self is not valid. Please choose a name _not_ among ['self', 'args', 'label', 'parent', 'overwrite_save', 'run_after_init', 'storage_backend', 'save_after_run', 'kwargs']

I like for a bunch of reasons:

• Function nodes should behave functionally, so let's not explicitly give the user tools to break that
• The self argument didn't play well with executors, so there were special exceptions to fail early and non-universal behaviour
• Accommodating this took extra special-case code and increased maintenance cost
• It seems very in line with @JNmpi's sentiment at the last meeting about how nodes should not be having extra properties

I.e. if you want state, use a state-ful Macro node and hold the state as IO of one of its children.

Non-goals:

• Failing early, like when the node class is defined, instead of waiting for the input to be accessed. I want this, and it should be easy, but it pertains to all protected signature items and is out of scope here.

EDIT: python syntax highlighting on the examples

[github-actions]

👈 Launch a binder notebook on branch pyiron/pyiron_workflow/function_selflessly

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ +0.03% (target: -1.00%)	✅ 100.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (0f33bec)	3447	3003	87.12%
Head commit (61eac44)	3431 (-16)	2990 (-13)	87.15% (+0.03%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#279)	4	4	100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

_{You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation}

[samwaseda]

Sorry I had a heated discussion yesterday elsewhere and could not join the pyiron meeting. I was never very comfortable with self (I guess same for you) so I'm ok with getting rid of it. But this only removes self right? I thought the intention of self was to pass information from outside, like:

from pyiron_workflow import Workflow

@Workflow.wrap.as_function_node()
def Selfish(self, some_executable):
    some_executable.cores = self.server.cores
    some_executable.run()
    return some_executable.output

Or do we now want to do everything via function arguments?

[liamhuber]

Ah, I had only been thinking in terms of adding state to the node rather than pulling data from existing state. At any rate, I agree we can probably manage to do it via the function input. E.g.

from pyiron_workflow import Workflow

@Workflow.wrap.as_function_node()
def Selfless(some_executable, cores):
    some_executable.cores = cores
    some_executable.run()
    return some_executable.output

n = Selfless()
n(some_executable=exe, cores=n.server.cores)

This is not identical functionality, but let's run with it and add back in self only in the dire case that it's unavoidably necessary for a particular application.

EDIT: python highlighting