From e59b7c7182188453882452437f981269d2d31e8b Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Sun, 13 Mar 2022 12:31:33 -0400 Subject: [PATCH 01/13] changes to _from_npe1 required for shim conversion --- npe2/_from_npe1.py | 395 +++++++++++------- npe2/manifest/utils.py | 133 +++++- tests/conftest.py | 70 +++- .../npe1-plugin-0.0.1.dist-info/METADATA | 1 + tests/npe1-plugin/npe1_module/__init__.py | 26 +- tests/npe1-plugin/setup.cfg | 1 + tests/test_cli.py | 2 +- tests/test_conversion.py | 54 ++- 8 files changed, 510 insertions(+), 172 deletions(-) diff --git a/npe2/_from_npe1.py b/npe2/_from_npe1.py index ff9f609b..68da7a88 100644 --- a/npe2/_from_npe1.py +++ b/npe2/_from_npe1.py @@ -1,18 +1,20 @@ import ast -import itertools +import inspect import re import sys import warnings from configparser import ConfigParser from dataclasses import dataclass from functools import lru_cache +from importlib import import_module from pathlib import Path +from types import ModuleType from typing import ( Any, Callable, DefaultDict, Dict, - Iterable, + Iterator, List, Optional, Tuple, @@ -20,17 +22,12 @@ cast, ) -from napari_plugin_engine import ( - HookCaller, - HookImplementation, - PluginManager, - napari_hook_specification, -) - from npe2.manifest import PluginManifest from npe2.manifest.commands import CommandContribution from npe2.manifest.themes import ThemeColors +from npe2.manifest.utils import SHIM_NAME_PREFIX, import_python_name, merge_manifests from npe2.manifest.widgets import WidgetContribution +from npe2.types import WidgetCreator try: from importlib import metadata @@ -39,28 +36,42 @@ NPE1_EP = "napari.plugin" NPE2_EP = "napari.manifest" +NPE1_IMPL_TAG = "napari_impl" # same as HookImplementation.format_tag("napari") + + +class HookImplementation: + def __init__( + self, + function: Callable, + plugin: Optional[ModuleType] = None, + plugin_name: Optional[str] = None, + **kwargs, + ): + self.function = function + self.plugin = plugin + self.plugin_name = plugin_name + self._specname = kwargs.get("specname") + def __repr__(self) -> str: # pragma: no cover + return ( + f"" + ) -# fmt: off -class HookSpecs: - def napari_provide_sample_data(): ... # type: ignore # noqa: E704 - def napari_get_reader(path): ... # noqa: E704 - def napari_get_writer(path, layer_types): ... # noqa: E704 - def napari_write_image(path, data, meta): ... # noqa: E704 - def napari_write_labels(path, data, meta): ... # noqa: E704 - def napari_write_points(path, data, meta): ... # noqa: E704 - def napari_write_shapes(path, data, meta): ... # noqa: E704 - def napari_write_surface(path, data, meta): ... # noqa: E704 - def napari_write_vectors(path, data, meta): ... # noqa: E704 - def napari_experimental_provide_function(): ... # type: ignore # noqa: E704 - def napari_experimental_provide_dock_widget(): ... # type: ignore # noqa: E704 - def napari_experimental_provide_theme(): ... # type: ignore # noqa: E704 -# fmt: on + @property + def specname(self) -> str: + return self._specname or self.function.__name__ -for m in dir(HookSpecs): - if m.startswith("napari"): - setattr(HookSpecs, m, napari_hook_specification(getattr(HookSpecs, m))) +def iter_hookimpls( + module: ModuleType, plugin_name: Optional[str] = None +) -> Iterator[HookImplementation]: + # yield all routines in module that have "{self.project_name}_impl" attr + for name in dir(module): + method = getattr(module, name) + if hasattr(method, NPE1_IMPL_TAG) and inspect.isroutine(method): + hookimpl_opts = getattr(method, NPE1_IMPL_TAG) + if isinstance(hookimpl_opts, dict): + yield HookImplementation(method, module, plugin_name, **hookimpl_opts) @dataclass @@ -71,11 +82,6 @@ class PluginPackage: top_module: str setup_cfg: Optional[Path] = None - @property - def name_pairs(self): - names = (self.ep_name, self.package_name, self.top_module) - return itertools.product(names, repeat=2) - @lru_cache() def plugin_packages() -> List[PluginPackage]: @@ -98,102 +104,112 @@ def plugin_packages() -> List[PluginPackage]: return packages -def ensure_package_name(name: str): - """Try all the tricks we know to find a package name given a plugin name.""" - for attr in ("package_name", "ep_name", "top_module"): - for p in plugin_packages(): - if name == getattr(p, attr): - return p.package_name - raise KeyError( # pragma: no cover - f"Unable to find a locally installed package for plugin {name!r}" - ) - - -@lru_cache() -def npe1_plugin_manager() -> Tuple[PluginManager, Tuple[int, list]]: - pm = PluginManager("napari", discover_entry_point=NPE1_EP) - pm.add_hookspecs(HookSpecs) - result = pm.discover() - return pm, result - - -def norm_plugin_name(plugin_name: Optional[str] = None, module: Any = None) -> str: - """Try all the things we know to detect something called `plugin_name`.""" - plugin_manager, (_, errors) = npe1_plugin_manager() - - # directly providing a module is mostly for testing. - if module is not None: - if plugin_name: # pragma: no cover - warnings.warn("module provided, plugin_name ignored") - plugin_name = getattr(module, "__name__", "dynamic_plugin") - if not plugin_manager.is_registered(plugin_name): - plugin_manager.register(module, plugin_name) - return cast(str, plugin_name) - - if plugin_name in plugin_manager.plugins: - return cast(str, plugin_name) - - for pkg in plugin_packages(): - for a, b in pkg.name_pairs: - if plugin_name == a and b in plugin_manager.plugins: - return b - - # we couldn't find it: - for e in errors: # pragma: no cover - if module and e.plugin == module: - raise type(e)(e.format()) - for pkg in plugin_packages(): - if plugin_name in (pkg.ep_name, pkg.package_name, pkg.top_module): - raise type(e)(e.format()) - - msg = f"We tried hard! but could not detect a plugin named {plugin_name!r}." - if plugin_manager.plugins: - msg += f" Plugins found include: {list(plugin_manager.plugins)}" - raise metadata.PackageNotFoundError(msg) - - def manifest_from_npe1( - plugin_name: Optional[str] = None, module: Any = None + plugin: Union[str, metadata.Distribution, None] = None, + module: Any = None, + shim=False, ) -> PluginManifest: - """Return manifest object given npe1 plugin_name or package name. + """Return manifest object given npe1 plugin or package name. - One of `plugin_name` or `module` must be provide. + One of `plugin` or `module` must be provide. Parameters ---------- - plugin_name : str - Name of package/plugin to convert, by default None - module : Module + plugin : Union[str, metadata.Distribution, None] + Name of package/plugin to convert. Or a `metadata.Distribution` object. + If a string, this function should be prepared to accept both the name of the + package, and the name of an npe1 `napari.plugin` entry_point. by default None + module : Optional[Module] namespace object, to directly import (mostly for testing.), by default None + shim : bool + If True, the resulting manifest will be used internally by NPE1Adaptor, but + is NOT necessarily suitable for export as npe2 manifest. This will handle + cases of locally defined functions and partials that don't have global + python_names that are not supported natively by npe2. by default False """ - plugin_manager, _ = npe1_plugin_manager() - plugin_name = norm_plugin_name(plugin_name, module) - - _module = plugin_manager.plugins[plugin_name] - package = ensure_package_name(plugin_name) if module is None else "dynamic" + if module is not None: + modules: List[str] = [module] + package_name = "dynamic" + plugin_name = getattr(module, "__name__", "dynamic_plugin") + elif isinstance(plugin, str): + + modules = [] + plugin_name = plugin + for pp in plugin_packages(): + if plugin in (pp.ep_name, pp.package_name): + modules.append(pp.ep_value) + package_name = pp.package_name + if not modules: + _avail = [f" {p.package_name} ({p.ep_name})" for p in plugin_packages()] + avail = "\n".join(_avail) + raise metadata.PackageNotFoundError( + f"No package or entry point found with name {plugin!r}: " + f"\nFound packages (entry_point):\n{avail}" + ) + elif hasattr(plugin, "entry_points") and hasattr(plugin, "metadata"): + plugin = cast(metadata.Distribution, plugin) + # don't use isinstance(Distribution), setuptools monkeypatches sys.meta_path: + # https://github.com/pypa/setuptools/issues/3169 + NPE1_ENTRY_POINT = "napari.plugin" + plugin_name = package_name = plugin.metadata["Name"] + modules = [ + ep.value for ep in plugin.entry_points if ep.group == NPE1_ENTRY_POINT + ] + assert modules, f"No npe1 entry points found in distribution {plugin_name!r}" + else: + raise ValueError("one of plugin or module must be provided") # pragma: no cover - parser = HookImplParser(package, plugin_name) - parser.parse_callers(plugin_manager._plugin2hookcallers[_module]) + manifests: List[PluginManifest] = [] + for mod_name in modules: + parser = HookImplParser(package_name, plugin_name or "", shim=shim) + _mod = import_module(mod_name) if isinstance(mod_name, str) else mod_name + parser.parse_module(_mod) + manifests.append(parser.manifest()) - return PluginManifest(name=package, contributions=dict(parser.contributions)) + assert manifests, "No npe1 entry points found in distribution {name}" + return merge_manifests(manifests) class HookImplParser: - def __init__(self, package: str, plugin_name: str) -> None: + def __init__(self, package: str, plugin_name: str, shim: bool = False) -> None: + """A visitor class to convert npe1 hookimpls to a npe2 manifest + + Parameters + ---------- + package : str + [description] + plugin_name : str + [description] + shim : bool, optional + If True, the resulting manifest will be used internally by NPE1Adaptor, but + is NOT necessarily suitable for export as npe2 manifest. This will handle + cases of locally defined functions and partials that don't have global + python_names that are not supported natively by npe2. by default False + + Examples + -------- + >>> parser = HookImplParser(package, plugin_name, shim=shim) + >>> parser.parse_callers(plugin_manager._plugin2hookcallers[_module]) + >>> mf = PluginManifest(name=package, contributions=dict(parser.contributions)) + """ self.package = package self.plugin_name = plugin_name self.contributions: DefaultDict[str, list] = DefaultDict(list) + self.shim = shim - def parse_callers(self, callers: Iterable[HookCaller]): - for caller in callers: - for impl in caller.get_hookimpls(): - if self.plugin_name and impl.plugin_name != self.plugin_name: - continue # pragma: no cover + def manifest(self) -> PluginManifest: + return PluginManifest(name=self.package, contributions=dict(self.contributions)) + + def parse_module(self, module: ModuleType): + for impl in iter_hookimpls(module, plugin_name=self.plugin_name): + if impl.plugin_name == self.plugin_name: # call the corresponding hookimpl parser try: getattr(self, impl.specname)(impl) except Exception as e: # pragma: no cover - warnings.warn(f"Failed to convert {impl.specname}: {e}") + warnings.warn( + f"Failed to convert {impl.specname} in {self.package!r}: {e}" + ) def napari_experimental_provide_theme(self, impl: HookImplementation): ThemeDict = Dict[str, Union[str, Tuple, List]] @@ -212,11 +228,14 @@ def napari_experimental_provide_theme(self, impl: HookImplementation): ) def napari_get_reader(self, impl: HookImplementation): + + patterns = _guess_fname_patterns(impl.function) + self.contributions["readers"].append( { "command": self.add_command(impl), "accepts_directories": True, - "filename_patterns": [""], + "filename_patterns": patterns, } ) @@ -224,7 +243,7 @@ def napari_provide_sample_data(self, impl: HookImplementation): module = sys.modules[impl.function.__module__.split(".", 1)[0]] samples: Dict[str, Union[dict, str, Callable]] = impl.function() - for key, sample in samples.items(): + for idx, (key, sample) in enumerate(samples.items()): _sample: Union[str, Callable] if isinstance(sample, dict): display_name = sample.get("display_name") @@ -238,9 +257,12 @@ def napari_provide_sample_data(self, impl: HookImplementation): if callable(_sample): # let these raise exceptions here immediately if they don't validate id = f"{self.package}.data.{_key}" + py_name = _python_name( + _sample, impl.function, shim_idx=idx if self.shim else None + ) cmd_contrib = CommandContribution( id=id, - python_name=_python_name(_sample), + python_name=py_name, title=f"{key} sample", ) self.contributions["commands"].append(cmd_contrib) @@ -254,14 +276,15 @@ def napari_provide_sample_data(self, impl: HookImplementation): def napari_experimental_provide_function(self, impl: HookImplementation): items: Union[Callable, List[Callable]] = impl.function() - if not isinstance(items, list): - items = [items] + items = [items] if not isinstance(items, list) else items + for idx, item in enumerate(items): try: cmd = f"{self.package}.{item.__name__}" - py_name = _python_name(item) - + py_name = _python_name( + item, impl.function, shim_idx=idx if self.shim else None + ) docsum = item.__doc__.splitlines()[0] if item.__doc__ else None cmd_contrib = CommandContribution( id=cmd, python_name=py_name, title=docsum or item.__name__ @@ -288,6 +311,8 @@ def napari_experimental_provide_dock_widget(self, impl: HookImplementation): if not isinstance(items, list): items = [items] # pragma: no cover + # "wdg_creator" will be the function given by the plugin that returns a widget + # while `impl` is the hook implementation that returned all the `wdg_creators` for idx, item in enumerate(items): if isinstance(item, tuple): wdg_creator = item[0] @@ -301,7 +326,11 @@ def napari_experimental_provide_dock_widget(self, impl: HookImplementation): continue try: - self._create_widget_contrib(impl, wdg_creator, kwargs) + func_name = getattr(wdg_creator, "__name__", "") + wdg_name = str(kwargs.get("name", "")) or _camel_to_spaces(func_name) + self._create_widget_contrib( + wdg_creator, display_name=wdg_name, idx=idx, hook=impl.function + ) except Exception as e: # pragma: no cover msg = ( f"Error converting dock widget [{idx}] " @@ -309,29 +338,18 @@ def napari_experimental_provide_dock_widget(self, impl: HookImplementation): ) warnings.warn(msg) - def _create_widget_contrib(self, impl, wdg_creator, kwargs, is_function=False): - # Get widget name - func_name = getattr(wdg_creator, "__name__", "") - wdg_name = str(kwargs.get("name", "")) or _camel_to_spaces(func_name) - - # in some cases, like partials and magic_factories, there might not be an - # easily accessible python name (from __module__.__qualname__)... - # so first we look for this object in the module namespace - py_name = None - cmd = None - for local_name, val in impl.function.__globals__.items(): - if val is wdg_creator: - py_name = f"{impl.function.__module__}:{local_name}" - cmd = f"{self.package}.{local_name}" - break - else: - try: - py_name = _python_name(wdg_creator) - cmd = ( - f"{self.package}.{func_name or wdg_name.lower().replace(' ', '_')}" - ) - except AttributeError: # pragma: no cover - pass + def _create_widget_contrib( + self, + wdg_creator: WidgetCreator, + display_name: str, + idx: int, + hook: Callable, + ): + # we provide both the wdg_creator object itself, as well as the hook impl that + # returned it... In the case that we can't get an absolute python name to the + # wdg_creator itself (e.g. it's defined in a local scope), then the py_name + # will use the hookimpl itself, and the index of the object returned. + py_name = _python_name(wdg_creator, hook, shim_idx=idx if self.shim else None) if not py_name: # pragma: no cover raise ValueError( @@ -339,18 +357,21 @@ def _create_widget_contrib(self, impl, wdg_creator, kwargs, is_function=False): "Is this a locally defined function or partial?" ) + func_name = getattr(wdg_creator, "__name__", "") + cmd = f"{self.package}.{func_name or display_name.lower().replace(' ', '_')}" + # let these raise exceptions here immediately if they don't validate cmd_contrib = CommandContribution( - id=cmd, python_name=py_name, title=f"Create {wdg_name}" + id=cmd, python_name=py_name, title=f"Create {display_name}" ) - wdg_contrib = WidgetContribution(command=cmd, display_name=wdg_name) + wdg_contrib = WidgetContribution(command=cmd, display_name=display_name) self.contributions["commands"].append(cmd_contrib) self.contributions["widgets"].append(wdg_contrib) def napari_get_writer(self, impl: HookImplementation): warnings.warn( - "Found a multi-layer writer, but it's not convertable. " - "Please add the writer manually." + f"Found a multi-layer writer in {self.package!r} - {impl.specname!r}, " + "but it's not convertable. Please add the writer manually." ) return NotImplemented # pragma: no cover @@ -376,7 +397,7 @@ def _parse_writer(self, impl: HookImplementation, layer: str): "command": id, "layer_types": [layer], "display_name": layer, - "filename_extensions": [""], + "filename_extensions": [], } ) @@ -403,8 +424,70 @@ def _safe_key(key: str) -> str: ) -def _python_name(object): - return f"{object.__module__}:{object.__qualname__}" +def _python_name( + obj: Any, hook: Callable = None, shim_idx: Optional[int] = None +) -> str: + """Get resolvable python name for `obj` returned from an npe1 `hook` implentation. + + Parameters + ---------- + obj : Any + a python obj + hook : Callable, optional + the npe1 hook implementation that returned `obj`, by default None. + This is used both to search the module namespace for `obj`, and also + in the shim python name if `obj` cannot be found. + shim_idx : int, optional + If `obj` cannot be found and `shim_idx` is not None, then a shim name. + of the form "__npe1shim__.{_python_name(hook)}_{shim_idx}" will be returned. + by default None. + + Returns + ------- + str + a string that can be imported with npe2.manifest.utils.import_python_name + + Raises + ------ + AttributeError + If a resolvable string cannot be found + """ + obj_name: Optional[str] = None + mod_name: Optional[str] = None + # first, check the global namespace of the module where the hook was declared + # if we find `obj` itself, we can just use it. + if hasattr(hook, "__module__"): + hook_mod = sys.modules.get(hook.__module__) + if hook_mod: + for local_name, _obj in vars(hook_mod).items(): + if _obj is obj: + obj_name = local_name + mod_name = hook_mod.__name__ + break + + # if that didn't work get the qualname of the object + # and, if it's not a locally defined qualname, get the name of the module + # in which it is defined + if not (mod_name and obj_name): + obj_name = getattr(obj, "__qualname__", "") + if obj_name and "" not in obj_name: + mod = inspect.getmodule(obj) or inspect.getmodule(hook) + if mod: + mod_name = mod.__name__ + + if not (mod_name and obj_name) and (hook and shim_idx is not None): + # we weren't able to resolve an absolute name... if we are shimming, then we + # can create a special py_name of the form `__npe1shim__.hookfunction_idx` + return f"{SHIM_NAME_PREFIX}{_python_name(hook)}_{shim_idx}" + + if obj_name and "" in obj_name: + raise ValueError("functions defined in local scopes are not yet supported.") + if not mod_name: + raise AttributeError(f"could not get resolvable python name for {obj}") + pyname = f"{mod_name}:{obj_name}" + if import_python_name(pyname) is not obj: # pragma: no cover + raise AttributeError(f"could not get resolvable python name for {obj}") + return pyname def _luma(r, g, b): @@ -572,3 +655,25 @@ def visit_Call(self, node: ast.Call) -> Any: self._entry_points.append( [i.strip() for i in item.split("=")] ) + + +def _guess_fname_patterns(func): + """Try to guess filename extension patterns from source code. Fallback to "*".""" + + patterns = ["*"] + # try to look at source code to guess file extensions + _, *b = inspect.getsource(func).split("endswith(") + if b: + try: + middle = b[0].split(")")[0] + if middle.startswith("("): + middle += ")" + files = ast.literal_eval(middle) + if isinstance(files, str): + files = [files] + if files: + patterns = [f"*{f}" for f in files] + except Exception: # pragma: no cover + # couldn't do it... just accept all filename patterns + pass + return patterns diff --git a/npe2/manifest/utils.py b/npe2/manifest/utils.py index 57cf0681..bcae439c 100644 --- a/npe2/manifest/utils.py +++ b/npe2/manifest/utils.py @@ -10,18 +10,23 @@ Dict, Generic, Optional, + Sequence, SupportsInt, Tuple, TypeVar, Union, ) +if TYPE_CHECKING: + from npe2.manifest.schema import PluginManifest + from ..types import PythonName if TYPE_CHECKING: from typing_extensions import Protocol from .._command_registry import CommandRegistry + from .contributions import ContributionPoints class ProvidesCommand(Protocol): command: str @@ -31,6 +36,7 @@ def get_callable(self, _registry: Optional[CommandRegistry] = None): R = TypeVar("R") +SHIM_NAME_PREFIX = "__npe1shim__." # TODO: add ParamSpec when it's supported better by mypy @@ -153,16 +159,131 @@ def __str__(self) -> str: return v -def import_python_name(python_name: PythonName) -> Any: +def _import_npe1_shim(shim_name: str) -> Any: + """Import npe1 shimmed python_name + + Some objects returned by npe1 hooks (such as locally defined partials or other + objects) don't have globally accessible python names. In such cases, we create + a "shim" python_name of the form: + + `__npe1shim__._` + + The implication is that the hook should be imported, called, and indexed to return + the corresponding item in the hook results. + + Parameters + ---------- + shim_name : str + A string in the form `__npe1shim__._` + + Returns + ------- + Any + The th object returned from the callable . + + Raises + ------ + IndexError + If len(()) <= + """ + + assert shim_name.startswith(SHIM_NAME_PREFIX), f"Invalid shim name: {shim_name}" + python_name, idx = shim_name[13:].rsplit("_", maxsplit=1) # TODO, make a function + index = int(idx) + + hook = import_python_name(python_name) + result = hook() + if isinstance(result, dict): + # things like sample_data hookspec return a dict, in which case we want the + # "idxth" item in the dict (assumes ordered dict, which is safe now) + result = list(result.values()) + if not isinstance(result, list): + result = [result] # pragma: no cover + + try: + out = result[index] + except IndexError as e: # pragma: no cover + raise IndexError(f"invalid npe1 shim index {index} for hook {hook}") from e + + if "dock_widget" in python_name and isinstance(out, tuple): + return out[0] + if "sample_data" in python_name and isinstance(out, dict): + # this was a nested sample data + return out.get("data") + + return out + + +def import_python_name(python_name: Union[PythonName, str]) -> Any: from importlib import import_module - from ._validators import PYTHON_NAME_PATTERN + from . import _validators - match = PYTHON_NAME_PATTERN.match(python_name) - if not match: # pragma: no cover - raise ValueError(f"Invalid python name: {python_name}") + if python_name.startswith(SHIM_NAME_PREFIX): + return _import_npe1_shim(python_name) - module_name, funcname = match.groups() + _validators.python_name(python_name) # shows the best error message + match = _validators.PYTHON_NAME_PATTERN.match(python_name) + module_name, funcname = match.groups() # type: ignore [union-attr] mod = import_module(module_name) return getattr(mod, funcname) + + +def deep_update(dct: dict, merge_dct: dict, copy=True) -> dict: + """Merge possibly nested dicts""" + from copy import deepcopy + + _dct = deepcopy(dct) if copy else dct + for k, v in merge_dct.items(): + if k in _dct and isinstance(dct[k], dict) and isinstance(v, dict): + deep_update(_dct[k], v, copy=False) + elif isinstance(v, list): + if k not in _dct: + _dct[k] = [] + _dct[k].extend(v) + else: + _dct[k] = v + return _dct + + +def merge_manifests(manifests: Sequence[PluginManifest]): + from npe2.manifest.schema import PluginManifest + + if not manifests: + raise ValueError("Cannot merge empty sequence of manifests") + if len(manifests) == 1: + return manifests[0] + + assert len({mf.name for mf in manifests}) == 1, "All manifests must have same name" + assert ( + len({mf.package_version for mf in manifests}) == 1 + ), "All manifests must have same version" + assert ( + len({mf.display_name for mf in manifests}) == 1 + ), "All manifests must have same display_name" + + mf0 = manifests[0] + info = mf0.dict(exclude={"contributions"}, exclude_unset=True) + info["contributions"] = merge_contributions([m.contributions for m in manifests]) + return PluginManifest(**info) + + +def merge_contributions(contribs: Sequence[Optional[ContributionPoints]]) -> dict: + _contribs = [c for c in contribs if c and c.dict(exclude_unset=True)] + if not _contribs: + return {} # pragma: no cover + + out = _contribs[0].dict(exclude_unset=True) + if len(_contribs) > 1: + for n, ctrb in enumerate(_contribs[1:]): + c = ctrb.dict(exclude_unset=True) + for cmd in c.get("commands", ()): + cmd["id"] = cmd["id"] + f"_{n + 2}" + for name, val in c.items(): + if isinstance(val, list): + for item in val: + if "command" in item: + item["command"] = item["command"] + f"_{n + 2}" + out = deep_update(out, c) + return out diff --git a/tests/conftest.py b/tests/conftest.py index 93fcc9fa..8cf39683 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,5 @@ import sys +from importlib import abc from pathlib import Path from unittest.mock import patch @@ -6,6 +7,11 @@ from npe2 import PluginManager, PluginManifest +try: + from importlib import metadata +except ImportError: + import importlib_metadata as metadata # type: ignore + @pytest.fixture def sample_path(): @@ -20,10 +26,12 @@ def sample_manifest(sample_path): @pytest.fixture def uses_sample_plugin(sample_path): sys.path.append(str(sample_path)) - pm = PluginManager.instance() - pm.discover() - yield - sys.path.remove(str(sample_path)) + try: + pm = PluginManager.instance() + pm.discover() + yield + finally: + sys.path.remove(str(sample_path)) @pytest.fixture @@ -55,6 +63,30 @@ def npe1_repo(): return Path(__file__).parent / "npe1-plugin" +@pytest.fixture +def uses_npe1_plugin(npe1_repo): + import site + + class Importer(abc.MetaPathFinder): + def find_spec(self, *_, **__): + return None + + def find_distributions(self, ctx, **k): + if ctx.name == "npe1-plugin": + pth = npe1_repo / "npe1-plugin-0.0.1.dist-info" + yield metadata.PathDistribution(pth) + return + + sys.meta_path.append(Importer()) + sys.path.append(str(npe1_repo)) + try: + pkgs = site.getsitepackages() + [str(npe1_repo)] + with patch("site.getsitepackages", return_value=pkgs): + yield + finally: + sys.path.remove(str(npe1_repo)) + + @pytest.fixture def npe1_plugin_module(npe1_repo): import sys @@ -74,23 +106,39 @@ def npe1_plugin_module(npe1_repo): @pytest.fixture def mock_npe1_pm(): - from napari_plugin_engine import PluginManager - - from npe2._from_npe1 import HookSpecs + from napari_plugin_engine import PluginManager, napari_hook_specification + + # fmt: off + class HookSpecs: + def napari_provide_sample_data(): ... # type: ignore # noqa: E704 + def napari_get_reader(path): ... # noqa: E704 + def napari_get_writer(path, layer_types): ... # noqa: E704 + def napari_write_image(path, data, meta): ... # noqa: E704 + def napari_write_labels(path, data, meta): ... # noqa: E704 + def napari_write_points(path, data, meta): ... # noqa: E704 + def napari_write_shapes(path, data, meta): ... # noqa: E704 + def napari_write_surface(path, data, meta): ... # noqa: E704 + def napari_write_vectors(path, data, meta): ... # noqa: E704 + def napari_experimental_provide_function(): ... # type: ignore # noqa: E704 + def napari_experimental_provide_dock_widget(): ... # type: ignore # noqa: E704 + def napari_experimental_provide_theme(): ... # type: ignore # noqa: E704 + # fmt: on + + for m in dir(HookSpecs): + if m.startswith("napari"): + setattr(HookSpecs, m, napari_hook_specification(getattr(HookSpecs, m))) pm = PluginManager("napari") pm.add_hookspecs(HookSpecs) - with patch("npe2._from_npe1.npe1_plugin_manager", new=lambda: (pm, (1, []))): - yield pm + yield pm @pytest.fixture -def mock_npe1_pm_with_plugin(npe1_repo, mock_npe1_pm, npe1_plugin_module): +def mock_npe1_pm_with_plugin(npe1_repo, npe1_plugin_module): """Mocks a fully installed local repository""" from npe2._from_npe1 import metadata, plugin_packages - mock_npe1_pm.register(npe1_plugin_module, "npe1-plugin") mock_dist = metadata.PathDistribution(npe1_repo / "npe1-plugin-0.0.1.dist-info") def _dists(): diff --git a/tests/npe1-plugin/npe1-plugin-0.0.1.dist-info/METADATA b/tests/npe1-plugin/npe1-plugin-0.0.1.dist-info/METADATA index a3006648..0e40ff76 100644 --- a/tests/npe1-plugin/npe1-plugin-0.0.1.dist-info/METADATA +++ b/tests/npe1-plugin/npe1-plugin-0.0.1.dist-info/METADATA @@ -1,2 +1,3 @@ Metadata-Version: 2.1 Name: npe1-plugin +Version: 0.1.0 diff --git a/tests/npe1-plugin/npe1_module/__init__.py b/tests/npe1-plugin/npe1_module/__init__.py index ad52238d..2dfe0fe4 100644 --- a/tests/npe1-plugin/npe1_module/__init__.py +++ b/tests/npe1-plugin/npe1_module/__init__.py @@ -1,3 +1,6 @@ +from functools import partial + +import numpy as np from magicgui import magic_factory from napari_plugin_engine import napari_hook_implementation @@ -33,11 +36,16 @@ def napari_write_labels(path, data, meta): def napari_provide_sample_data(): return { "random data": gen_data, + "local data": partial(np.ones, (4, 4)), "random image": "https://picsum.photos/1024", "sample_key": { "display_name": "Some Random Data (512 x 512)", "data": gen_data, }, + "local_ones": { + "display_name": "Some local ones", + "data": partial(np.ones, (4, 4)), + }, } @@ -65,11 +73,25 @@ def napari_experimental_provide_theme(): } +factory = magic_factory(some_function) + + @napari_hook_implementation def napari_experimental_provide_dock_widget(): - return [MyWidget, (magic_factory(some_function), {"name": "My Other Widget"})] + @magic_factory + def local_widget(y: str): + ... + + return [ + MyWidget, + (factory, {"name": "My Other Widget"}), + (local_widget, {"name": "Local Widget"}), + ] @napari_hook_implementation def napari_experimental_provide_function(): - return some_function + def local_function(x: int): + ... + + return [some_function, local_function] diff --git a/tests/npe1-plugin/setup.cfg b/tests/npe1-plugin/setup.cfg index 3df0296b..08691809 100644 --- a/tests/npe1-plugin/setup.cfg +++ b/tests/npe1-plugin/setup.cfg @@ -1,5 +1,6 @@ [metadata] name = npe1-plugin +version = 0.1.0 [options.entry_points] napari.plugin = diff --git a/tests/test_cli.py b/tests/test_cli.py index c0a17946..6c9934aa 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -63,7 +63,7 @@ def test_cli_convert_repo_dry_run(npe1_repo, mock_npe1_pm_with_plugin): def test_cli_convert_svg(): result = runner.invoke(app, ["convert", "napari-svg"]) assert "Some issues occured:" in result.stdout - assert "Found a multi-layer writer, but it's not convertable" in result.stdout + assert "Found a multi-layer writer in 'napari-svg'" in result.stdout assert result.exit_code == 0 diff --git a/tests/test_conversion.py b/tests/test_conversion.py index ada5dcfc..8a410023 100644 --- a/tests/test_conversion.py +++ b/tests/test_conversion.py @@ -1,5 +1,6 @@ import pytest +from npe2 import _from_npe1 from npe2._from_npe1 import convert_repository, get_top_module_path, manifest_from_npe1 try: @@ -9,12 +10,15 @@ @pytest.mark.filterwarnings("ignore:The distutils package is deprecated") -@pytest.mark.filterwarnings("ignore:Found a multi-layer writer, but it's not") +@pytest.mark.filterwarnings("ignore:Found a multi-layer writer in") @pytest.mark.parametrize("package", ["svg"]) def test_conversion(package): assert manifest_from_npe1(package) +@pytest.mark.filterwarnings("ignore:Failed to convert napari_provide_sample_data") +@pytest.mark.filterwarnings("ignore:Error converting function") +@pytest.mark.filterwarnings("ignore:Error converting dock widget") def test_conversion_from_module(mock_npe1_pm, npe1_plugin_module): mf = manifest_from_npe1(module=npe1_plugin_module) assert isinstance(mf.dict(), dict) @@ -39,6 +43,9 @@ def f(x: int): assert isinstance(mf.dict(), dict) +@pytest.mark.filterwarnings("ignore:Failed to convert napari_provide_sample_data") +@pytest.mark.filterwarnings("ignore:Error converting function") +@pytest.mark.filterwarnings("ignore:Error converting dock widget") def test_conversion_from_package(npe1_repo, mock_npe1_pm_with_plugin): setup_cfg = npe1_repo / "setup.cfg" before = setup_cfg.read_text() @@ -60,6 +67,20 @@ def test_conversion_from_package(npe1_repo, mock_npe1_pm_with_plugin): assert "Is this package already converted?" in str(e.value) +def _assert_expected_errors(record: pytest.WarningsRecorder): + assert len(record) == 4 + msg = str(record[0].message) + assert "Error converting dock widget [2] from 'npe1_module'" in msg + msg = str(record[1].message) + assert "Error converting function [1] from 'npe1_module'" in msg + msg = str(record[2].message) + assert "Failed to convert napari_provide_sample_data in 'npe1-plugin'" in msg + assert "could not get resolvable python name" in msg + msg = str(record[3].message) + assert "Cannot auto-update setup.py, please edit setup.py as follows" in msg + assert "npe1-plugin = npe1_module:napari.yaml" in msg + + def test_conversion_from_package_setup_py(npe1_repo, mock_npe1_pm_with_plugin): (npe1_repo / "setup.cfg").unlink() (npe1_repo / "setup.py").write_text( @@ -73,9 +94,7 @@ def test_conversion_from_package_setup_py(npe1_repo, mock_npe1_pm_with_plugin): ) with pytest.warns(UserWarning) as record: convert_repository(npe1_repo) - msg = record[0].message - assert "Cannot auto-update setup.py, please edit setup.py as follows" in str(msg) - assert "npe1-plugin = npe1_module:napari.yaml" in str(msg) + _assert_expected_errors(record) def test_conversion_entry_point_string(npe1_repo, mock_npe1_pm_with_plugin): @@ -91,9 +110,7 @@ def test_conversion_entry_point_string(npe1_repo, mock_npe1_pm_with_plugin): ) with pytest.warns(UserWarning) as record: convert_repository(npe1_repo) - msg = record[0].message - assert "Cannot auto-update setup.py, please edit setup.py as follows" in str(msg) - assert "npe1-plugin = npe1_module:napari.yaml" in str(msg) + _assert_expected_errors(record) def test_conversion_missing(): @@ -112,3 +129,26 @@ def test_convert_repo(): def test_get_top_module_path(mock_npe1_pm_with_plugin): get_top_module_path("npe1-plugin") + + +def test_python_name_local(): + def f(): + return lambda x: None + + with pytest.raises(ValueError) as e: + _from_npe1._python_name(f()) + + assert "functions defined in local scopes are not yet supported" in str(e.value) + + +def test_guess_fname_patterns(): + def get_reader1(path): + if isinstance(path, str) and path.endswith((".tiff", ".tif")): + return 1 + + def get_reader2(path): + if path.endswith(".xyz"): + return 1 + + assert _from_npe1._guess_fname_patterns(get_reader1) == ["*.tiff", "*.tif"] + assert _from_npe1._guess_fname_patterns(get_reader2) == ["*.xyz"] From bbb530ec675a4e414c4192f0b4b15d14ca8acc7f Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Sun, 13 Mar 2022 12:38:03 -0400 Subject: [PATCH 02/13] shim2 just the shim part --- npe2/_plugin_manager.py | 12 +++- npe2/manifest/_npe1_shim.py | 38 +++++++++++ npe2/manifest/schema.py | 125 ++++++++++++++++++++++++------------ tests/test_manifest.py | 10 ++- tests/test_shim.py | 102 +++++++++++++++++++++++++++++ tests/test_utils.py | 52 ++++++++++++++- tests/test_validations.py | 7 ++ 7 files changed, 299 insertions(+), 47 deletions(-) create mode 100644 npe2/manifest/_npe1_shim.py create mode 100644 tests/test_shim.py diff --git a/npe2/_plugin_manager.py b/npe2/_plugin_manager.py index 5a23ae0d..a46c801d 100644 --- a/npe2/_plugin_manager.py +++ b/npe2/_plugin_manager.py @@ -24,7 +24,8 @@ from psygnal import Signal, SignalGroup from ._command_registry import CommandRegistry -from .manifest import PluginManifest +from .manifest._npe1_shim import NPE1Shim +from .manifest.schema import PluginManifest from .manifest.writers import LayerType, WriterContribution from .types import PathLike, PythonName @@ -217,6 +218,7 @@ def __init__( self._contrib = _ContributionsIndex() self._manifests: Dict[PluginName, PluginManifest] = {} self.events = PluginManagerEvents(self) + self._shims: List[NPE1Shim] = [] # up to napari 0.4.15, discovery happened in the init here # so if we're running on an older version of napari, we need to discover @@ -269,6 +271,12 @@ def discover(self, paths: Sequence[str] = (), clear=False) -> None: if result.manifest and result.manifest.name not in self._manifests: self.register(result.manifest, warn_disabled=False) + def index_npe1_shims(self): + with warnings.catch_warnings(): + warnings.showwarning = lambda e, *_: print(str(e).split(" Please add")[0]) + while self._shims: + self._contrib.index_contributions(self._shims.pop()) + def register(self, manifest: PluginManifest, warn_disabled=True) -> None: """Register a plugin manifest""" if manifest.name in self._manifests: @@ -281,6 +289,8 @@ def register(self, manifest: PluginManifest, warn_disabled=True) -> None: f"Disabled plugin {manifest.name!r} was registered, but will not " "be indexed. Use `warn_disabled=False` to suppress this message." ) + elif isinstance(manifest, NPE1Shim): + self._shims.append(manifest) else: self._contrib.index_contributions(manifest) self.events.plugins_registered.emit({manifest}) diff --git a/npe2/manifest/_npe1_shim.py b/npe2/manifest/_npe1_shim.py new file mode 100644 index 00000000..ed618a0e --- /dev/null +++ b/npe2/manifest/_npe1_shim.py @@ -0,0 +1,38 @@ +import logging + +from .._from_npe1 import manifest_from_npe1 +from .package_metadata import PackageMetadata +from .schema import PluginManifest, discovery_blocked + +try: + from importlib import metadata +except ImportError: + import importlib_metadata as metadata # type: ignore + + +logger = logging.getLogger(__name__) + + +class NPE1Shim(PluginManifest): + _is_loaded: bool = False + _dist: metadata.Distribution + + def __init__(self, dist: metadata.Distribution): + meta = PackageMetadata.from_dist_metadata(dist.metadata) + super().__init__(name=dist.metadata["Name"], package_metadata=meta) + self._dist = dist + + def __getattribute__(self, __name: str): + if __name == "contributions" and not self._is_loaded: + self._load_contributions() + return super().__getattribute__(__name) + + def _load_contributions(self) -> None: + """imports and inspects package using npe1 plugin manager""" + + with discovery_blocked(): + mf = manifest_from_npe1(self._dist, shim=True) + self.contributions = mf.contributions + logger.debug("%r npe1 shim imported", self.name) + + self._is_loaded = True diff --git a/npe2/manifest/schema.py b/npe2/manifest/schema.py index fc56a7ad..9787b81a 100644 --- a/npe2/manifest/schema.py +++ b/npe2/manifest/schema.py @@ -6,7 +6,7 @@ from logging import getLogger from pathlib import Path from textwrap import dedent -from typing import TYPE_CHECKING, Iterator, NamedTuple, Optional, Sequence, Union +from typing import Iterator, NamedTuple, Optional, Sequence, Union from pydantic import Extra, Field, ValidationError, root_validator, validator from pydantic.error_wrappers import ErrorWrapper @@ -24,24 +24,25 @@ except ImportError: import importlib_metadata as metadata # type: ignore -if TYPE_CHECKING: - from importlib.metadata import EntryPoint - logger = getLogger(__name__) SCHEMA_VERSION = "0.1.0" ENTRY_POINT = "napari.manifest" +NPE1_ENTRY_POINT = "napari.plugin" class DiscoverResults(NamedTuple): manifest: Optional[PluginManifest] - entrypoint: Optional[EntryPoint] + distribution: Optional[metadata.Distribution] error: Optional[Exception] class PluginManifest(ImportExportModel): + class Config: + underscore_attrs_are_private = True + extra = Extra.forbid # VS Code uses . as a unique ID for the extension # should this just be the package name ... not the module name? (yes) @@ -126,8 +127,18 @@ class PluginManifest(ImportExportModel): "For normal (non-dynamic) plugins, this data will come from the package's " "setup.cfg", hide_docs=True, + exclude=True, ) + def __init__(self, **data): + super().__init__(**data) + if self.package_metadata is None and self.name: + try: + meta = metadata.distribution(self.name).metadata + self.package_metadata = PackageMetadata.from_dist_metadata(meta) + except metadata.PackageNotFoundError: + pass + def __hash__(self): return hash((self.name, self.package_version)) @@ -214,22 +225,16 @@ def from_distribution(cls, name: str) -> PluginManifest: If the manifest is not valid """ dist = metadata.distribution(name) # may raise PackageNotFoundError - for ep in dist.entry_points: - if ep.group == ENTRY_POINT: - return PluginManifest._from_entrypoint(ep, dist) - raise ValueError( - "Distribution {name!r} exists but does not provide a napari manifest" - ) - - class Config: - underscore_attrs_are_private = True - extra = Extra.forbid + pm = _from_dist(dist) + if not pm: + raise ValueError( + "Distribution {name!r} exists but does not provide a napari manifest" + ) + return pm @classmethod def discover( - cls, - entry_point_group: str = ENTRY_POINT, - paths: Sequence[Union[str, Path]] = (), + cls, paths: Sequence[Union[str, Path]] = () ) -> Iterator[DiscoverResults]: """Discover manifests in the environment. @@ -255,8 +260,6 @@ def discover( Parameters ---------- - entry_point_group : str, optional - name of entry point group to discover, by default 'napari.manifest' paths : Sequence[str], optional paths to add to sys.path while discovering. @@ -267,32 +270,30 @@ def discover( """ with _temporary_path_additions(paths): for dist in metadata.distributions(): - for ep in dist.entry_points: - if ep.group != entry_point_group: - continue - try: - pm = cls._from_entrypoint(ep, dist) - yield DiscoverResults(pm, ep, None) - except ValidationError as e: - module_name, filename = ep.value.split(":") - logger.warning( - "Invalid schema for package %r, please run" - " 'npe2 validate %s' to check for manifest errors.", - module_name, - module_name, - ) - yield DiscoverResults(None, ep, e) - except Exception as e: - logger.error( - "%s -> %r could not be imported: %s" - % (entry_point_group, ep.value, e) - ) - yield DiscoverResults(None, ep, e) + try: + pm = _from_dist(dist) + if pm: + yield DiscoverResults(pm, dist, None) + except ValidationError as e: + logger.warning( + "Invalid schema for package %r, please run" + " 'npe2 validate %s' to check for manifest errors.", + dist.metadata["Name"], + dist.metadata["Name"], + ) + yield DiscoverResults(None, dist, e) + + except Exception as e: + logger.error( + "%s -> %r could not be imported: %s" + % (ENTRY_POINT, dist.metadata["Name"], e) + ) + yield DiscoverResults(None, dist, e) @classmethod def _from_entrypoint( cls, - entry_point: EntryPoint, + entry_point: metadata.EntryPoint, distribution: Optional[metadata.Distribution] = None, ) -> PluginManifest: @@ -369,6 +370,10 @@ def _from_package_or_name( "package name or a file.." ) from e + def _serialized_data(self, **kwargs): + kwargs.setdefault("exclude", {"package_metadata"}) + return super()._serialized_data(**kwargs) + def validate_imports(self) -> None: """Checks recursively that all `python_name` fields are actually importable.""" from .utils import import_python_name @@ -397,8 +402,24 @@ def check_pynames(m: BaseModel, loc=()): ValidationError = ValidationError # for convenience of access +def _noop(*_, **__): + return [] # pragma: no cover + + +@contextmanager +def discovery_blocked(): + orig = PluginManifest.discover + setattr(PluginManifest, "discover", _noop) + try: + yield + finally: + setattr(PluginManifest, "discover", orig) + + @contextmanager def _temporary_path_additions(paths: Sequence[Union[str, Path]] = ()): + if paths and (not isinstance(paths, Sequence) or isinstance(paths, str)): + raise TypeError("paths must be a sequence of strings") # pragma: no cover for p in reversed(paths): sys.path.insert(0, str(p)) try: @@ -408,5 +429,25 @@ def _temporary_path_additions(paths: Sequence[Union[str, Path]] = ()): sys.path.remove(str(p)) +def _from_dist(dist: metadata.Distribution) -> Optional[PluginManifest]: + """Return PluginManifest or NPE1Shim for a metadata.Distribution object. + + ...depending on which entry points are available. + """ + _npe1, _npe2 = [], None + for ep in dist.entry_points: + if ep.group == NPE1_ENTRY_POINT: + _npe1.append(ep) + elif ep.group == ENTRY_POINT: + _npe2 = ep + if _npe2: + return PluginManifest._from_entrypoint(_npe2, dist) + elif _npe1: + from ._npe1_shim import NPE1Shim + + return NPE1Shim(dist=dist) + return None + + if __name__ == "__main__": print(PluginManifest.schema_json(indent=2)) diff --git a/tests/test_manifest.py b/tests/test_manifest.py index 16c9188e..ed113a81 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -39,8 +39,10 @@ def test_schema(): def test_discover(uses_sample_plugin): discover_results = list(PluginManifest.discover()) assert len(discover_results) == 1 - [(manifest, entrypoint, error)] = discover_results + [(manifest, distribution, error)] = discover_results assert manifest and manifest.name == SAMPLE_PLUGIN_NAME + assert distribution + entrypoint = tuple(distribution.entry_points)[0] assert entrypoint and entrypoint.group == "napari.manifest" == ENTRY_POINT assert entrypoint.value == f"{SAMPLE_MODULE_NAME}:napari.yaml" assert error is None @@ -83,11 +85,13 @@ def test_discover_errors(tmp_path: Path): assert len(discover_results) == 2 res_a, res_b = discover_results assert res_a.manifest is None - assert res_a.entrypoint.value == bad_value # type: ignore + assert res_a.distribution + assert tuple(res_a.distribution.entry_points)[0].value == bad_value assert "Cannot find module 'asdfsad'" in str(res_a.error) assert res_b.manifest is None - assert res_b.entrypoint.value == "module:napari.yaml" # type: ignore + assert res_b.distribution + assert tuple(res_b.distribution.entry_points)[0].value == "module:napari.yaml" assert isinstance(res_b.error, ValidationError) diff --git a/tests/test_shim.py b/tests/test_shim.py new file mode 100644 index 00000000..62c1d7bb --- /dev/null +++ b/tests/test_shim.py @@ -0,0 +1,102 @@ +from functools import partial +from unittest.mock import patch + +import numpy as np +from magicgui._magicgui import MagicFactory + +from npe2 import PluginManager +from npe2.manifest import _npe1_shim, utils +from npe2.manifest.sample_data import SampleDataGenerator + + +def test_shim_no_npe1(): + pm = PluginManager() + pm.discover() + assert not pm._shims + + +def test_npe1_shim(uses_npe1_plugin): + """Test that the plugin manager detects npe1 plugins, and can index contribs""" + pm = PluginManager() + pm.discover() + + # we've found a shim + assert len(pm._shims) == 1 + mf = pm.get_manifest("npe1-plugin") + assert isinstance(mf, _npe1_shim.NPE1Shim) + assert mf.package_metadata + assert mf.package_metadata.version == "0.1.0" + assert mf.package_metadata.name == "npe1-plugin" + + # it's currently unindexed and unstored + + with patch.object( + _npe1_shim, + "manifest_from_npe1", + wraps=_npe1_shim.manifest_from_npe1, # type: ignore + ) as mock: + pm.index_npe1_shims() + # the shim has been cleared by the indexing + assert len(pm._shims) == 0 + # manifest_from_npe1 was called + mock.assert_called_once_with(mf._dist, shim=True) + # NOTE: accessing the `.contributions` object would have also triggered + # importing, like pm.index_npe1_shims() above, but it would not have + # injected the contributions into the pm._contrib object. + assert mf.contributions.sample_data + + +def _get_mf() -> _npe1_shim.NPE1Shim: + pm = PluginManager.instance() + pm.discover() + pm.index_npe1_shims() + mf = pm.get_manifest("npe1-plugin") + assert isinstance(mf, _npe1_shim.NPE1Shim) + return mf + + +def test_shim_pyname_sample_data(uses_npe1_plugin): + """Test that objects defined locally in npe1 hookspecs can be retrieved.""" + mf = _get_mf() + samples = mf.contributions.sample_data + assert samples + sample_generator = next(s for s in samples if s.key == "local_data") + assert isinstance(sample_generator, SampleDataGenerator) + + ONES = np.ones((4, 4)) + with patch.object(utils, "_import_npe1_shim", wraps=utils._import_npe1_shim) as m: + func = sample_generator.get_callable() + assert isinstance(func, partial) # this is how it was defined in npe1-plugin + pyname = "__npe1shim__.npe1_module:napari_provide_sample_data_1" + m.assert_called_once_with(pyname) + assert np.array_equal(func(), ONES) + + # test nested sample data too + sample_generator = next(s for s in samples if s.display_name == "Some local ones") + func = sample_generator.get_callable() + assert np.array_equal(func(), ONES) + + +def test_shim_pyname_dock_widget(uses_npe1_plugin): + """Test that objects defined locally in npe1 hookspecs can be retrieved.""" + mf = _get_mf() + widgets = mf.contributions.widgets + assert widgets + wdg_contrib = next(w for w in widgets if w.display_name == "Local Widget") + + with patch.object(utils, "_import_npe1_shim", wraps=utils._import_npe1_shim) as m: + caller = wdg_contrib.get_callable() + assert isinstance(caller, MagicFactory) + assert ".local_widget" in caller.keywords["function"].__qualname__ + pyname = "__npe1shim__.npe1_module:napari_experimental_provide_dock_widget_2" + m.assert_called_once_with(pyname) + + m.reset_mock() + wdg_contrib2 = next( + w for w in widgets if w.display_name == "local function" and w.autogenerate + ) + caller2 = wdg_contrib2.get_callable() + assert isinstance(caller2, MagicFactory) + assert ".local_function" in caller2.keywords["function"].__qualname__ + pyname = "__npe1shim__.npe1_module:napari_experimental_provide_function_1" + m.assert_called_once_with(pyname) diff --git a/tests/test_utils.py b/tests/test_utils.py index f1613388..7525f404 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,6 +1,7 @@ import pytest -from npe2.manifest.utils import Version +from npe2.manifest.schema import PluginManifest +from npe2.manifest.utils import Version, deep_update, merge_manifests def test_version(): @@ -21,3 +22,52 @@ def test_version(): with pytest.raises(TypeError): Version.parse(1.2) # type: ignore + + +def test_merge_manifests(): + with pytest.raises(ValueError): + merge_manifests([]) + + with pytest.raises(AssertionError) as e: + merge_manifests([PluginManifest(name="p1"), PluginManifest(name="p2")]) + assert "All manifests must have same name" in str(e.value) + + pm1 = PluginManifest( + name="plugin", + contributions={ + "commands": [{"id": "plugin.command", "title": "some writer"}], + "writers": [{"command": "plugin.command", "layer_types": ["image"]}], + }, + ) + pm2 = PluginManifest( + name="plugin", + contributions={ + "commands": [{"id": "plugin.command", "title": "some reader"}], + "readers": [{"command": "plugin.command", "filename_patterns": [".tif"]}], + }, + ) + expected_merge = PluginManifest( + name="plugin", + contributions={ + "commands": [ + {"id": "plugin.command", "title": "some writer"}, + {"id": "plugin.command_2", "title": "some reader"}, # no dupes + ], + "writers": [{"command": "plugin.command", "layer_types": ["image"]}], + "readers": [{"command": "plugin.command_2", "filename_patterns": [".tif"]}], + }, + ) + + assert merge_manifests([pm1]) is pm1 + assert merge_manifests([pm1, pm2]) == expected_merge + + +def test_deep_update(): + a = {"a": {"b": 1, "c": 2}, "e": 2} + b = {"a": {"d": 4, "c": 3}, "f": 0} + c = deep_update(a, b, copy=True) + assert c == {"a": {"b": 1, "d": 4, "c": 3}, "e": 2, "f": 0} + assert a == {"a": {"b": 1, "c": 2}, "e": 2} + + deep_update(a, b, copy=False) + assert a == {"a": {"b": 1, "d": 4, "c": 3}, "e": 2, "f": 0} diff --git a/tests/test_validations.py b/tests/test_validations.py index 66d8b56d..c2413f1f 100644 --- a/tests/test_validations.py +++ b/tests/test_validations.py @@ -36,6 +36,12 @@ def _mutator_python_name_no_colon(data): data["contributions"]["commands"][0]["python_name"] = "this.has.no.colon" +def _mutator_python_name_locals(data): + """functions defined in local scopes are not yet supported""" + assert "contributions" in data + data["contributions"]["commands"][0]["python_name"] = "mod:func..another" + + def _mutator_python_name_starts_with_number(data): """'1starts_with_number' is not a valid python_name.""" assert "contributions" in data @@ -81,6 +87,7 @@ def _mutator_schema_version_too_high(data): _mutator_invalid_package_name2, _mutator_command_not_begin_with_package_name, _mutator_python_name_no_colon, + _mutator_python_name_locals, _mutator_python_name_starts_with_number, _mutator_no_contributes_extra_field, _mutator_writer_requires_non_empty_layer_types, From d41fea538483ad4873046728f0d4e1446c0d9210 Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Sun, 13 Mar 2022 12:40:31 -0400 Subject: [PATCH 03/13] add tests --- tests/test_utils.py | 52 ++++++++++++++++++++++++++++++++++++++- tests/test_validations.py | 7 ++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index f1613388..7525f404 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,6 +1,7 @@ import pytest -from npe2.manifest.utils import Version +from npe2.manifest.schema import PluginManifest +from npe2.manifest.utils import Version, deep_update, merge_manifests def test_version(): @@ -21,3 +22,52 @@ def test_version(): with pytest.raises(TypeError): Version.parse(1.2) # type: ignore + + +def test_merge_manifests(): + with pytest.raises(ValueError): + merge_manifests([]) + + with pytest.raises(AssertionError) as e: + merge_manifests([PluginManifest(name="p1"), PluginManifest(name="p2")]) + assert "All manifests must have same name" in str(e.value) + + pm1 = PluginManifest( + name="plugin", + contributions={ + "commands": [{"id": "plugin.command", "title": "some writer"}], + "writers": [{"command": "plugin.command", "layer_types": ["image"]}], + }, + ) + pm2 = PluginManifest( + name="plugin", + contributions={ + "commands": [{"id": "plugin.command", "title": "some reader"}], + "readers": [{"command": "plugin.command", "filename_patterns": [".tif"]}], + }, + ) + expected_merge = PluginManifest( + name="plugin", + contributions={ + "commands": [ + {"id": "plugin.command", "title": "some writer"}, + {"id": "plugin.command_2", "title": "some reader"}, # no dupes + ], + "writers": [{"command": "plugin.command", "layer_types": ["image"]}], + "readers": [{"command": "plugin.command_2", "filename_patterns": [".tif"]}], + }, + ) + + assert merge_manifests([pm1]) is pm1 + assert merge_manifests([pm1, pm2]) == expected_merge + + +def test_deep_update(): + a = {"a": {"b": 1, "c": 2}, "e": 2} + b = {"a": {"d": 4, "c": 3}, "f": 0} + c = deep_update(a, b, copy=True) + assert c == {"a": {"b": 1, "d": 4, "c": 3}, "e": 2, "f": 0} + assert a == {"a": {"b": 1, "c": 2}, "e": 2} + + deep_update(a, b, copy=False) + assert a == {"a": {"b": 1, "d": 4, "c": 3}, "e": 2, "f": 0} diff --git a/tests/test_validations.py b/tests/test_validations.py index 66d8b56d..c2413f1f 100644 --- a/tests/test_validations.py +++ b/tests/test_validations.py @@ -36,6 +36,12 @@ def _mutator_python_name_no_colon(data): data["contributions"]["commands"][0]["python_name"] = "this.has.no.colon" +def _mutator_python_name_locals(data): + """functions defined in local scopes are not yet supported""" + assert "contributions" in data + data["contributions"]["commands"][0]["python_name"] = "mod:func..another" + + def _mutator_python_name_starts_with_number(data): """'1starts_with_number' is not a valid python_name.""" assert "contributions" in data @@ -81,6 +87,7 @@ def _mutator_schema_version_too_high(data): _mutator_invalid_package_name2, _mutator_command_not_begin_with_package_name, _mutator_python_name_no_colon, + _mutator_python_name_locals, _mutator_python_name_starts_with_number, _mutator_no_contributes_extra_field, _mutator_writer_requires_non_empty_layer_types, From 3115e32238a95341bfde50cc659a56a057e60256 Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Sun, 13 Mar 2022 12:42:45 -0400 Subject: [PATCH 04/13] shim3 add caching --- npe2/_command_registry.py | 4 +- npe2/cli.py | 57 ++++++++++++++++++++++++- npe2/manifest/_npe1_shim.py | 68 ++++++++++++++++++++++++++++++ tests/conftest.py | 8 ++++ tests/sample/my_plugin/napari.yaml | 1 - tests/test_cli.py | 41 ++++++++++++++++++ tests/test_shim.py | 54 ++++++++++++++++++++++-- 7 files changed, 225 insertions(+), 8 deletions(-) diff --git a/npe2/_command_registry.py b/npe2/_command_registry.py index dd90414a..4b2eb6bd 100644 --- a/npe2/_command_registry.py +++ b/npe2/_command_registry.py @@ -7,8 +7,8 @@ from psygnal import Signal +from .manifest import utils from .manifest._validators import DOTTED_NAME_PATTERN -from .manifest.utils import import_python_name from .types import PythonName PDisposable = Callable[[], None] @@ -27,7 +27,7 @@ def resolve(self) -> Callable: raise RuntimeError("cannot resolve command without python_name") try: - self.function = import_python_name(self.python_name) + self.function = utils.import_python_name(self.python_name) except Exception as e: raise RuntimeError(f"Failed to import command at {self.python_name!r}: {e}") return self.function diff --git a/npe2/cli.py b/npe2/cli.py index b7360f08..033ead90 100644 --- a/npe2/cli.py +++ b/npe2/cli.py @@ -1,12 +1,18 @@ +import builtins import warnings from pathlib import Path from textwrap import indent -from typing import Optional +from typing import List, Optional import typer from npe2 import PluginManifest +try: + from importlib.metadata import distribution +except ImportError: + from importlib_metadata import distribution # type: ignore + app = typer.Typer() @@ -138,5 +144,54 @@ def convert( ) +@app.command() +def cache( + clear: Optional[bool] = typer.Option( + False, "--clear", "-d", help="Clear the npe1 shim manifest cache" + ), + names: List[str] = typer.Argument( + None, help="Name(s) of distributions to list/delete" + ), + list_: Optional[bool] = typer.Option( + False, "--list", "-l", help="List cached manifests" + ), + verbose: Optional[bool] = typer.Option(False, "--verbose", "-v", help="verbose"), +): + """Cache utils""" + from npe2.manifest._npe1_shim import SHIM_CACHE, clear_cache + + if clear: + _cleared = clear_cache(names) + if _cleared: + nf = "\n".join(f" - {i.name}" for i in _cleared) + typer.secho("Cleared these files from cache:") + typer.secho(nf, fg=typer.colors.RED) + else: + msg = "Nothing to clear" + if names: + msg += f" for plugins: {','.join(names)}" + typer.secho(msg, fg=typer.colors.RED) + + typer.Exit() + if list_: + files = builtins.list(SHIM_CACHE.glob("*.yaml")) + if names: + files = [f for f in files if any(f.name.startswith(n) for n in names)] + + if not files: + if names: + typer.secho(f"Nothing cached for plugins: {','.join(names)}") + else: + typer.secho("Nothing cached") + typer.Exit() + for fname in files: + mf = PluginManifest.from_file(fname) + if verbose: + _pprint_yaml(mf.yaml()) # pragma: no cover + else: + d = distribution(mf.name) + typer.secho(f"{mf.name}: {d.version}", fg=typer.colors.GREEN) + + def main(): app() diff --git a/npe2/manifest/_npe1_shim.py b/npe2/manifest/_npe1_shim.py index ed618a0e..9f86852d 100644 --- a/npe2/manifest/_npe1_shim.py +++ b/npe2/manifest/_npe1_shim.py @@ -1,4 +1,11 @@ import logging +import os +import site +from pathlib import Path +from shutil import rmtree +from typing import List, Sequence + +from appdirs import user_cache_dir from .._from_npe1 import manifest_from_npe1 from .package_metadata import PackageMetadata @@ -11,6 +18,34 @@ logger = logging.getLogger(__name__) +SHIM_CACHE = Path(user_cache_dir("napari", "napari")) / "npe2" / "shims" +NPE2_NOCACHE = "NPE2_NOCACHE" + + +def clear_cache(names: Sequence[str] = ()) -> List[Path]: + """Clear cached npe1 shim files + + Parameters + ---------- + names : Sequence[str], optional + selection of plugin names to clear, by default, all will be cleared + + Returns + ------- + List[Path] + List of filepaths cleared + """ + _cleared: List[Path] = [] + if SHIM_CACHE.exists(): + if names: + for f in SHIM_CACHE.glob("*.yaml"): + if any(f.name.startswith(f"{n}_") for n in names): + f.unlink() + _cleared.append(f) + else: + _cleared = list(SHIM_CACHE.iterdir()) + rmtree(SHIM_CACHE) + return _cleared class NPE1Shim(PluginManifest): @@ -30,9 +65,42 @@ def __getattribute__(self, __name: str): def _load_contributions(self) -> None: """imports and inspects package using npe1 plugin manager""" + if self._cache_path().exists() and not os.getenv(NPE2_NOCACHE): + mf = PluginManifest.from_file(self._cache_path()) + self.contributions = mf.contributions + self._is_loaded = True + logger.debug("%r npe1 shim loaded from cache", self.name) + return + with discovery_blocked(): mf = manifest_from_npe1(self._dist, shim=True) self.contributions = mf.contributions logger.debug("%r npe1 shim imported", self.name) self._is_loaded = True + if not _is_editable_install(self._dist): + self._save_to_cache() + + def _save_to_cache(self): + cache_path = self._cache_path() + cache_path.parent.mkdir(exist_ok=True, parents=True) + cache_path.write_text(self.yaml()) + + def _cache_path(self) -> Path: + """Return cache path for manifest corresponding to distribution.""" + return _cached_shim_path(self.name, self.package_version or "") + + +def _cached_shim_path(name: str, version: str) -> Path: + """Return cache path for manifest corresponding to distribution.""" + return SHIM_CACHE / f"{name}_{version}.yaml" + + +def _is_editable_install(dist: metadata.Distribution) -> bool: + """Return True if dist is installed as editable. + + i.e: if the package isn't in site-packages or user site-packages. + """ + root = str(dist.locate_file("")) + installed_paths = site.getsitepackages() + [site.getusersitepackages()] + return all(loc not in root for loc in installed_paths) diff --git a/tests/conftest.py b/tests/conftest.py index 8cf39683..5654ca87 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,6 +6,7 @@ import pytest from npe2 import PluginManager, PluginManifest +from npe2.manifest import _npe1_shim try: from importlib import metadata @@ -164,3 +165,10 @@ def _from_name(name): new_manifest.unlink() if (npe1_repo / "setup.py").exists(): (npe1_repo / "setup.py").unlink() + + +@pytest.fixture +def mock_cache(tmp_path, monkeypatch): + with monkeypatch.context() as m: + m.setattr(_npe1_shim, "SHIM_CACHE", tmp_path) + yield tmp_path diff --git a/tests/sample/my_plugin/napari.yaml b/tests/sample/my_plugin/napari.yaml index 54e9584f..9a6bc15f 100644 --- a/tests/sample/my_plugin/napari.yaml +++ b/tests/sample/my_plugin/napari.yaml @@ -66,7 +66,6 @@ contributions: - command: my-plugin.hello_world mysubmenu: - command: my-plugin.another_command - - command: my-plugin.affinder submenus: - id: mysubmenu label: My SubMenu diff --git a/tests/test_cli.py b/tests/test_cli.py index 6c9934aa..29877048 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -86,3 +86,44 @@ def test_cli_main(monkeypatch, sample_path): with pytest.raises(SystemExit) as e: main() assert e.value.code == 0 + + +def test_cli_cache_list_empty(mock_cache): + result = runner.invoke(app, ["cache", "--list"]) + assert "Nothing cached" in result.stdout + assert result.exit_code == 0 + + +def test_cli_cache_list_full(uses_npe1_plugin, mock_cache): + (mock_cache / "npe1-plugin.yaml").write_text("name: npe1-plugin\n") + result = runner.invoke(app, ["cache", "--list"]) + assert result.stdout == "npe1-plugin: 0.1.0\n" + assert result.exit_code == 0 + + +def test_cli_cache_list_named(uses_npe1_plugin, mock_cache): + (mock_cache / "npe1-plugin.yaml").write_text("name: npe1-plugin\n") + result = runner.invoke(app, ["cache", "--list", "not-a-plugin"]) + assert result.stdout == "Nothing cached for plugins: not-a-plugin\n" + assert result.exit_code == 0 + + +def test_cli_cache_clear_empty(mock_cache): + result = runner.invoke(app, ["cache", "--clear"]) + assert "Nothing to clear" in result.stdout + assert result.exit_code == 0 + + +def test_cli_cache_clear_full(mock_cache): + (mock_cache / "npe1-plugin.yaml").write_text("name: npe1-plugin\n") + result = runner.invoke(app, ["cache", "--clear"]) + assert "Cleared these files from cache" in result.stdout + assert "- npe1-plugin.yaml" in result.stdout + assert result.exit_code == 0 + + +def test_cli_cache_clear_named(mock_cache): + (mock_cache / "npe1-plugin.yaml").write_text("name: npe1-plugin\n") + result = runner.invoke(app, ["cache", "--clear", "not-a-plugin"]) + assert result.stdout == "Nothing to clear for plugins: not-a-plugin\n" + assert result.exit_code == 0 diff --git a/tests/test_shim.py b/tests/test_shim.py index 62c1d7bb..eec883dc 100644 --- a/tests/test_shim.py +++ b/tests/test_shim.py @@ -1,4 +1,5 @@ from functools import partial +from pathlib import Path from unittest.mock import patch import numpy as np @@ -15,7 +16,7 @@ def test_shim_no_npe1(): assert not pm._shims -def test_npe1_shim(uses_npe1_plugin): +def test_npe1_shim(uses_npe1_plugin, mock_cache: Path): """Test that the plugin manager detects npe1 plugins, and can index contribs""" pm = PluginManager() pm.discover() @@ -29,6 +30,8 @@ def test_npe1_shim(uses_npe1_plugin): assert mf.package_metadata.name == "npe1-plugin" # it's currently unindexed and unstored + assert not mf._cache_path().exists() + assert not list(mock_cache.iterdir()) with patch.object( _npe1_shim, @@ -38,13 +41,56 @@ def test_npe1_shim(uses_npe1_plugin): pm.index_npe1_shims() # the shim has been cleared by the indexing assert len(pm._shims) == 0 - # manifest_from_npe1 was called + # manifest_from_npe1 was called and the result was cached mock.assert_called_once_with(mf._dist, shim=True) + assert mf._cache_path().exists() # NOTE: accessing the `.contributions` object would have also triggered # importing, like pm.index_npe1_shims() above, but it would not have # injected the contributions into the pm._contrib object. assert mf.contributions.sample_data + mock.reset_mock() + # clear and rediscover... this time we expect the cache to kick in + pm.discover(clear=True) + assert len(pm._shims) == 1 + pm.index_npe1_shims() + assert len(pm._shims) == 0 + mock.assert_not_called() + + +def test_npe1_shim_cache(uses_npe1_plugin, mock_cache: Path): + """Test that we can clear cache, etc..""" + pm = PluginManager() + pm.discover() + + with patch.object( + _npe1_shim, + "manifest_from_npe1", + wraps=_npe1_shim.manifest_from_npe1, # type: ignore + ) as mock: + + # if we clear the cache, it should import again + mf = pm.get_manifest("npe1-plugin") + assert isinstance(mf, _npe1_shim.NPE1Shim) + pm.index_npe1_shims() + mock.assert_called_once_with(mf._dist, shim=True) + assert mf._cache_path().exists() + + _npe1_shim.clear_cache() + assert not mf._cache_path().exists() + + mock.reset_mock() + pm.discover(clear=True) + pm.index_npe1_shims() + mf = pm.get_manifest("npe1-plugin") + assert isinstance(mf, _npe1_shim.NPE1Shim) + mock.assert_called_once_with(mf._dist, shim=True) + assert mf._cache_path().exists() + _npe1_shim.clear_cache(names=["not-our-plugin"]) + assert mf._cache_path().exists() + _npe1_shim.clear_cache(names=["npe1-plugin"]) + assert not mf._cache_path().exists() + def _get_mf() -> _npe1_shim.NPE1Shim: pm = PluginManager.instance() @@ -55,7 +101,7 @@ def _get_mf() -> _npe1_shim.NPE1Shim: return mf -def test_shim_pyname_sample_data(uses_npe1_plugin): +def test_shim_pyname_sample_data(uses_npe1_plugin, mock_cache): """Test that objects defined locally in npe1 hookspecs can be retrieved.""" mf = _get_mf() samples = mf.contributions.sample_data @@ -77,7 +123,7 @@ def test_shim_pyname_sample_data(uses_npe1_plugin): assert np.array_equal(func(), ONES) -def test_shim_pyname_dock_widget(uses_npe1_plugin): +def test_shim_pyname_dock_widget(uses_npe1_plugin, mock_cache): """Test that objects defined locally in npe1 hookspecs can be retrieved.""" mf = _get_mf() widgets = mf.contributions.widgets From d6365c464a4ec806f3522ee2cfd50d3dd7060962 Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Mon, 14 Mar 2022 09:04:29 -0400 Subject: [PATCH 05/13] add test for failed import --- npe2/manifest/_npe1_shim.py | 13 ++++++++++--- tests/test_shim.py | 26 ++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/npe2/manifest/_npe1_shim.py b/npe2/manifest/_npe1_shim.py index ed618a0e..4e51a6eb 100644 --- a/npe2/manifest/_npe1_shim.py +++ b/npe2/manifest/_npe1_shim.py @@ -1,4 +1,5 @@ import logging +import warnings from .._from_npe1 import manifest_from_npe1 from .package_metadata import PackageMetadata @@ -31,8 +32,14 @@ def _load_contributions(self) -> None: """imports and inspects package using npe1 plugin manager""" with discovery_blocked(): - mf = manifest_from_npe1(self._dist, shim=True) + self._is_loaded = True # if we fail once, we still don't try again. + try: + mf = manifest_from_npe1(self._dist, shim=True) + except Exception as e: + warnings.warn( + f"Failed to detect contributions for np1e plugin {self.name!r}: {e}" + ) + return + self.contributions = mf.contributions logger.debug("%r npe1 shim imported", self.name) - - self._is_loaded = True diff --git a/tests/test_shim.py b/tests/test_shim.py index 62c1d7bb..54f7fe02 100644 --- a/tests/test_shim.py +++ b/tests/test_shim.py @@ -2,12 +2,18 @@ from unittest.mock import patch import numpy as np +import pytest from magicgui._magicgui import MagicFactory from npe2 import PluginManager from npe2.manifest import _npe1_shim, utils from npe2.manifest.sample_data import SampleDataGenerator +try: + from importlib import metadata +except ImportError: + import importlib_metadata as metadata # type: ignore + def test_shim_no_npe1(): pm = PluginManager() @@ -100,3 +106,23 @@ def test_shim_pyname_dock_widget(uses_npe1_plugin): assert ".local_function" in caller2.keywords["function"].__qualname__ pyname = "__npe1shim__.npe1_module:napari_experimental_provide_function_1" m.assert_called_once_with(pyname) + + +def test_shim_error_on_import(): + class FakeDist(metadata.Distribution): + def read_text(self, filename): + if filename == "METADATA": + return "Name: fake-plugin\nVersion: 0.1.0\n" + + def locate_file(self, *_): + ... + + shim = _npe1_shim.NPE1Shim(FakeDist()) + + def err(): + raise ImportError("No package found.") + + with pytest.warns(UserWarning) as record: + with patch.object(_npe1_shim, "manifest_from_npe1", wraps=err): + shim.contributions + assert "Failed to detect contributions" in str(record[0]) From 909f8c3ee973470383c509f9025ab626abb459e3 Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Mon, 14 Mar 2022 09:28:00 -0400 Subject: [PATCH 06/13] add appdirs --- setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.cfg b/setup.cfg index 106bc2e2..d7ba2cd6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -25,6 +25,7 @@ project_urls = packages = find: install_requires = PyYAML + appdirs magicgui>=0.3.3 napari-plugin-engine psygnal>=0.3.0 From 6bcf9dc5491de63d5d4996bf704896e8a587ce22 Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Mon, 14 Mar 2022 09:29:38 -0400 Subject: [PATCH 07/13] remove napari-plugin-engine --- setup.cfg | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index d7ba2cd6..1468650b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -27,7 +27,6 @@ install_requires = PyYAML appdirs magicgui>=0.3.3 - napari-plugin-engine psygnal>=0.3.0 pydantic pytomlpp From 99bf394509ee8d635ad6ccccbc98327efddb676f Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Wed, 16 Mar 2022 14:08:11 -0400 Subject: [PATCH 08/13] add magic factory finding --- npe2/_from_npe1.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/npe2/_from_npe1.py b/npe2/_from_npe1.py index 68da7a88..0ac70439 100644 --- a/npe2/_from_npe1.py +++ b/npe2/_from_npe1.py @@ -22,6 +22,8 @@ cast, ) +import magicgui + from npe2.manifest import PluginManifest from npe2.manifest.commands import CommandContribution from npe2.manifest.themes import ThemeColors @@ -465,6 +467,15 @@ def _python_name( mod_name = hook_mod.__name__ break + # trick if it's a magic_factory + if isinstance(obj, magicgui._magicgui.MagicFactory): + f = obj.keywords.get("function") + if f: + v = getattr(f, "__globals__", {}).get(getattr(f, "__name__", "")) + if v is obj: + mod_name = f.__module__ + obj_name = f.__qualname__ + # if that didn't work get the qualname of the object # and, if it's not a locally defined qualname, get the name of the module # in which it is defined From 667ee5e115da5ea8ed564891254ca4b9e50c40dd Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Wed, 16 Mar 2022 14:09:04 -0400 Subject: [PATCH 09/13] add magicfactory --- npe2/_from_npe1.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/npe2/_from_npe1.py b/npe2/_from_npe1.py index 68da7a88..0ac70439 100644 --- a/npe2/_from_npe1.py +++ b/npe2/_from_npe1.py @@ -22,6 +22,8 @@ cast, ) +import magicgui + from npe2.manifest import PluginManifest from npe2.manifest.commands import CommandContribution from npe2.manifest.themes import ThemeColors @@ -465,6 +467,15 @@ def _python_name( mod_name = hook_mod.__name__ break + # trick if it's a magic_factory + if isinstance(obj, magicgui._magicgui.MagicFactory): + f = obj.keywords.get("function") + if f: + v = getattr(f, "__globals__", {}).get(getattr(f, "__name__", "")) + if v is obj: + mod_name = f.__module__ + obj_name = f.__qualname__ + # if that didn't work get the qualname of the object # and, if it's not a locally defined qualname, get the name of the module # in which it is defined From 950634fbd5f17e105c956ae38acc999ebe805890 Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Wed, 23 Mar 2022 17:37:01 -0400 Subject: [PATCH 10/13] rename to adapter --- npe2/_from_npe1.py | 7 +++++++ npe2/_plugin_manager.py | 6 +++--- .../{_npe1_shim.py => _npe1_adapter.py} | 7 ++++--- npe2/manifest/schema.py | 4 ++-- tests/{test_shim.py => test_npe1_adapter.py} | 18 +++++++++--------- 5 files changed, 25 insertions(+), 17 deletions(-) rename npe2/manifest/{_npe1_shim.py => _npe1_adapter.py} (85%) rename tests/{test_shim.py => test_npe1_adapter.py} (89%) diff --git a/npe2/_from_npe1.py b/npe2/_from_npe1.py index 0ac70439..0480e6a6 100644 --- a/npe2/_from_npe1.py +++ b/npe2/_from_npe1.py @@ -7,6 +7,7 @@ from dataclasses import dataclass from functools import lru_cache from importlib import import_module +from logging import getLogger from pathlib import Path from types import ModuleType from typing import ( @@ -36,6 +37,7 @@ except ImportError: import importlib_metadata as metadata # type: ignore +logger = getLogger(__name__) NPE1_EP = "napari.plugin" NPE2_EP = "napari.manifest" NPE1_IMPL_TAG = "napari_impl" # same as HookImplementation.format_tag("napari") @@ -163,6 +165,11 @@ def manifest_from_npe1( manifests: List[PluginManifest] = [] for mod_name in modules: + logger.debug( + "Discovering contributions for npe1 plugin %r: module %r", + package_name, + mod_name, + ) parser = HookImplParser(package_name, plugin_name or "", shim=shim) _mod = import_module(mod_name) if isinstance(mod_name, str) else mod_name parser.parse_module(_mod) diff --git a/npe2/_plugin_manager.py b/npe2/_plugin_manager.py index ee39220d..2ef2043d 100644 --- a/npe2/_plugin_manager.py +++ b/npe2/_plugin_manager.py @@ -24,7 +24,7 @@ from psygnal import Signal, SignalGroup from ._command_registry import CommandRegistry -from .manifest._npe1_shim import NPE1Shim +from .manifest._npe1_adapter import NPE1Adapter from .manifest.schema import PluginManifest from .manifest.writers import LayerType, WriterContribution from .types import PathLike, PythonName, _ensure_str_or_seq_str @@ -225,7 +225,7 @@ def __init__( self._contrib = _ContributionsIndex() self._manifests: Dict[PluginName, PluginManifest] = {} self.events = PluginManagerEvents(self) - self._shims: List[NPE1Shim] = [] + self._shims: List[NPE1Adapter] = [] # up to napari 0.4.15, discovery happened in the init here # so if we're running on an older version of napari, we need to discover @@ -296,7 +296,7 @@ def register(self, manifest: PluginManifest, warn_disabled=True) -> None: f"Disabled plugin {manifest.name!r} was registered, but will not " "be indexed. Use `warn_disabled=False` to suppress this message." ) - elif isinstance(manifest, NPE1Shim): + elif isinstance(manifest, NPE1Adapter): self._shims.append(manifest) else: self._contrib.index_contributions(manifest) diff --git a/npe2/manifest/_npe1_shim.py b/npe2/manifest/_npe1_adapter.py similarity index 85% rename from npe2/manifest/_npe1_shim.py rename to npe2/manifest/_npe1_adapter.py index 4e51a6eb..b23c4fd4 100644 --- a/npe2/manifest/_npe1_shim.py +++ b/npe2/manifest/_npe1_adapter.py @@ -14,7 +14,7 @@ logger = logging.getLogger(__name__) -class NPE1Shim(PluginManifest): +class NPE1Adapter(PluginManifest): _is_loaded: bool = False _dist: metadata.Distribution @@ -29,7 +29,7 @@ def __getattribute__(self, __name: str): return super().__getattribute__(__name) def _load_contributions(self) -> None: - """imports and inspects package using npe1 plugin manager""" + """import and inspect package contributions.""" with discovery_blocked(): self._is_loaded = True # if we fail once, we still don't try again. @@ -37,7 +37,8 @@ def _load_contributions(self) -> None: mf = manifest_from_npe1(self._dist, shim=True) except Exception as e: warnings.warn( - f"Failed to detect contributions for np1e plugin {self.name!r}: {e}" + "Error importing contributions for first-generation " + f"napari plugin {self.name!r}: {e}" ) return diff --git a/npe2/manifest/schema.py b/npe2/manifest/schema.py index 2dc68205..03d47737 100644 --- a/npe2/manifest/schema.py +++ b/npe2/manifest/schema.py @@ -447,9 +447,9 @@ def _from_dist(dist: metadata.Distribution) -> Optional[PluginManifest]: if _npe2: return PluginManifest._from_entrypoint(_npe2, dist) elif _npe1: - from ._npe1_shim import NPE1Shim + from ._npe1_adapter import NPE1Adapter - return NPE1Shim(dist=dist) + return NPE1Adapter(dist=dist) return None diff --git a/tests/test_shim.py b/tests/test_npe1_adapter.py similarity index 89% rename from tests/test_shim.py rename to tests/test_npe1_adapter.py index 54f7fe02..6c6b5de5 100644 --- a/tests/test_shim.py +++ b/tests/test_npe1_adapter.py @@ -6,7 +6,7 @@ from magicgui._magicgui import MagicFactory from npe2 import PluginManager -from npe2.manifest import _npe1_shim, utils +from npe2.manifest import _npe1_adapter, utils from npe2.manifest.sample_data import SampleDataGenerator try: @@ -29,7 +29,7 @@ def test_npe1_shim(uses_npe1_plugin): # we've found a shim assert len(pm._shims) == 1 mf = pm.get_manifest("npe1-plugin") - assert isinstance(mf, _npe1_shim.NPE1Shim) + assert isinstance(mf, _npe1_adapter.NPE1Adapter) assert mf.package_metadata assert mf.package_metadata.version == "0.1.0" assert mf.package_metadata.name == "npe1-plugin" @@ -37,9 +37,9 @@ def test_npe1_shim(uses_npe1_plugin): # it's currently unindexed and unstored with patch.object( - _npe1_shim, + _npe1_adapter, "manifest_from_npe1", - wraps=_npe1_shim.manifest_from_npe1, # type: ignore + wraps=_npe1_adapter.manifest_from_npe1, # type: ignore ) as mock: pm.index_npe1_shims() # the shim has been cleared by the indexing @@ -52,12 +52,12 @@ def test_npe1_shim(uses_npe1_plugin): assert mf.contributions.sample_data -def _get_mf() -> _npe1_shim.NPE1Shim: +def _get_mf() -> _npe1_adapter.NPE1Adapter: pm = PluginManager.instance() pm.discover() pm.index_npe1_shims() mf = pm.get_manifest("npe1-plugin") - assert isinstance(mf, _npe1_shim.NPE1Shim) + assert isinstance(mf, _npe1_adapter.NPE1Adapter) return mf @@ -117,12 +117,12 @@ def read_text(self, filename): def locate_file(self, *_): ... - shim = _npe1_shim.NPE1Shim(FakeDist()) + shim = _npe1_adapter.NPE1Adapter(FakeDist()) def err(): raise ImportError("No package found.") with pytest.warns(UserWarning) as record: - with patch.object(_npe1_shim, "manifest_from_npe1", wraps=err): + with patch.object(_npe1_adapter, "manifest_from_npe1", wraps=err): shim.contributions - assert "Failed to detect contributions" in str(record[0]) + assert "Error importing contributions for" in str(record[0]) From f739e16e6178b1f267e9775c4cfdc846f588de6e Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Wed, 23 Mar 2022 17:59:15 -0400 Subject: [PATCH 11/13] more renaming and docs --- npe2/_from_npe1.py | 42 ++++++++++++++++++---------------- npe2/_plugin_manager.py | 10 ++++---- npe2/manifest/_npe1_adapter.py | 29 +++++++++++++++++++++-- npe2/manifest/schema.py | 2 +- tests/test_npe1_adapter.py | 37 ++++++++++++++++-------------- 5 files changed, 75 insertions(+), 45 deletions(-) diff --git a/npe2/_from_npe1.py b/npe2/_from_npe1.py index 0480e6a6..20f084dc 100644 --- a/npe2/_from_npe1.py +++ b/npe2/_from_npe1.py @@ -111,7 +111,7 @@ def plugin_packages() -> List[PluginPackage]: def manifest_from_npe1( plugin: Union[str, metadata.Distribution, None] = None, module: Any = None, - shim=False, + adapter=False, ) -> PluginManifest: """Return manifest object given npe1 plugin or package name. @@ -125,8 +125,8 @@ def manifest_from_npe1( package, and the name of an npe1 `napari.plugin` entry_point. by default None module : Optional[Module] namespace object, to directly import (mostly for testing.), by default None - shim : bool - If True, the resulting manifest will be used internally by NPE1Adaptor, but + adapter : bool + If True, the resulting manifest will be used internally by NPE1Adapter, but is NOT necessarily suitable for export as npe2 manifest. This will handle cases of locally defined functions and partials that don't have global python_names that are not supported natively by npe2. by default False @@ -170,7 +170,7 @@ def manifest_from_npe1( package_name, mod_name, ) - parser = HookImplParser(package_name, plugin_name or "", shim=shim) + parser = HookImplParser(package_name, plugin_name or "", adapter=adapter) _mod = import_module(mod_name) if isinstance(mod_name, str) else mod_name parser.parse_module(_mod) manifests.append(parser.manifest()) @@ -180,31 +180,31 @@ def manifest_from_npe1( class HookImplParser: - def __init__(self, package: str, plugin_name: str, shim: bool = False) -> None: + def __init__(self, package: str, plugin_name: str, adapter: bool = False) -> None: """A visitor class to convert npe1 hookimpls to a npe2 manifest Parameters ---------- package : str - [description] + Name of package plugin_name : str - [description] - shim : bool, optional - If True, the resulting manifest will be used internally by NPE1Adaptor, but + Name of plugin (will almost always be name of package) + adapter : bool, optional + If True, the resulting manifest will be used internally by NPE1Adapter, but is NOT necessarily suitable for export as npe2 manifest. This will handle cases of locally defined functions and partials that don't have global python_names that are not supported natively by npe2. by default False Examples -------- - >>> parser = HookImplParser(package, plugin_name, shim=shim) + >>> parser = HookImplParser(package, plugin_name) >>> parser.parse_callers(plugin_manager._plugin2hookcallers[_module]) >>> mf = PluginManifest(name=package, contributions=dict(parser.contributions)) """ self.package = package self.plugin_name = plugin_name self.contributions: DefaultDict[str, list] = DefaultDict(list) - self.shim = shim + self.adapter = adapter def manifest(self) -> PluginManifest: return PluginManifest(name=self.package, contributions=dict(self.contributions)) @@ -267,7 +267,7 @@ def napari_provide_sample_data(self, impl: HookImplementation): # let these raise exceptions here immediately if they don't validate id = f"{self.package}.data.{_key}" py_name = _python_name( - _sample, impl.function, shim_idx=idx if self.shim else None + _sample, impl.function, hook_idx=idx if self.adapter else None ) cmd_contrib = CommandContribution( id=id, @@ -292,7 +292,7 @@ def napari_experimental_provide_function(self, impl: HookImplementation): cmd = f"{self.package}.{item.__name__}" py_name = _python_name( - item, impl.function, shim_idx=idx if self.shim else None + item, impl.function, hook_idx=idx if self.adapter else None ) docsum = item.__doc__.splitlines()[0] if item.__doc__ else None cmd_contrib = CommandContribution( @@ -358,7 +358,9 @@ def _create_widget_contrib( # returned it... In the case that we can't get an absolute python name to the # wdg_creator itself (e.g. it's defined in a local scope), then the py_name # will use the hookimpl itself, and the index of the object returned. - py_name = _python_name(wdg_creator, hook, shim_idx=idx if self.shim else None) + py_name = _python_name( + wdg_creator, hook, hook_idx=idx if self.adapter else None + ) if not py_name: # pragma: no cover raise ValueError( @@ -434,7 +436,7 @@ def _safe_key(key: str) -> str: def _python_name( - obj: Any, hook: Callable = None, shim_idx: Optional[int] = None + obj: Any, hook: Callable = None, hook_idx: Optional[int] = None ) -> str: """Get resolvable python name for `obj` returned from an npe1 `hook` implentation. @@ -446,9 +448,9 @@ def _python_name( the npe1 hook implementation that returned `obj`, by default None. This is used both to search the module namespace for `obj`, and also in the shim python name if `obj` cannot be found. - shim_idx : int, optional - If `obj` cannot be found and `shim_idx` is not None, then a shim name. - of the form "__npe1shim__.{_python_name(hook)}_{shim_idx}" will be returned. + hook_idx : int, optional + If `obj` cannot be found and `hook_idx` is not None, then a shim name. + of the form "__npe1shim__.{_python_name(hook)}_{hook_idx}" will be returned. by default None. Returns @@ -493,10 +495,10 @@ def _python_name( if mod: mod_name = mod.__name__ - if not (mod_name and obj_name) and (hook and shim_idx is not None): + if not (mod_name and obj_name) and (hook and hook_idx is not None): # we weren't able to resolve an absolute name... if we are shimming, then we # can create a special py_name of the form `__npe1shim__.hookfunction_idx` - return f"{SHIM_NAME_PREFIX}{_python_name(hook)}_{shim_idx}" + return f"{SHIM_NAME_PREFIX}{_python_name(hook)}_{hook_idx}" if obj_name and "" in obj_name: raise ValueError("functions defined in local scopes are not yet supported.") diff --git a/npe2/_plugin_manager.py b/npe2/_plugin_manager.py index 2ef2043d..e88c6802 100644 --- a/npe2/_plugin_manager.py +++ b/npe2/_plugin_manager.py @@ -225,7 +225,7 @@ def __init__( self._contrib = _ContributionsIndex() self._manifests: Dict[PluginName, PluginManifest] = {} self.events = PluginManagerEvents(self) - self._shims: List[NPE1Adapter] = [] + self._npe1_adapters: List[NPE1Adapter] = [] # up to napari 0.4.15, discovery happened in the init here # so if we're running on an older version of napari, we need to discover @@ -278,11 +278,11 @@ def discover(self, paths: Sequence[str] = (), clear=False) -> None: if result.manifest and result.manifest.name not in self._manifests: self.register(result.manifest, warn_disabled=False) - def index_npe1_shims(self): + def index_npe1_adapters(self): with warnings.catch_warnings(): warnings.showwarning = lambda e, *_: print(str(e).split(" Please add")[0]) - while self._shims: - self._contrib.index_contributions(self._shims.pop()) + while self._npe1_adapters: + self._contrib.index_contributions(self._npe1_adapters.pop()) def register(self, manifest: PluginManifest, warn_disabled=True) -> None: """Register a plugin manifest""" @@ -297,7 +297,7 @@ def register(self, manifest: PluginManifest, warn_disabled=True) -> None: "be indexed. Use `warn_disabled=False` to suppress this message." ) elif isinstance(manifest, NPE1Adapter): - self._shims.append(manifest) + self._npe1_adapters.append(manifest) else: self._contrib.index_contributions(manifest) self.events.plugins_registered.emit({manifest}) diff --git a/npe2/manifest/_npe1_adapter.py b/npe2/manifest/_npe1_adapter.py index b23c4fd4..6dd0c7ba 100644 --- a/npe2/manifest/_npe1_adapter.py +++ b/npe2/manifest/_npe1_adapter.py @@ -15,10 +15,35 @@ class NPE1Adapter(PluginManifest): + """PluginManifest subclass that acts as an adapter for 1st gen plugins. + + During plugin discovery, packages that provide a first generation + 'napari.plugin' entry_point (but do *not* provide a second generation + 'napari.manifest' entrypoint) will be stored as `NPE1Adapter` manifests + in the `PluginManager._npe1_adapters` list. + + This class is instantiated with only a distribution object, but lacks + contributions at construction time. When `self.contributions` is accesses for the + first time, `_load_contributions` is called triggering and import and indexing of + all plugin modules using the same logic as `npe2 convert`. After import, the + discovered contributions are cached in a manifest for use in future sessions. + (The cache can be cleared using `npe2 cache --clear [plugin-name]`). + + + + Parameters + ---------- + dist : metadata.Distribution + A Distribution object for a package installed in the environment. (Minimally, + the distribution object must implement the `metadata` and `entry_points` + attributes.). It will be passed to `manifest_from_npe1` + """ + _is_loaded: bool = False _dist: metadata.Distribution def __init__(self, dist: metadata.Distribution): + """_summary_""" meta = PackageMetadata.from_dist_metadata(dist.metadata) super().__init__(name=dist.metadata["Name"], package_metadata=meta) self._dist = dist @@ -34,7 +59,7 @@ def _load_contributions(self) -> None: with discovery_blocked(): self._is_loaded = True # if we fail once, we still don't try again. try: - mf = manifest_from_npe1(self._dist, shim=True) + mf = manifest_from_npe1(self._dist, adapter=True) except Exception as e: warnings.warn( "Error importing contributions for first-generation " @@ -43,4 +68,4 @@ def _load_contributions(self) -> None: return self.contributions = mf.contributions - logger.debug("%r npe1 shim imported", self.name) + logger.debug("%r npe1 adapter imported", self.name) diff --git a/npe2/manifest/schema.py b/npe2/manifest/schema.py index 03d47737..6274f4f1 100644 --- a/npe2/manifest/schema.py +++ b/npe2/manifest/schema.py @@ -434,7 +434,7 @@ def _temporary_path_additions(paths: Sequence[Union[str, Path]] = ()): def _from_dist(dist: metadata.Distribution) -> Optional[PluginManifest]: - """Return PluginManifest or NPE1Shim for a metadata.Distribution object. + """Return PluginManifest or NPE1Adapter for a metadata.Distribution object. ...depending on which entry points are available. """ diff --git a/tests/test_npe1_adapter.py b/tests/test_npe1_adapter.py index 6c6b5de5..a026fb88 100644 --- a/tests/test_npe1_adapter.py +++ b/tests/test_npe1_adapter.py @@ -8,6 +8,7 @@ from npe2 import PluginManager from npe2.manifest import _npe1_adapter, utils from npe2.manifest.sample_data import SampleDataGenerator +from npe2.manifest.utils import SHIM_NAME_PREFIX try: from importlib import metadata @@ -15,19 +16,19 @@ import importlib_metadata as metadata # type: ignore -def test_shim_no_npe1(): +def test_adapter_no_npe1(): pm = PluginManager() pm.discover() - assert not pm._shims + assert not pm._npe1_adapters -def test_npe1_shim(uses_npe1_plugin): +def test_npe1_adapter(uses_npe1_plugin): """Test that the plugin manager detects npe1 plugins, and can index contribs""" pm = PluginManager() pm.discover() - # we've found a shim - assert len(pm._shims) == 1 + # we've found an adapter + assert len(pm._npe1_adapters) == 1 mf = pm.get_manifest("npe1-plugin") assert isinstance(mf, _npe1_adapter.NPE1Adapter) assert mf.package_metadata @@ -41,13 +42,13 @@ def test_npe1_shim(uses_npe1_plugin): "manifest_from_npe1", wraps=_npe1_adapter.manifest_from_npe1, # type: ignore ) as mock: - pm.index_npe1_shims() - # the shim has been cleared by the indexing - assert len(pm._shims) == 0 + pm.index_npe1_adapters() + # the adapter has been cleared by the indexing + assert len(pm._npe1_adapters) == 0 # manifest_from_npe1 was called - mock.assert_called_once_with(mf._dist, shim=True) + mock.assert_called_once_with(mf._dist, adapter=True) # NOTE: accessing the `.contributions` object would have also triggered - # importing, like pm.index_npe1_shims() above, but it would not have + # importing, like pm.index_npe1_adapters() above, but it would not have # injected the contributions into the pm._contrib object. assert mf.contributions.sample_data @@ -55,13 +56,13 @@ def test_npe1_shim(uses_npe1_plugin): def _get_mf() -> _npe1_adapter.NPE1Adapter: pm = PluginManager.instance() pm.discover() - pm.index_npe1_shims() + pm.index_npe1_adapters() mf = pm.get_manifest("npe1-plugin") assert isinstance(mf, _npe1_adapter.NPE1Adapter) return mf -def test_shim_pyname_sample_data(uses_npe1_plugin): +def test_adapter_pyname_sample_data(uses_npe1_plugin): """Test that objects defined locally in npe1 hookspecs can be retrieved.""" mf = _get_mf() samples = mf.contributions.sample_data @@ -73,7 +74,7 @@ def test_shim_pyname_sample_data(uses_npe1_plugin): with patch.object(utils, "_import_npe1_shim", wraps=utils._import_npe1_shim) as m: func = sample_generator.get_callable() assert isinstance(func, partial) # this is how it was defined in npe1-plugin - pyname = "__npe1shim__.npe1_module:napari_provide_sample_data_1" + pyname = f"{SHIM_NAME_PREFIX}npe1_module:napari_provide_sample_data_1" m.assert_called_once_with(pyname) assert np.array_equal(func(), ONES) @@ -83,7 +84,7 @@ def test_shim_pyname_sample_data(uses_npe1_plugin): assert np.array_equal(func(), ONES) -def test_shim_pyname_dock_widget(uses_npe1_plugin): +def test_adapter_pyname_dock_widget(uses_npe1_plugin): """Test that objects defined locally in npe1 hookspecs can be retrieved.""" mf = _get_mf() widgets = mf.contributions.widgets @@ -94,7 +95,9 @@ def test_shim_pyname_dock_widget(uses_npe1_plugin): caller = wdg_contrib.get_callable() assert isinstance(caller, MagicFactory) assert ".local_widget" in caller.keywords["function"].__qualname__ - pyname = "__npe1shim__.npe1_module:napari_experimental_provide_dock_widget_2" + pyname = ( + f"{SHIM_NAME_PREFIX}npe1_module:napari_experimental_provide_dock_widget_2" + ) m.assert_called_once_with(pyname) m.reset_mock() @@ -104,11 +107,11 @@ def test_shim_pyname_dock_widget(uses_npe1_plugin): caller2 = wdg_contrib2.get_callable() assert isinstance(caller2, MagicFactory) assert ".local_function" in caller2.keywords["function"].__qualname__ - pyname = "__npe1shim__.npe1_module:napari_experimental_provide_function_1" + pyname = f"{SHIM_NAME_PREFIX}npe1_module:napari_experimental_provide_function_1" m.assert_called_once_with(pyname) -def test_shim_error_on_import(): +def test_adapter_error_on_import(): class FakeDist(metadata.Distribution): def read_text(self, filename): if filename == "METADATA": From bc9c5b9921939432ae042655bc3652bf8d760ebb Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Wed, 23 Mar 2022 18:03:00 -0400 Subject: [PATCH 12/13] no cover --- codecov.yml | 2 +- npe2/_from_npe1.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/codecov.yml b/codecov.yml index 640bd899..26a09307 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,5 +1,5 @@ coverage: status: - patch: + project: default: target: 100% diff --git a/npe2/_from_npe1.py b/npe2/_from_npe1.py index 20f084dc..434bcb3c 100644 --- a/npe2/_from_npe1.py +++ b/npe2/_from_npe1.py @@ -481,7 +481,7 @@ def _python_name( f = obj.keywords.get("function") if f: v = getattr(f, "__globals__", {}).get(getattr(f, "__name__", "")) - if v is obj: + if v is obj: # pragma: no cover mod_name = f.__module__ obj_name = f.__qualname__ From 0ae93ccbc010b02926ff544e00b744c5063409aa Mon Sep 17 00:00:00 2001 From: Talley Lambert Date: Wed, 23 Mar 2022 18:22:19 -0400 Subject: [PATCH 13/13] more shim word changes --- npe2/cli.py | 2 +- npe2/manifest/_npe1_adapter.py | 2 +- tests/test_npe1_adapter.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/npe2/cli.py b/npe2/cli.py index 2712d54a..4525d2d8 100644 --- a/npe2/cli.py +++ b/npe2/cli.py @@ -147,7 +147,7 @@ def convert( @app.command() def cache( clear: Optional[bool] = typer.Option( - False, "--clear", "-d", help="Clear the npe1 shim manifest cache" + False, "--clear", "-d", help="Clear the npe1 adapter manifest cache" ), names: List[str] = typer.Argument( None, help="Name(s) of distributions to list/delete" diff --git a/npe2/manifest/_npe1_adapter.py b/npe2/manifest/_npe1_adapter.py index dab698dd..8a3bfd37 100644 --- a/npe2/manifest/_npe1_adapter.py +++ b/npe2/manifest/_npe1_adapter.py @@ -95,7 +95,7 @@ def _load_contributions(self) -> None: if self._cache_path().exists() and not os.getenv(NPE2_NOCACHE): mf = PluginManifest.from_file(self._cache_path()) self.contributions = mf.contributions - logger.debug("%r npe1 shim loaded from cache", self.name) + logger.debug("%r npe1 adapter loaded from cache", self.name) return with discovery_blocked(): diff --git a/tests/test_npe1_adapter.py b/tests/test_npe1_adapter.py index b3383662..a1f3b0f1 100644 --- a/tests/test_npe1_adapter.py +++ b/tests/test_npe1_adapter.py @@ -166,12 +166,12 @@ def read_text(self, filename): def locate_file(self, *_): ... - shim = _npe1_adapter.NPE1Adapter(FakeDist()) + adapter = _npe1_adapter.NPE1Adapter(FakeDist()) def err(): raise ImportError("No package found.") with pytest.warns(UserWarning) as record: with patch.object(_npe1_adapter, "manifest_from_npe1", wraps=err): - shim.contributions + adapter.contributions assert "Error importing contributions for" in str(record[0])