diff --git a/pyproject.toml b/pyproject.toml index c9ab299..04d40b6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ name = "snakemake-interface-common" version = "1.22.0" description = "Common functions and classes for Snakemake and its plugins" readme = "README.md" -requires-python = ">=3.8" +requires-python = ">=3.11" dependencies = ["argparse-dataclass>=2.0.0", "ConfigArgParse>=1.7", "packaging >=24.0,<26.0"] [[project.authors]] @@ -63,18 +63,21 @@ ignore_missing_imports = true module = "snakemake" follow_untyped_imports = true +[tool.pytest.ini_options] +python_files = ["test_*.py", "tests.py"] + [tool.pixi.feature.dev.tasks.test] cmd = [ "pytest", "--cov=snakemake_interface_common", "--cov-report=xml:coverage-report/coverage.xml", "--cov-report=term-missing", - "tests/tests.py" + "tests/", ] description = "Run tests and generate coverage report" [tool.pixi.feature.dev.tasks] -format = "ruff format src" +format = "ruff format src tests" lint = "ruff check" type-check = "mypy src/" qc = { depends-on = ["format", "lint", "type-check"] } diff --git a/src/snakemake_interface_common/plugin_registry/__init__.py b/src/snakemake_interface_common/plugin_registry/__init__.py index a991b8e..68b1c55 100644 --- a/src/snakemake_interface_common/plugin_registry/__init__.py +++ b/src/snakemake_interface_common/plugin_registry/__init__.py @@ -8,7 +8,18 @@ import types import pkgutil import importlib -from typing import Dict, List, Mapping, TYPE_CHECKING, Type, TypeVar, Generic +from typing import ( + Dict, + List, + Mapping, + TYPE_CHECKING, + Type, + TypeVar, + Generic, + Optional, + Self, +) +from importlib.metadata import entry_points from snakemake_interface_common.exceptions import InvalidPluginException from snakemake_interface_common.plugin_registry.plugin import PluginBase @@ -21,14 +32,44 @@ class PluginRegistryBase(ABC, Generic[TPlugin]): - """This class is a singleton that holds all registered executor plugins.""" + """Base class to discover and record all available plugins of a given type. - _instance = None + This class is a singleton, all calls to the constructor will return the same instance. + ``__init__()`` should not take any arguments. + + Derived class names are expected to end with ``PluginRegistry``, where the prefix is the type of + plugin (e.g. ``ExecutorPluginRegistry``). This is returned by :meth:`get_plugin_type`. + + Package discovery happens in two ways, by package name and by entry point. + + Named-based discovery works by searching through all importable top-level modules in + ``sys.path`` and selecting those where the full name matches + ``"{self.module_prefix}_{plugin_name}"``, where :attr:`module_prefix` should be + ``"snakemake_{plugin_type}_plugin_"``. The plugin will be registered under the name + ``plugin_name``, but with underscores replaced with dashes. Note that this will not detect + packages installed in editable mode ``(pip install -e .)``. + + Example: a package named ``snakemake_executor_plugin_my_executor`` will be discovered by the + ``executor`` registry (with ``module_prefix = "snakemake_executor_plugin_"``) and registered + under the name ``"my-executor"``. The corresponding Pip/distribution package should be named + ``snakemake-executor-plugin-my-executor``, although this is not enforced. + + Entry point discovery uses importlib's `entry point system`_. If the :attr:`entry_point` + property is overridden to return a string, plugins can be registered under the + ``snakemake.{entry_point}`` group in their ``pyproject.toml``. For example, if the entry point + group is ``executors``, the following registers the module ``my_executor.submodule`` under the + name ``my-executor``: + + [project.entry-points.'snakemake.executors'] + my-executor = "my_executor.submodule" + + .. _entry point system: https://packaging.python.org/en/latest/specifications/entry-points/ + """ + + _instance: Self | None = None plugins: Dict[str, TPlugin] - def __new__( - cls: Type["PluginRegistryBase[TPlugin]"], - ) -> "PluginRegistryBase[TPlugin]": + def __new__(cls) -> Self: if cls._instance is None: cls._instance = super().__new__(cls) return cls._instance @@ -39,6 +80,32 @@ def __init__(self) -> None: return self.collect_plugins() + ######## Abstract/overridable methods ######## + + @property + @abstractmethod + def module_prefix(self) -> str: + """Prefix used to identify plugins by importable module name.""" + + @property + def entry_point(self) -> Optional[str]: + """Group name used to register plugins through the entry point system. + + The full group name is ``"snakemake.{name}"``. If None the entry point system + will not be used. + """ + return None + + @abstractmethod + def load_plugin(self, name: str, module: types.ModuleType) -> TPlugin: + """Instantiate the plugin object given its name and imported module.""" + + @abstractmethod + def expected_attributes(self) -> Mapping[str, AttributeType]: + """Get expected attributes of imported plugin module.""" + + ######## Other methods ######## + def get_registered_plugins(self) -> List[str]: """Return a list of registered plugin names.""" return [name for name in self.plugins.keys()] @@ -47,29 +114,41 @@ def is_installed(self, plugin_name: str) -> bool: """Return True if the plugin is registered.""" return plugin_name in self.plugins - def get_plugin(self, plugin_name: str) -> PluginBase: - """Get a plugin by name.""" + def get_plugin(self, plugin_name: str) -> TPlugin: + """Get a registered plugin by name. + + Raises + ------ + InvalidPluginException + If the plugin is not registered. + """ try: return self.plugins[plugin_name] except KeyError: + pkgname = self.get_plugin_package_name(plugin_name) raise InvalidPluginException( plugin_name, - f"The package {self.module_prefix.replace('_', '-')}{plugin_name} is " - "not installed.", + f"The package {pkgname} is not installed.", ) def get_plugin_package_name(self, plugin_name: str) -> str: - """Get the package name of a plugin by name.""" + """Get the package name of a plugin by name. + + This is the pip-installable package name, not the name used to import the plugin module. + """ return f"{self.module_prefix.replace('_', '-')}{plugin_name}" def register_cli_args(self, argparser: "ArgumentParser") -> None: - """Add arguments derived from self.executor_settings to given - argparser.""" + """Add arguments derived from all registered plugins to given argparser.""" plugin_type = self.get_plugin_type() for _, plugin in self.plugins.items(): plugin.register_cli_args(argparser, plugin_type) def get_plugin_type(self) -> str: + """Get a string describing the type of plugin tracked by the registry. + + This is derived from the class name. + """ m = re.match(r"(?P.+)PluginRegistry", self.__class__.__name__) if m is not None: return m.group("type").lower() @@ -80,43 +159,91 @@ def get_plugin_type(self) -> str: ) def collect_plugins(self) -> None: - """Collect plugins and call register_plugin for each.""" + """Collect plugins, import their modules, and call :meth:`register_plugin` for each.""" self.plugins = dict() + self._collect_plugins_by_name() + self._collect_plugins_by_entry_point() - # Executor plugins are externally installed plugins named - # "snakemake_executor_". - # They should follow the same convention if on pip, - # snakemake-executor-. - # Note that these will not be detected installed in editable - # mode (pip install -e .). + def _collect_plugins_by_name(self) -> None: for moduleinfo in pkgutil.iter_modules(): if not moduleinfo.ispkg or not moduleinfo.name.startswith( self.module_prefix ): continue + + name = moduleinfo.name.removeprefix(self.module_prefix).replace("_", "-") module = importlib.import_module(moduleinfo.name) - self.register_plugin(moduleinfo.name, module) + self.register_plugin(name, module) - def register_plugin(self, name: str, plugin: types.ModuleType) -> None: + def _collect_plugins_by_entry_point(self) -> None: + if self.entry_point is None: + return + group = "snakemake." + self.entry_point + + for ep in entry_points(group=group): + # Should be a module + if ":" in ep.value: + raise InvalidPluginException( + ep.name, + f"invalid entry point {ep.value!r} (should not contain a colon)", + ) + + # Unlike registration by package name, there can be duplicates + if ep.name in self.plugins: + continue + + try: + module = ep.load() + except ImportError as exc: + raise InvalidPluginException( + ep.name, f"unable to import {ep.value}" + ) from exc + + self.register_plugin(ep.name, module) + + def register_plugin(self, name: str, module: types.ModuleType) -> None: """Validate and register a plugin. Does nothing if the plugin is already registered. + + Parameters + ---------- + name + The name to register the plugin under. If derived from the module name, it should have + :attr:`module_prefix` removed. + module + The plugin's imported module. + + Raises + ------ + InvalidPluginException + If validation fails. """ if name in self.plugins: return - self.validate_plugin(name, plugin) - - # Derive the shortened name for future access - plugin_name = name.removeprefix(self.module_prefix).replace("_", "-") - - self.plugins[plugin_name] = self.load_plugin(plugin_name, plugin) + self.validate_plugin(name, module) + self.plugins[name] = self.load_plugin(name, module) def is_valid_plugin_package_name(self, name: str) -> bool: return True def validate_plugin(self, name: str, module: types.ModuleType) -> None: - """Validate a plugin for attributes and naming""" + """Validate a plugin module for attributes and naming. + + Parameters + ---------- + name + The name the plugin is to be registered under. + module + The plugin's imported module. + + Raises + ------ + InvalidPluginException + If any module attributes have an incorrect type or a required attribute is missing. + """ + expected_attributes = self.expected_attributes() for attr, attr_type in expected_attributes.items(): # check if attr is missing and fail if it is not optional @@ -140,15 +267,3 @@ def validate_plugin(self, name: str, module: types.ModuleType) -> None: raise InvalidPluginException( name, f"{attr} must be of type {attr_type.cls.__name__}." ) - - @property - @abstractmethod - def module_prefix(self) -> str: ... - - @abstractmethod - def load_plugin(self, name: str, module: types.ModuleType) -> TPlugin: - """Load a plugin by name.""" - ... - - @abstractmethod - def expected_attributes(self) -> Mapping[str, AttributeType]: ... diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..28ee3fb --- /dev/null +++ b/tests/__init__.py @@ -0,0 +1 @@ +# Make test dir a package to allow for relative imports diff --git a/tests/example_plugin.py b/tests/example_plugin.py new file mode 100644 index 0000000..f67d909 --- /dev/null +++ b/tests/example_plugin.py @@ -0,0 +1,76 @@ +"""Define example plugin type for testing.""" + +from dataclasses import dataclass +from types import ModuleType +from pathlib import Path +from typing import Self + +from snakemake_interface_common.plugin_registry import PluginRegistryBase +from snakemake_interface_common.plugin_registry.plugin import PluginBase, SettingsBase +from snakemake_interface_common.plugin_registry.attribute_types import ( + AttributeType, + AttributeMode, + AttributeKind, +) + + +@dataclass +class ExamplePlugin(PluginBase): + _name: str + _settings_cls: type[SettingsBase] | None + file: Path + string_attr: str + + @property + def name(self) -> str: + return self._name + + @property + def cli_prefix(self) -> str: + return "example-plugin-" + self.name + + @property + def settings_cls(self) -> type[SettingsBase] | None: + return self._settings_cls + + +class ExamplePluginRegistry(PluginRegistryBase[ExamplePlugin]): + @classmethod + def new(cls) -> Self: + """Create a new non-singleton instance for testing.""" + instance = object.__new__(cls) + instance.__init__() + return instance + + @property + def module_prefix(self) -> str: + return "snakemake_example_plugin_" + + @property + def entry_point(self) -> str: + return "example_plugins" + + def load_plugin(self, name: str, module: ModuleType) -> ExamplePlugin: + settings_cls = getattr(module, "ExampleSettings", None) + string_attr = module.example_string + + return ExamplePlugin( + _name=name, + _settings_cls=settings_cls, + file=Path(module.__file__), + string_attr=string_attr, + ) + + def expected_attributes(self): + return { + "ExampleSettings": AttributeType( + cls=SettingsBase, + mode=AttributeMode.OPTIONAL, + kind=AttributeKind.CLASS, + ), + "example_string": AttributeType( + cls=str, + mode=AttributeMode.REQUIRED, + kind=AttributeKind.OBJECT, + ), + } diff --git a/tests/plugins/invalid-class/snakemake_example_plugin_invalid_class/__init__.py b/tests/plugins/invalid-class/snakemake_example_plugin_invalid_class/__init__.py new file mode 100644 index 0000000..ab80c04 --- /dev/null +++ b/tests/plugins/invalid-class/snakemake_example_plugin_invalid_class/__init__.py @@ -0,0 +1,7 @@ +"""Plugin module with ExampleSettings class that does not inherit from correct base class.""" + +example_string = "foo" + + +class ExampleSettings: + pass diff --git a/tests/plugins/invalid-object/snakemake_example_plugin_invalid_object/__init__.py b/tests/plugins/invalid-object/snakemake_example_plugin_invalid_object/__init__.py new file mode 100644 index 0000000..af33f94 --- /dev/null +++ b/tests/plugins/invalid-object/snakemake_example_plugin_invalid_object/__init__.py @@ -0,0 +1,3 @@ +"""Plugin module with wrong type for "example_string" attribute.""" + +example_string = 1 diff --git a/tests/plugins/missing-attr/snakemake_example_plugin_missing_attr/__init__.py b/tests/plugins/missing-attr/snakemake_example_plugin_missing_attr/__init__.py new file mode 100644 index 0000000..baf8bcb --- /dev/null +++ b/tests/plugins/missing-attr/snakemake_example_plugin_missing_attr/__init__.py @@ -0,0 +1 @@ +"""Plugin module missing required "example_string" attribute.""" diff --git a/tests/plugins/valid/snakemake_example_plugin_valid_1/__init__.py b/tests/plugins/valid/snakemake_example_plugin_valid_1/__init__.py new file mode 100644 index 0000000..fdd108d --- /dev/null +++ b/tests/plugins/valid/snakemake_example_plugin_valid_1/__init__.py @@ -0,0 +1,10 @@ +"""Valid plugin with optional settings class.""" + +from snakemake_interface_common.plugin_registry.plugin import SettingsBase + + +example_string = "valid 1" + + +class ExampleSettings(SettingsBase): + pass # TODO: add attributes diff --git a/tests/plugins/valid/snakemake_example_plugin_valid_2/__init__.py b/tests/plugins/valid/snakemake_example_plugin_valid_2/__init__.py new file mode 100644 index 0000000..a3c3a9d --- /dev/null +++ b/tests/plugins/valid/snakemake_example_plugin_valid_2/__init__.py @@ -0,0 +1,3 @@ +"""Valid plugin without optional settings class.""" + +example_string = "valid 2" diff --git a/tests/test_registry.py b/tests/test_registry.py new file mode 100644 index 0000000..7cb2737 --- /dev/null +++ b/tests/test_registry.py @@ -0,0 +1,178 @@ +"""Test the PluginRegistry class.""" + +from typing import Iterable +from pathlib import Path +from importlib.metadata import EntryPoints, EntryPoint + +import pytest + +import snakemake_interface_common.plugin_registry +from snakemake_interface_common.plugin_registry.plugin import SettingsBase +from snakemake_interface_common.exceptions import InvalidPluginException +from .example_plugin import ExamplePlugin, ExamplePluginRegistry + + +# Directory containing importable example plugins +PLUGIN_DIR = Path(__file__).parent / "plugins" + + +def patch_entry_points( + monkeypatch: pytest.MonkeyPatch, entry_points: Iterable[EntryPoint] +) -> None: + """ + Monkeypatch the ``importlib.metadata.entry_points()`` function (as imported in the module that + defines ``PluginRegistryBase``) to return the given entry points. + """ + entry_points_obj = EntryPoints(entry_points) + + def entry_points_mocked(**kw): + return entry_points_obj.select(**kw) + + monkeypatch.setattr( + snakemake_interface_common.plugin_registry, "entry_points", entry_points_mocked + ) + + +def test_basic(): + """Test basic attributes and behavior.""" + registry = ExamplePluginRegistry() + assert registry.get_plugin_type() == "example" + + # Check singleton + assert ExamplePluginRegistry() is registry + + # Check plugin not found + assert not registry.is_installed("foo") + with pytest.raises(InvalidPluginException): + registry.get_plugin("foo") + + +def test_discovery(monkeypatch: pytest.MonkeyPatch): + """Test plugin discovery and initialization.""" + + # Add directory of valid plugins to import path so they can be discovered by module name + monkeypatch.syspath_prepend(str(PLUGIN_DIR / "valid")) + + # Mock entry points + patch_entry_points( + monkeypatch, + [ + # Example plugin + EntryPoint( + "entrypoint", + "entrypoint_example_plugin.submodule", + "snakemake.example_plugins", + ), + # Also add a couple entry points that shouldn't be matched + EntryPoint("my-logger", "my_snakemake_logger.logger", "snakemake.loggers"), + EntryPoint("some-tool", "some_tool.cli:cli", "console_scripts"), + ], + ) + + # Now instantiate the registry and check discovery + registry = ExamplePluginRegistry.new() + + expected_plugins = {"valid-1", "valid-2", "entrypoint"} + plugins: dict[str, ExamplePlugin] = {} + + assert set(registry.get_registered_plugins()) == expected_plugins + + for name in expected_plugins: + assert registry.is_installed(name) + plugins[name] = registry.get_plugin(name) + assert isinstance(plugins[name], ExamplePlugin) + assert plugins[name].name == name + assert plugins[name].file.is_relative_to(PLUGIN_DIR) + + # Valid plugin 1 + assert plugins["valid-1"].string_attr == "valid 1" + assert plugins["valid-1"].settings_cls is not None + assert issubclass(plugins["valid-1"].settings_cls, SettingsBase) + + # Valid plugin 2 + assert plugins["valid-2"].string_attr == "valid 2" + assert plugins["valid-2"].settings_cls is None + + # Entrypoint plugin + assert plugins["entrypoint"].string_attr == "entrypoint" + assert plugins["entrypoint"].settings_cls is None + + +def test_missing_attr(monkeypatch: pytest.MonkeyPatch): + """Test plugin with missing required attribute.""" + + monkeypatch.syspath_prepend(str(PLUGIN_DIR / "missing-attr")) + + with pytest.raises(InvalidPluginException) as exc_info: + ExamplePluginRegistry.new() + + errmsg = str(exc_info.value) + assert "missing-attr" in errmsg + assert "plugin does not define example_string" in errmsg + + +def test_invalid_object(monkeypatch: pytest.MonkeyPatch): + """Test plugin with invalid object attribute.""" + + monkeypatch.syspath_prepend(str(PLUGIN_DIR / "invalid-object")) + + with pytest.raises(InvalidPluginException) as exc_info: + ExamplePluginRegistry.new() + + errmsg = str(exc_info.value) + assert "invalid-object" in errmsg + assert "example_string must be of type str" in errmsg + + +def test_invalid_class(monkeypatch: pytest.MonkeyPatch): + """Test plugin with invalid class attribute.""" + + monkeypatch.syspath_prepend(str(PLUGIN_DIR / "invalid-class")) + + with pytest.raises(InvalidPluginException) as exc_info: + ExamplePluginRegistry.new() + + errmsg = str(exc_info.value) + assert "invalid-class" in errmsg + assert "ExampleSettings must be a subclass of" in errmsg + assert "SettingsBase" in errmsg + + +def test_invalid_entry_point(monkeypatch): + """Test invalid entry points.""" + + # Colon in entry point value + with monkeypatch.context() as ctx: + patch_entry_points( + ctx, + [ + EntryPoint( + "non-module", "my_example_plugin:foo", "snakemake.example_plugins" + ) + ], + ) + + with pytest.raises(InvalidPluginException) as exc_info: + ExamplePluginRegistry.new() + + errmsg = str(exc_info.value) + assert "non-module" in errmsg + assert "my_example_plugin:foo" in errmsg + + # Not importable + with monkeypatch.context() as ctx: + patch_entry_points( + ctx, + [ + EntryPoint( + "unimportable", "my_example_plugin.foo", "snakemake.example_plugins" + ) + ], + ) + + with pytest.raises(InvalidPluginException) as exc_info: + ExamplePluginRegistry.new() + + errmsg = str(exc_info.value) + assert "unimportable" in errmsg + assert "unable to import my_example_plugin.foo" in errmsg