Skip to content

Commit

Permalink
Merge pull request #156 from pyiron/registration_by_identifier_alone
Browse files Browse the repository at this point in the history
Registration by identifier alone
  • Loading branch information
liamhuber authored Jan 15, 2024
2 parents d5fcb48 + 05b6356 commit 5bb9eaf
Show file tree
Hide file tree
Showing 14 changed files with 130 additions and 62 deletions.
2 changes: 1 addition & 1 deletion docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions notebooks/atomistics_nodes.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -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\")"
]
},
{
Expand Down
4 changes: 2 additions & 2 deletions notebooks/deepdive.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion notebooks/quickstart.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions pyiron_workflow/composite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down
77 changes: 53 additions & 24 deletions pyiron_workflow/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand All @@ -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 "
Expand Down Expand Up @@ -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"):
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pyiron_workflow/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion pyiron_workflow/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down
25 changes: 24 additions & 1 deletion tests/unit/test_files.py → tests/unit/snippets/test_files.py
Original file line number Diff line number Diff line change
@@ -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


Expand Down Expand Up @@ -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()
2 changes: 1 addition & 1 deletion tests/unit/test_composite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
Loading

0 comments on commit 5bb9eaf

Please sign in to comment.