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

Fix node registration #53

Merged
merged 40 commits into from
Oct 31, 2023
Merged
Changes from 1 commit
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
9436f82
Raise from the original error
liamhuber Oct 26, 2023
9630641
Add the type hint to channel presentation
liamhuber Oct 26, 2023
359ad26
Store the type hints right on the decorator-created classes
liamhuber Oct 26, 2023
5dffb5b
Store a reference to the import path
liamhuber Oct 26, 2023
f551f59
Pre-register the normal packages
liamhuber Oct 26, 2023
7ff73d7
Handle the absence of return hints more gracefully
liamhuber Oct 26, 2023
2960d6f
Refactor the function node decorators
liamhuber Oct 26, 2023
b607c87
Format black
pyiron-runner Oct 26, 2023
8040316
Add a unit test for registration
liamhuber Oct 27, 2023
cb43cc8
Refactor tests
liamhuber Oct 27, 2023
b1dda61
Allow re-registering the _same thing_ to the same place
liamhuber Oct 27, 2023
7b9837d
Expand tests
liamhuber Oct 27, 2023
4dbcf22
Merge remote-tracking branch 'origin/fix_node_registration' into fix_…
liamhuber Oct 27, 2023
7851e7b
Refactor: rename
liamhuber Oct 27, 2023
63f30c0
Add docstring
liamhuber Oct 27, 2023
0328c22
Fail early if the provided package identifier will fail
liamhuber Oct 27, 2023
f43958e
Refactor: extract the "just re-registering" logic
liamhuber Oct 27, 2023
3136905
Format black
pyiron-runner Oct 27, 2023
20fa4ae
Version guard package registration
liamhuber Oct 30, 2023
08d3dc0
Explain intent for future devs
liamhuber Oct 30, 2023
8059b57
Be more generous waiting for the timeout
liamhuber Oct 30, 2023
d058cd0
Give a shortcut to node registration
liamhuber Oct 30, 2023
39dec3f
Add an integration test for the thing that originally hurt us
liamhuber Oct 30, 2023
4289210
Format black
pyiron-runner Oct 30, 2023
bdcb100
Make standard node package really standard
liamhuber Oct 30, 2023
b00aedc
Don't register atomistics by default
liamhuber Oct 30, 2023
9e45028
Purge atomistics module from unit tests
liamhuber Oct 30, 2023
0ffd8c9
Give the demo nodes complex typing
liamhuber Oct 30, 2023
3c2433b
Make it easier to ensure you can import the static demo nodes
liamhuber Oct 30, 2023
33474ed
Use a node name that doesn't conflict with Composite methods
liamhuber Oct 30, 2023
44e3a07
Use demo instead of atomistics nodes
liamhuber Oct 30, 2023
19110a9
Make the registration shortcut a class method
liamhuber Oct 30, 2023
664b06b
Use the demo nodes instead of atomistics
liamhuber Oct 30, 2023
6e4de97
Merge remote-tracking branch 'origin/fix_node_registration' into fix_…
liamhuber Oct 30, 2023
7e20641
:bug: fix node name typo
liamhuber Oct 30, 2023
6189e02
:bug: when you fix it use the right damned name
liamhuber Oct 30, 2023
8722e33
PEP8 whitespace
liamhuber Oct 30, 2023
00f8690
Update and rerun example notebook
liamhuber Oct 30, 2023
d8695ae
Format black
pyiron-runner Oct 30, 2023
23b8827
Add a note on registration and macros
liamhuber Oct 30, 2023
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
28 changes: 28 additions & 0 deletions pyiron_workflow/interfaces.py
Original file line number Diff line number Diff line change
@@ -101,6 +101,7 @@ def register(self, domain: str, package_identifier: str) -> None:
with the stored identifier.
AttributeError: If you try to register at a domain that is already another
method or attribute of the creator.
ValueError: If the identifier can't be parsed.
"""
if domain in self._node_packages.keys():
if package_identifier != self._node_packages[domain]:
@@ -113,8 +114,35 @@ def register(self, domain: str, package_identifier: str) -> None:
elif domain in self.__dir__():
raise AttributeError(f"{domain} is already an attribute of {self}")

self._verify_identifier(package_identifier)

self._node_packages[domain] = package_identifier

@staticmethod
def _verify_identifier(package_identifier: str):
"""
Logic for verifying whether new package identifiers will actually be usable for
creating node packages when their domain is called. Lets us fail early in
registration.

Right now, we just make sure it's a string from which we can import a list of
nodes.
"""
from pyiron_workflow.node import Node
try:
module = import_module(package_identifier)
nodes = module.nodes
if not all(issubclass(node, Node) for node in nodes):
raise TypeError(
f"At least one node in {nodes} was not of the type {Node.__name__}"
)
except Exception as e:
raise ValueError(
f"The package identifier is {package_identifier} is not valid. Please "
f"ensure it is an importable module with a list of {Node.__name__} "
f"objects stored in the variable `nodes`."
) from e


class Wrappers(metaclass=Singleton):
"""
13 changes: 13 additions & 0 deletions tests/static/faulty_node_package.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"""
An incorrect node package for the purpose of testing node package registration.
"""

from pyiron_workflow import Workflow


@Workflow.wrap_as.single_value_node("sum")
def add(x: int, y: int) -> int:
return x + y


nodes = [add, 42] # Not everything here is a node!
13 changes: 13 additions & 0 deletions tests/static/forgetful_node_package.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"""
An incorrect node package for the purpose of testing node package registration.
"""

from pyiron_workflow import Workflow


@Workflow.wrap_as.single_value_node("sum")
def add(x: int, y: int) -> int:
return x + y


# nodes = [add] # Oops, we "forgot" to populate a `nodes` list
6 changes: 6 additions & 0 deletions tests/static/not_a_node_package.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
"""
A module that is not a node package at all for the purpose of testing node package
registration.
"""

this_variable = "is not a list of Node objects called `nodes`"
27 changes: 27 additions & 0 deletions tests/unit/test_interfaces.py
Original file line number Diff line number Diff line change
@@ -54,3 +54,30 @@ def test_registration(self):
):
some_field = self.creator.dir()[0]
self.creator.register(some_field, "static.demo_nodes")

with self.subTest("Test failure cases"):
n_initial_packages = len(self.creator._node_packages)

with self.assertRaises(
ValueError,
msg="Mustn't allow importing from things that are not node packages"
):
self.creator.register("not_even", "static.not_a_node_package")

with self.assertRaises(
ValueError,
msg="Must require a `nodes` property in the module"
):
self.creator.register("forgetful", "static.forgetful_node_package")

with self.assertRaises(
ValueError,
msg="Must have only nodes in the iterable `nodes` property"
):
self.creator.register("faulty", "static.faulty_node_package")

self.assertEqual(
n_initial_packages,
len(self.creator._node_packages),
msg="Packages should not be getting added if exceptions are raised"
)