diff --git a/docs/README.md b/docs/README.md index 1baa9b10..40e13367 100644 --- a/docs/README.md +++ b/docs/README.md @@ -51,7 +51,7 @@ But the intent is to collect them together into a workflow and leverage existing ```python >>> from pyiron_workflow import Workflow ->>> Workflow.register("plotting", "pyiron_workflow.node_library.plotting") +>>> Workflow.register("pyiron_workflow.node_library.plotting", "plotting") >>> >>> @Workflow.wrap_as.single_value_node() ... def Arange(n: int): diff --git a/notebooks/atomistics_nodes.ipynb b/notebooks/atomistics_nodes.ipynb index 44731001..4b34fb26 100644 --- a/notebooks/atomistics_nodes.ipynb +++ b/notebooks/atomistics_nodes.ipynb @@ -27,8 +27,8 @@ "metadata": {}, "outputs": [], "source": [ - "Workflow.register(\"atomistics\", \"pyiron_workflow.node_library.atomistics\")\n", - "Workflow.register(\"plotting\", \"pyiron_workflow.node_library.plotting\")" + "Workflow.register(\"pyiron_workflow.node_library.atomistics\", \"atomistics\")\n", + "Workflow.register(\"pyiron_workflow.node_library.plotting\", \"plotting\")" ] }, { diff --git a/notebooks/deepdive.ipynb b/notebooks/deepdive.ipynb index d1d086ec..b913fd02 100644 --- a/notebooks/deepdive.ipynb +++ b/notebooks/deepdive.ipynb @@ -1664,8 +1664,8 @@ } ], "source": [ - "wf.register(\"pyiron_atomistics\", \"pyiron_workflow.node_library.pyiron_atomistics\")\n", - "wf.register(\"plotting\", \"pyiron_workflow.node_library.plotting\")\n", + "wf.register(\"pyiron_workflow.node_library.pyiron_atomistics\", \"pyiron_atomistics\")\n", + "wf.register(\"pyiron_workflow.node_library.plotting\", \"plotting\")\n", "\n", "wf = Workflow(\"with_prebuilt\")\n", "\n", diff --git a/notebooks/quickstart.ipynb b/notebooks/quickstart.ipynb index 76ee81cd..2944dcb7 100644 --- a/notebooks/quickstart.ipynb +++ b/notebooks/quickstart.ipynb @@ -833,7 +833,7 @@ " \"\"\"\n", " return np.arange(n), n\n", "\n", - "wf.register(\"plotting\", \"pyiron_workflow.node_library.plotting\")\n", + "wf.register(\"pyiron_workflow.node_library.plotting\", \"plotting\")\n", "\n", "wf.arange = Arange(10)\n", "wf.plot = wf.create.plotting.Scatter(\n", diff --git a/pyiron_workflow/composite.py b/pyiron_workflow/composite.py index 512f4d88..663f47c8 100644 --- a/pyiron_workflow/composite.py +++ b/pyiron_workflow/composite.py @@ -507,8 +507,8 @@ def _rebuild_data_io(self): @classmethod @wraps(Creator.register) - def register(cls, domain: str, package_identifier: str) -> None: - cls.create.register(domain=domain, package_identifier=package_identifier) + def register(cls, package_identifier: str, domain: Optional[str] = None) -> None: + cls.create.register(package_identifier=package_identifier, domain=domain) def executor_shutdown(self, wait=True, *, cancel_futures=False): """ diff --git a/pyiron_workflow/interfaces.py b/pyiron_workflow/interfaces.py index 55e92f44..90e19338 100644 --- a/pyiron_workflow/interfaces.py +++ b/pyiron_workflow/interfaces.py @@ -8,7 +8,7 @@ import pkgutil from sys import version_info from types import ModuleType -from typing import TYPE_CHECKING +from typing import Optional, TYPE_CHECKING from bidict import bidict from pyiron_workflow.snippets.singleton import Singleton @@ -78,7 +78,7 @@ def __init__(self): # in python >=3.10 # If the CI skips testing on 3.9 gets dropped, we can think about removing # this if-clause and just letting users of python <3.10 hit an error. - self.register("standard", "pyiron_workflow.node_library.standard") + self.register("pyiron_workflow.node_library.standard", "standard") @property def PyFluxExecutor(self): @@ -138,31 +138,53 @@ def __getattr__(self, item): f"forget to register node package to this key?" ) from e + def __getitem__(self, item): + try: + return self._package_registry[item] + except KeyError as e: + raise KeyError( + f"Could not find the package {item} -- are you sure it's registered?" + ) from e + def __getstate__(self): return self.__dict__ def __setstate__(self, state): self.__dict__ = state - def register(self, domain: str, package_identifier: str) -> None: + def register(self, package_identifier: str, domain: Optional[str] = None) -> None: """ + Add a new package of nodes from the provided identifier. + The new package is available by item-access using the identifier, and, if a + domain was provided, by attribute access under that domain path ("."-split + strings allow for deep-registration of domains). Add a new package of nodes under the provided attribute, e.g. after adding nodes to the domain `"my_nodes"`, and instance of creator can call things like `creator.my_nodes.some_node_that_is_there()`. + Currently, :param:`package_identifier` is just a python module string, and we + allow recursive registration of multiple node packages when a module is + provided whose sub-modules are node packages. If a :param:`domain` was + provided, then it is extended by the same semantic path as the modules, e.g. + if `my_python_module` is registered to the domain `"mpm"`, and + `my_python_module.submod1` and `my_python_module.submod2.subsub` are both node + packages, then `mpm.submod1` and `mpm.submod2.subsub` will both be available + for attribute access. + Note: If a macro is going to use a creator, the node registration should be _inside_ the macro definition to make sure the node actually has access to those nodes! It also needs to be _able_ to register those nodes, i.e. have import access to that location, but we don't for that check that. Args: - domain (str): The attribute name at which to register the new package. - (Note: no sanitizing is done here except for splitting on "." to create - sub-domains, so if you provide a string that won't work as an attribute - name, that's your problem.) package_identifier (str): An identifier for the node package. (Right now that's just a string version of the path to the module, e.g. `pyiron_workflow.node_library.standard`.) + domain (str|None): The attribute name at which to register the new package. + (Note: no sanitizing is done here except for splitting on "." to create + sub-domains, so if you provide a string that won't work as an attribute + name, that's your problem.) (Default is None, don't provide attribute + access to this package.) Raises: KeyError: If the domain already exists, but the identifier doesn't match @@ -179,11 +201,14 @@ def register(self, domain: str, package_identifier: str) -> None: try: module = import_module(package_identifier) - if "." in domain: - domain, container = self._get_deep_container(domain) + if domain is not None: + if "." in domain: + domain, container = self._get_deep_container(domain) + else: + container = self._package_access + self._register_recursively_from_module(module, domain, container) else: - container = self._package_access - self._register_recursively_from_module(module, domain, container) + self._register_recursively_from_module(module, None, None) except ModuleNotFoundError as e: raise ModuleNotFoundError( f"In the current implementation, we expect package identifiers to be " @@ -212,17 +237,20 @@ def _get_deep_container(self, semantic_domain: str) -> tuple[str, DotDict]: return domain, container def _register_recursively_from_module( - self, module: ModuleType, domain: str, container: DotDict + self, module: ModuleType, domain: str | None, container: DotDict | None ) -> None: if hasattr(module, "__path__"): - if domain not in container.keys(): - container[domain] = DotDict() - subcontainer = container[domain] + if domain is not None: + if domain not in container.keys(): + container[domain] = DotDict() + subcontainer = container[domain] + else: + subcontainer = None for _, submodule_name, _ in pkgutil.walk_packages( module.__path__, module.__name__ + "." ): submodule = import_module(submodule_name) - subdomain = submodule_name.split(".")[-1] + subdomain = None if domain is None else submodule_name.split(".")[-1] if not hasattr(submodule, "__path__"): if hasattr(submodule, "nodes"): @@ -239,18 +267,19 @@ def _register_recursively_from_module( self._register_package_from_module(module, domain, container) def _register_package_from_module( - self, module: ModuleType, domain: str, container: dict | DotDict + self, module: ModuleType, domain: str | None, container: dict | DotDict | None ) -> None: package = self._get_existing_package_or_register_a_new_one(module.__name__) # NOTE: Here we treat the package identifier and the module name as equivalent - if domain not in container.keys(): - # If the container _doesn't_ yet have anything at this domain, just add it - container[domain] = package - else: - self._raise_error_unless_new_package_matches_existing( - container, domain, package - ) + if domain is not None: + if domain not in container.keys(): + # If the container _doesn't_ yet have anything at this domain, just add it + container[domain] = package + else: + self._raise_error_unless_new_package_matches_existing( + container, domain, package + ) def _get_existing_package_or_register_a_new_one( self, package_identifier: str diff --git a/pyiron_workflow/node.py b/pyiron_workflow/node.py index ae6ad1c6..a4751550 100644 --- a/pyiron_workflow/node.py +++ b/pyiron_workflow/node.py @@ -19,7 +19,7 @@ NotData, ) from pyiron_workflow.draw import Node as GraphvizNode -from pyiron_workflow.files import DirectoryObject +from pyiron_workflow.snippets.files import DirectoryObject from pyiron_workflow.has_to_dict import HasToDict from pyiron_workflow.io import Signals from pyiron_workflow.topology import ( diff --git a/pyiron_workflow/files.py b/pyiron_workflow/snippets/files.py similarity index 91% rename from pyiron_workflow/files.py rename to pyiron_workflow/snippets/files.py index 05c6af03..8914913b 100644 --- a/pyiron_workflow/files.py +++ b/pyiron_workflow/snippets/files.py @@ -43,8 +43,9 @@ def __init__(self, directory): def create(self): self.path.mkdir(parents=True, exist_ok=True) - def delete(self): - delete_files_and_directories_recursively(self.path) + def delete(self, only_if_empty: bool = False): + if self.is_empty() or not only_if_empty: + delete_files_and_directories_recursively(self.path) def list_content(self): return categorize_folder_items(self.path) @@ -71,6 +72,9 @@ def create_subdirectory(self, path): def create_file(self, file_name): return FileObject(file_name, self) + def is_empty(self) -> bool: + return len(self) == 0 + class FileObject: def __init__(self, file_name: str, directory: DirectoryObject): diff --git a/pyiron_workflow/workflow.py b/pyiron_workflow/workflow.py index f05f1194..8e89229e 100644 --- a/pyiron_workflow/workflow.py +++ b/pyiron_workflow/workflow.py @@ -164,7 +164,7 @@ class Workflow(Composite): namespaces. These need to be registered first, like the standard package is automatically registered: - >>> Workflow.register("standard", "pyiron_workflow.node_library.standard") + >>> Workflow.register("pyiron_workflow.node_library.standard", "standard") When your workflow's data follows a directed-acyclic pattern, it will determine the execution flow automatically. diff --git a/tests/integration/test_workflow.py b/tests/integration/test_workflow.py index 128ca00d..41c3328a 100644 --- a/tests/integration/test_workflow.py +++ b/tests/integration/test_workflow.py @@ -83,7 +83,7 @@ def sqrt(value=0): ) def test_for_loop(self): - Workflow.register("demo", "static.demo_nodes") + Workflow.register("static.demo_nodes", "demo") n = 5 @@ -188,7 +188,7 @@ def test_executor_and_creator_interaction(self): C.f. `pyiron_workflow.function._wrapper_factory` for more detail. """ wf = Workflow("depickle") - wf.register("demo", "static.demo_nodes") + wf.register("static.demo_nodes", "demo") wf.before_pickling = wf.create.demo.OptionallyAdd(1) wf.before_pickling.executor = wf.create.Executor() diff --git a/tests/unit/test_files.py b/tests/unit/snippets/test_files.py similarity index 60% rename from tests/unit/test_files.py rename to tests/unit/snippets/test_files.py index 0093fea9..f67fdc90 100644 --- a/tests/unit/test_files.py +++ b/tests/unit/snippets/test_files.py @@ -1,5 +1,5 @@ import unittest -from pyiron_workflow.files import DirectoryObject, FileObject +from pyiron_workflow.snippets.files import DirectoryObject, FileObject from pathlib import Path @@ -45,6 +45,29 @@ def test_is_file(self): f.delete() self.assertFalse(f.is_file()) + def test_is_empty(self): + self.assertTrue(self.directory.is_empty()) + self.directory.write(file_name="test.txt", content="something") + self.assertFalse(self.directory.is_empty()) + + def test_delete(self): + self.assertTrue( + Path("test").exists() and Path("test").is_dir(), + msg="Sanity check on initial state" + ) + self.directory.write(file_name="test.txt", content="something") + self.directory.delete(only_if_empty=True) + self.assertFalse( + self.directory.is_empty(), + msg="Flag argument on delete should have prevented removal" + ) + self.directory.delete() + self.assertFalse( + Path("test").exists(), + msg="Delete should remove the entire directory" + ) + self.directory = DirectoryObject("test") # Rebuild it so the tearDown works + if __name__ == '__main__': unittest.main() diff --git a/tests/unit/test_composite.py b/tests/unit/test_composite.py index 376d3ca8..d5764f6d 100644 --- a/tests/unit/test_composite.py +++ b/tests/unit/test_composite.py @@ -66,7 +66,7 @@ def bar(x: int = 0) -> int: ) def test_creator_access_and_registration(self): - self.comp.register("demo", "static.demo_nodes") + self.comp.register("static.demo_nodes", "demo") # Test invocation self.comp.add_node(self.comp.create.demo.OptionallyAdd(label="by_add")) diff --git a/tests/unit/test_interfaces.py b/tests/unit/test_interfaces.py index e11f0c20..87442fa1 100644 --- a/tests/unit/test_interfaces.py +++ b/tests/unit/test_interfaces.py @@ -21,24 +21,35 @@ def test_registration(self): ): self.creator.demo_nodes - self.creator.register("demo", "static.demo_nodes") + self.creator.register("static.demo_nodes") - node = self.creator.demo.OptionallyAdd(1, 2) - self.assertEqual( - 3, - node(), - msg="Node should get instantiated from creator and be operable" - ) + with self.subTest("Test access by item"): + node = self.creator["static.demo_nodes"].OptionallyAdd(1, 2) + self.assertEqual( + 3, + node(), + msg="Node should get instantiated from creator and be operable" + ) + + self.creator.register("static.demo_nodes", "demo") + + with self.subTest("Test access by attribute"): + node = self.creator.demo.OptionallyAdd(1, 2) + self.assertEqual( + 3, + node(), + msg="Node should get instantiated from creator and be operable" + ) - self.creator.register("sub", "static.nodes_subpackage") + self.creator.register("static.nodes_subpackage", "sub") self.assertIsInstance(self.creator.sub.demo_nodes, NodePackage) self.assertIsInstance(self.creator.sub.subsub_package.demo_nodes, NodePackage) with self.subTest("Test re-registration"): - self.creator.register("demo", "static.demo_nodes") + self.creator.register("static.demo_nodes", "demo") # Same thing to the same location should be fine - self.creator.register("a_key_other_than_demo", "static.demo_nodes") + self.creator.register("static.demo_nodes", "a_key_other_than_demo") # The same thing to another key is usually dumb, but totally permissible self.assertIs( self.creator.demo, @@ -49,54 +60,55 @@ def test_registration(self): with self.assertRaises( ValueError, - msg="Should not be able to register a new package to an existing domain" + msg="Should not be able to register different package to an existing " + "domain" ): - self.creator.register("demo", "pyiron_workflow.node_library.standard") + self.creator.register("pyiron_workflow.node_library.standard", "demo") with self.assertRaises( AttributeError, msg="Should not be able to register to existing fields" ): some_field = self.creator.dir()[0] - self.creator.register(some_field, "static.demo_nodes") + self.creator.register("static.demo_nodes", some_field) with self.subTest("Test semantic domain"): - self.creator.register("some.path", "static.demo_nodes") + self.creator.register("static.demo_nodes", "some.path") self.assertIsInstance(self.creator.some.path, NodePackage) - self.creator.register("some.deeper.path", "static.demo_nodes") + self.creator.register("static.demo_nodes", "some.deeper.path") self.assertIsInstance(self.creator.some.deeper.path, NodePackage) with self.assertRaises( ValueError, msg="Can't inject a branch on a package" ): - self.creator.register("some.path.deeper", "static.demo_nodes") + self.creator.register("static.demo_nodes", "some.path.deeper") with self.subTest("Test failure cases"): - n_initial_packages = len(self.creator._package_access) + n_initial_packages = len(self.creator._package_registry) with self.assertRaises( NotANodePackage, msg="Mustn't allow importing from things that are not node packages" ): - self.creator.register("not_even", "static.not_a_node_package") + self.creator.register("static.not_a_node_package") with self.assertRaises( NotANodePackage, msg="Must require a `nodes` property in the module" ): - self.creator.register("forgetful", "static.forgetful_node_package") + self.creator.register("static.forgetful_node_package") with self.assertRaises( NotANodePackage, msg="Must have only node classes in the iterable `nodes` property" ): - self.creator.register("faulty", "static.faulty_node_package") + self.creator.register("static.faulty_node_package") self.assertEqual( n_initial_packages, - len(self.creator._package_access), + len(self.creator._package_registry), msg="Packages should not be getting added if exceptions are raised" ) diff --git a/tests/unit/test_node.py b/tests/unit/test_node.py index 3c239d01..439eaf28 100644 --- a/tests/unit/test_node.py +++ b/tests/unit/test_node.py @@ -4,7 +4,7 @@ import unittest from pyiron_workflow.channels import InputData, OutputData, NotData -from pyiron_workflow.files import DirectoryObject +from pyiron_workflow.snippets.files import DirectoryObject from pyiron_workflow.interfaces import Executor from pyiron_workflow.io import Inputs, Outputs from pyiron_workflow.node import Node