-
Notifications
You must be signed in to change notification settings - Fork 2
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
Export to dict #586
base: main
Are you sure you want to change the base?
Export to dict #586
Changes from 22 commits
e7e0c16
284138a
ba30633
e08708a
34a6fe3
1323218
1763d18
256002c
92c3aec
6fc0acb
7b9dd6e
ff0505b
9133a6c
73f9946
e6b31e4
4616970
0a9f67f
e9f7ad7
e693908
f85980c
fd60fc2
359ea9b
6879e91
b280e64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -13,6 +13,7 @@ | |||||
from pyiron_snippets.colors import SeabornColors | ||||||
from pyiron_snippets.dotdict import DotDict | ||||||
|
||||||
from pyiron_workflow.channels import NOT_DATA, Channel | ||||||
from pyiron_workflow.create import HasCreator | ||||||
from pyiron_workflow.mixin.semantics import SemanticParent | ||||||
from pyiron_workflow.node import Node | ||||||
|
@@ -26,6 +27,130 @@ | |||||
from pyiron_workflow.storage import StorageInterface | ||||||
|
||||||
|
||||||
def _extract_data(item: Channel, with_values=True, with_default=True) -> dict: | ||||||
data = {} | ||||||
data_dict = {"default": NOT_DATA, "value": NOT_DATA, "type_hint": None} | ||||||
if not with_values: | ||||||
data_dict.pop("value") | ||||||
if not with_default: | ||||||
data_dict.pop("default") | ||||||
for key, value in data_dict.items(): | ||||||
if getattr(item, key) is not value: | ||||||
data[key] = getattr(item, key) | ||||||
return data | ||||||
|
||||||
|
||||||
def _is_internal_connection(channel: Channel, workflow: Composite, io_: str) -> bool: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. *sigh*. What I would like is all channels are only ever connected to other channels in the same scope. Unfortunately, nodes with no parent at all at least complicate this. Anyhow, this function isn't bad, it's just that it ought to be sufficient to merely use |
||||||
""" | ||||||
Check if a channel is connected to another channel in the same workflow. | ||||||
|
||||||
Args: | ||||||
channel (Channel): The channel to check. | ||||||
workflow (Composite): The workflow to check whether the channel is connected to. | ||||||
io_ (str): The IO direction to check. | ||||||
|
||||||
Returns: | ||||||
bool: Whether the channel is connected to another channel in the same workflow. | ||||||
""" | ||||||
if not channel.connected: | ||||||
return False | ||||||
return any(channel.connections[0] in getattr(n, io_) for n in workflow) | ||||||
|
||||||
|
||||||
def _get_scoped_label(channel: Channel, io_: str) -> str: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Function name lies! |
||||||
return channel.scoped_label.replace("__", f".{io_}.") | ||||||
|
||||||
|
||||||
def _io_to_dict( | ||||||
node: Node, with_values: bool = True, with_default: bool = True | ||||||
) -> dict: | ||||||
data = {"inputs": {}, "outputs": {}} | ||||||
is_composite = isinstance(node, Composite) | ||||||
for io_ in ["inputs", "outputs"]: | ||||||
for inp in getattr(node, io_): | ||||||
if is_composite: | ||||||
data[io_][inp.scoped_label] = _extract_data( | ||||||
inp, with_values=with_values, with_default=with_default | ||||||
) | ||||||
else: | ||||||
data[io_][inp.label] = _extract_data( | ||||||
inp, with_values=with_values, with_default=with_default | ||||||
) | ||||||
return data | ||||||
|
||||||
|
||||||
def export_node_to_dict( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will not work. You hint It is also not a good feeling to have I also think that the composite's probably should also get a "node function", but it's something like Honestly, I think you will have a much easier time with this whole PR if you (or you wait for me eventually to) clean up the space you're working in -- i.e. if #504 is closed, we'll have a much cleaner division of "composite" vs "atomic" nodes right in the class hierarchy. #360 might help too, but it's harder and I think you can get away without it here. |
||||||
node: Node, with_values: bool = True, with_default: bool = True | ||||||
) -> dict: | ||||||
""" | ||||||
Export a node to a dictionary. | ||||||
|
||||||
Args: | ||||||
node (Node): The node to export. | ||||||
with_values (bool): Whether to include the values of the channels in the | ||||||
dictionary. (Default is True.) | ||||||
|
||||||
Returns: | ||||||
dict: The exported node as a dictionary. | ||||||
""" | ||||||
data = {"inputs": {}, "outputs": {}, "function": node.node_function} | ||||||
data.update(_io_to_dict(node, with_values=with_values, with_default=with_default)) | ||||||
return data | ||||||
|
||||||
|
||||||
def export_composite_to_dict( | ||||||
workflow: Composite, with_values: bool = True, with_default: bool = True | ||||||
) -> dict: | ||||||
""" | ||||||
Export a composite to a dictionary. | ||||||
|
||||||
Args: | ||||||
workflow (Composite): The composite to export. | ||||||
with_values (bool): Whether to include the values of the channels in the | ||||||
dictionary. (Default is True.) | ||||||
|
||||||
Returns: | ||||||
dict: The exported composite as a dictionary. | ||||||
""" | ||||||
data = {"inputs": {}, "outputs": {}, "nodes": {}, "edges": []} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All composite's are |
||||||
for inp in workflow.inputs: | ||||||
if inp.value_receiver is not None: | ||||||
data["edges"].append( | ||||||
( | ||||||
f"inputs.{inp.scoped_label}", | ||||||
_get_scoped_label(inp.value_receiver, "inputs"), | ||||||
) | ||||||
) | ||||||
for node in workflow: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is 100% correct, but it reminds me that I meant to open another issue... #588 We could proceed with such a definition now, but we'd need to remember to update it as part of that issue. |
||||||
label = node.label | ||||||
if isinstance(node, Composite): | ||||||
data["nodes"][label] = export_composite_to_dict( | ||||||
node, with_values=with_values | ||||||
) | ||||||
else: | ||||||
data["nodes"][label] = export_node_to_dict(node, with_values=with_values) | ||||||
for inp in node.inputs: | ||||||
if _is_internal_connection(inp, workflow, "outputs"): | ||||||
data["edges"].append( | ||||||
( | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still advocate for |
||||||
_get_scoped_label(inp.connections[0], "outputs"), | ||||||
_get_scoped_label(inp, "inputs"), | ||||||
) | ||||||
) | ||||||
for out in node.outputs: | ||||||
if out.value_receiver is not None: | ||||||
data["edges"].append( | ||||||
( | ||||||
_get_scoped_label(out, "outputs"), | ||||||
f"outputs.{out.value_receiver.scoped_label}", | ||||||
) | ||||||
) | ||||||
Comment on lines
+146
to
+153
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is super similar to the same scraping for the input value receiver pairs, just a sort of conjugate. I think it should be possible to nicely pull out a |
||||||
data.update( | ||||||
_io_to_dict(workflow, with_values=with_values, with_default=with_default) | ||||||
) | ||||||
return data | ||||||
|
||||||
|
||||||
def _get_graph_as_dict(composite: Composite) -> dict: | ||||||
if not isinstance(composite, Composite): | ||||||
return composite | ||||||
|
@@ -459,6 +584,13 @@ def graph_as_dict(self) -> dict: | |||||
""" | ||||||
return _get_graph_as_dict(self) | ||||||
|
||||||
def export_to_dict( | ||||||
self, with_values: bool = True, with_default: bool = True | ||||||
) -> dict: | ||||||
return export_composite_to_dict( | ||||||
self, with_values=with_values, with_default=with_default | ||||||
) | ||||||
|
||||||
def _get_connections_as_strings( | ||||||
self, panel_getter: Callable | ||||||
) -> list[tuple[tuple[str, str], tuple[str, str]]]: | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import unittest | ||
|
||
from pyiron_workflow._tests import ensure_tests_in_python_path | ||
from pyiron_workflow.nodes.composite import Composite | ||
from pyiron_workflow.workflow import Workflow | ||
|
||
ensure_tests_in_python_path() | ||
|
||
|
||
@Workflow.wrap.as_function_node | ||
def add_one(a: int): | ||
result = a + 1 | ||
return result | ||
|
||
@Workflow.wrap.as_function_node | ||
def add_two(b: int = 10) -> int: | ||
result = b + 2 | ||
return result | ||
|
||
@Workflow.wrap.as_macro_node | ||
def add_three(macro: Composite | None = None, c: int = 0) -> int: | ||
macro.one = add_one(a=c) | ||
macro.two = add_two(b=macro.one) | ||
w = macro.two | ||
return w | ||
|
||
|
||
class TestExport(unittest.TestCase): | ||
def test_io_independence(self): | ||
wf = Workflow("my_wf") | ||
wf.three = add_three(c=1) | ||
wf.four = add_one(a=wf.three) | ||
wf.run() | ||
data = wf.export_to_dict() | ||
self.assertEqual( | ||
set(data.keys()), {"edges", "inputs", "nodes", "outputs"} | ||
) | ||
self.assertEqual( | ||
data["inputs"], {'three__c': {"default": 0, 'value': 1, 'type_hint': int}} | ||
) | ||
|
||
|
||
if __name__ == "__main__": | ||
unittest.main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type hint is wrong as
Channel
doesn't havevalue
ordefault
, and IMO this method should probably just live directly on the relevant class -- move it over toDataChannel
?