Cleanup manifest (#38)

* Some cleanup

* Some more cleanup

* Change 'publisher' to 'authors' (#39)

* Update npe2/_plugin_manager.py

* Update npe2/_from_npe1.py

Co-authored-by: Talley Lambert <[email protected]>

* Update npe2/manifest/schema.py

Co-authored-by: Talley Lambert <[email protected]>

* Add back some comments

* fix test

* fix test

* add back license field (#17)

* add back license field try2 (#17)

Co-authored-by: Talley Lambert <[email protected]>

npe2/_command_registry.py

@@ -27,7 +27,7 @@ def register(self, id: str, command: Callable) -> PDisposable:
         if not callable(command):
             raise TypeError(f"Cannot register non-callable command: {command}")
 
-        # TODO: validate argumemnts and type constraints
+        # TODO: validate arguments and type constraints
         # possibly wrap command in a type validator?
 
         self._commands[id] = command

npe2/_from_npe1.py

@@ -80,13 +80,19 @@ def manifest_from_npe1(
     plugin_name = cast(str, plugin_name)
 
     if not plugin_manager.is_registered(plugin_name):
-        # TODO: it would be nice to add some logic to prevent confusion here.
-        # for example... if the plugin name doesn't equal the package name, we
-        # should still be able to find it if the user gives a package name
+        # "plugin name" is not necessarily the package name. If the user
+        # supplies the package name, try to look it up and see if it's a plugin
 
         try:
-            dist = distribution(plugin_name)  # returns a list.  multiple plugins?
-            plugin_name = dist.entry_points[0].name
+            dist = distribution(plugin_name)
+            plugin_name = next(
+                e.name for e in dist.entry_points if e.group == "napari.plugin"
+            )
+        except StopIteration:
+            raise PackageNotFoundError(
+                f"Could not find plugin {plugin_name!r}. Found a package by "
+                "that name but it lacked the 'napari.plugin' entry point group"
+            )
         except PackageNotFoundError:
             raise PackageNotFoundError(
                 f"Could not find plugin {plugin_name!r}\n"

npe2/_plugin_manager.py

@@ -52,7 +52,7 @@ def __getitem__(self, index: Union[int, slice]) -> Set[Interval[T]]:
             ...
 
 
-# this is `PluginManifest.name`
+PluginName = str  # this is `PluginManifest.name`
 
 
 class _ContributionsIndex:
@@ -318,8 +318,10 @@ def get_writer(
             elif not ext and len(layer_types) == 1:  # No extension, single layer.
                 ext = next(iter(writer.filename_extensions), "")
                 return writer, path + ext
-            else:
-                raise ValueError
+            # When the list of extensions for the writer doesn't match the
+            # extension in the filename, keep searching.
+
+        # Nothing got found
         return None, path
 
 

npe2/manifest/schema.py

@@ -14,7 +14,6 @@
     Any,
     Callable,
     Iterator,
-    List,
     NamedTuple,
     Optional,
     Sequence,
@@ -36,7 +35,6 @@
     from email.message import Message
     from importlib.metadata import EntryPoint
 
-
 spdx_ids = (Path(__file__).parent / "spdx.txt").read_text().splitlines()
 SPDX = Enum("SPDX", {i.replace("-", "_"): i for i in spdx_ids})  # type: ignore
 
@@ -54,12 +52,14 @@ class DiscoverResults(NamedTuple):
 class PluginManifest(BaseModel):
 
     # VS Code uses <publisher>.<name> as a unique ID for the extension
-    # should this just be the package name ... not the module name? (probably yes)
-    # do we normalize this? (i.e. underscores / dashes ?)
+    # should this just be the package name ... not the module name? (yes)
+    # do we normalize this? (i.e. underscores / dashes ?) (no)
     # TODO: enforce that this matches the package name
+
     name: str = Field(
         ...,
-        description="The name of the plugin - should be all lowercase with no spaces.",
+        description="The name of the plugin. Should correspond to the python "
+        "package name for this plugin.",
     )
 
     author: Optional[str] = Field(
@@ -76,55 +76,35 @@ class PluginManifest(BaseModel):
         # non-word character.
         regex=r"^[^\W_][\w -~]{1,38}[^\W_]$",
     )
-    # take this from setup.cfg
+
     description: Optional[str] = Field(
         description="A short description of what your extension is and does."
+        "When unspecified, the description is taken from package metadata."
     )
 
     # TODO:
     # Perhaps we should version the plugin interface (not so the manifest, but
     # the actual mechanism/consumption of plugin information) independently
     # of napari itself
 
-    # mechanistic things:
-    # this is the module that has the activate() function
+    # The module that has the activate() function
     entry_point: Optional[str] = Field(
         default=None,
-        description="The extension entry point. This should be a fully qualified "
-        "module string. e.g. `foo.bar.baz`",
+        description="The extension entry point. This should be a fully "
+        "qualified module string. e.g. `foo.bar.baz` for a module containing "
+        "the plugin's activate() function.",
     )
 
-    # this comes from setup.cfg
-    version: Optional[str] = Field(None, description="SemVer compatible version.")
-    # this should come from setup.cfg ... but they don't requireq SPDX
+    # this should come from setup.cfg ... but they don't require SPDX
     license: Optional[SPDX] = None
 
-    contributions: Optional[ContributionPoints]
-
-    categories: List[str] = Field(
-        default_factory=list,
-        description="specifically defined classifiers",
-    )
-
-    # in the absense of input. should be inferred from version (require using rc ...)
-    # or use `classifiers = Status`
-    preview: Optional[bool] = Field(
+    version: Optional[str] = Field(
         None,
-        description="Sets the extension to be flagged as a Preview in napari-hub.",
+        description="SemVer compatible version. When unspecified the version "
+        "is taken from package metadata.",
     )
-    private: bool = Field(False, description="Whether this extension is private")
 
-    # activationEvents: Optional[List[ActivationEvent]] = Field(
-    #     default_factory=list,
-    #     description="Events upon which your extension becomes active.",
-    # )
-
-    # @validator("activationEvents", pre=True)
-    # def _validateActivationEvent(cls, val):
-    #     return [
-    #         dict(zip(("kind", "id"), x.split(":"))) if ":" in x else x
-    #         for x in val
-    #     ]
+    contributions: Optional[ContributionPoints]
 
     _manifest_file: Optional[Path] = None
 
@@ -276,13 +256,6 @@ def _populate_missing_meta(self, metadata: Message):
             self.description = metadata["Summary"]
         if not self.license:
             self.license = metadata["License"]
-        if self.preview is None:
-            for k, v in getattr(metadata, "_headers"):
-                if k.lower() == "classifier" and v.lower().startswith(
-                    "development status"
-                ):
-                    self.preview = int(v.split(":: ")[-1][0]) < 3
-                    break
 
     @classmethod
     def discover(

tests/sample/my_plugin/napari.yaml

@@ -1,6 +1,5 @@
 name: my_plugin
 display_name: My Plugin
-license: BSD-3-Clause
 entry_point: my_plugin
 contributions:
   commands:

tests/test_conversion.py

@@ -4,6 +4,11 @@
 
 from npe2._from_npe1 import manifest_from_npe1
 
+try:
+    from importlib.metadata import PackageNotFoundError
+except ImportError:
+    from importlib_metadata import PackageNotFoundError  # type: ignore
+
 
 def gen_data():
     ...
@@ -93,3 +98,8 @@ def test_conversion2():
 def test_conversion_missing():
     with pytest.raises(ModuleNotFoundError), pytest.warns(UserWarning):
         manifest_from_npe1("does-not-exist-asdf6as987")
+
+
+def test_conversion_package_is_not_a_plugin():
+    with pytest.raises(PackageNotFoundError):
+        manifest_from_npe1("pytest")