cc @nclack, here's what I was kinda thinking in that meeting just now:

class _BaseWriter:
    command: str  # command id
    extensions: Set[str]  # compatible extensions

# these can be composed.  If a user has selected multiple layers, we can
# dispatch to each LayerWriter.
# usage scenario, user has one or more of the SAME type of layer selected,
# then they have a dialog with a set of possible extensions (and plugin) to choose from.
class LayerWriter(_BaseWriter):
    layer_type: LayerType  # the type of layer this writer knows how to save
    accepts_multiple: bool  # whether it accepts multiple layers (of the SAME type)

# these plugins will never be composed.  They have to take the selected layers and
# save them all.
# usage scenario: user has multiple layers selected, and hits save (all). 
# Then they are presented with a dialog that shows plugins able to
# save this set, and their extensions.
class MultiLayerWriter(_BaseWriter):
    layer_types: Set[LayerType]  # the set of layers it accepts
    subset_ok: bool  # whether a subset of `layer_types` is ok (or must have all)



class Contributions:
    ...
    writers: List[Union[LayerWriter, MultiLayerWriter]]

This issue proposes a task for adding a validator for display_name.

One proposal:
64 character, ascii only, cannot start with space, no tabs

display_name is presented in a few different contexts, one of which is the save file dialog.

It appears like the longest plugin name to date is 39 characters long.

The current spec around reader plugins has some problems that would be nice to address with a schema change. This would likely require a change of the schema version.

Opening this issue to start gathering ideas/requirements.

Problems:

• The two-call pattern is unnecessary now since file type detection is handled by the manifest.
• Clarify behavior when readers are passed a list of paths. It would be nice to signal when a list of layers is expected vs a desire to aggregate into a single Layer (see discussion in #105).

Opportunities:

• add support for streaming (e.g. consuming a video bytestream)
• add support for partial reads (e.g. frames 100-200 of a tiff stack)

I think that the command field of each command is misleading. Especially when submenu also have a command field
and one of the command field defines an id, and another one refers to an existing id.

Again I know that it's what was code does, but I believe it would be more understandable to rename the command id field to just id, or something else not ambiguous.

I just retroactively tagged the version right after DynamicPlugin was added and released it as v0.2.2 so that @DragaDoncila and others can use it while testing in napari. Since the shim is merged into main, we need to do more to check compatibility with napari before releasing main

hey @nclack ... I was looking at the vscode docs, and realized that they use "engine" to mean the version of vscode that the package is targeting (not the version of the manifest). On its own, I don't think "engine" is immediately obvious (though, perhaps someone could figure it out because it's called npe). But, what would you think of using schema_version instead? Hard to get that wrong

Description

Fields that use a regex to validate have difficult to interpret error messages. For example:

🅇 Invalid! 1 validation error for PluginManifest
contributions -> commands -> 0 -> id
  string does not match regex "^((([a-zA-Z_][a-zA-Z_0-9]+)\.)*([a-zA-Z_][a-zA-Z_0-9]+))$" (type=value_error.str.regex; pattern=^((([a-zA-Z_][a-zA-Z_0-9]+)\.)*([a-zA-Z_][a-zA-Z_0-9]+))$)

It would be nice to replace these with something more friendly.

• npe2 version: 0.2.2
• Python version: 3.7-3.9
• Operating System: All

Description

When running test everything pass for PyQt5 but fail with PySide2 https://github.com/4DNucleome/PartSeg/actions/runs/2085569645
When downgrading npe2 to version 0.2.1 everything works correctly.

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/czaki/.pyenv/versions/3.8.3/lib/python3.8/multiprocessing/spawn.py", line 116, in spawn_main
    exitcode = _main(fd, parent_sentinel)
  File "/home/czaki/.pyenv/versions/3.8.3/lib/python3.8/multiprocessing/spawn.py", line 126, in _main
    self = reduction.pickle.load(from_parent)
  File "/home/czaki/Projekty/PartSeg/package/tests/test_PartSeg/test_sentry.py", line 12, in <module>
    from PartSeg.common_backend.base_argparser import safe_repr
  File "/tmp/tox/py38-PySide2-all/lib/python3.8/site-packages/PartSeg/common_backend/__init__.py", line 14, in <module>
    from napari.settings import get_settings as _napari_get_settings
  File "/tmp/tox/py38-PySide2-all/lib/python3.8/site-packages/napari/settings/__init__.py", line 6, in <module>
    from ._napari_settings import NapariSettings
  File "/tmp/tox/py38-PySide2-all/lib/python3.8/site-packages/napari/settings/_napari_settings.py", line 9, in <module>
    from ._appearance import AppearanceSettings
  File "/tmp/tox/py38-PySide2-all/lib/python3.8/site-packages/napari/settings/_appearance.py", line 4, in <module>
    from ..utils.theme import available_themes
  File "/tmp/tox/py38-PySide2-all/lib/python3.8/site-packages/napari/utils/theme.py", line 335, in <module>
    _install_npe2_themes(_themes)
  File "/tmp/tox/py38-PySide2-all/lib/python3.8/site-packages/napari/utils/theme.py", line 325, in _install_npe2_themes
    import npe2
  File "/tmp/tox/py38-PySide2-all/lib/python3.8/site-packages/shiboken2/files.dir/shibokensupport/__feature__.py", line 142, in _import
    return original_import(name, *args, **kwargs)
  File "/tmp/tox/py38-PySide2-all/lib/python3.8/site-packages/npe2/__init__.py", line 8, in <module>
    from ._dynamic_plugin import DynamicPlugin
  File "/tmp/tox/py38-PySide2-all/lib/python3.8/site-packages/shiboken2/files.dir/shibokensupport/__feature__.py", line 142, in _import
    return original_import(name, *args, **kwargs)
  File "/tmp/tox/py38-PySide2-all/lib/python3.8/site-packages/npe2/_dynamic_plugin.py", line 32, in <module>
    COMMAND_PARAMS = inspect.signature(CommandContribution).parameters
  File "/home/czaki/.pyenv/versions/3.8.3/lib/python3.8/inspect.py", line 3093, in signature
    return Signature.from_callable(obj, follow_wrapped=follow_wrapped)
  File "/home/czaki/.pyenv/versions/3.8.3/lib/python3.8/inspect.py", line 2842, in from_callable
    return _signature_from_callable(obj, sigcls=cls,
  File "/home/czaki/.pyenv/versions/3.8.3/lib/python3.8/inspect.py", line 2252, in _signature_from_callable
    raise TypeError(
TypeError: unexpected object <pydantic.utils.ClassAttribute object at 0x7f4639fda8b0> in __signature__ attribute

Following the current flow in napari/npe2 it looks to me like iter_compat_reader (at least, maybe write as well) will never get a Path, it will only get strings.

It's not like the hook spec themselves, and though I would be in favor of also making them stricter, this is more of an internal api.

Another reason is that napari uses:

for url in event.mimeData().urls():
            if url.isLocalFile():
                filenames.append(url.toLocalFile())
            else:
                filenames.append(url.toString())

Which in then ends ends up in iter_compat_reader so my guess is that some of the things are not representable by path anyway.

• npe2 version: npe2-0.1.2
• Python version: 3.9
• Operating System: macOS Big Sur

Description

When trying to convert an npe1 plugin with a setup.py file, the entry point is being parsed as a list of lists, with each inner list being a letter. Inspecting _entry_points here while debugging gives:

> _entry_points
[['S'], ['t'], ['a'], ['r'], ['D'], ['i'], ['s'], ['t'], [''], ['', ''], [''], ['s'], ['t'], ['a'], ...]

The process then terminates with

Conversion failed:
ValueError: not enough values to unpack (expected 2, got 1)

based on the unpacking happening on the next line

What I Did

pip install npe2
git clone https://github.com/stardist/stardist-napari.git
cd stardist-napari
pip install -e .
npe2 convert .

I'm not sure if this is a stardist specific issue or not (feel free to close if so)

• npe2 version: NA
• Python version: NA
• Operating System: NA

Description

There is a missing screenshot when browsing online docs:

https://napari.org/plugins/stable/first_plugin.html

For any dialog or widget, I generally would want to call it "Do A Thing", with the corresponding menubar item called "Do A Thing…", see Apple's Human Interface Guidelines on menu item titles:

Use an ellipsis whenever choosing a menu item requires additional input from the user. The ellipsis character (…) means a dialog or separate window will open and prompt the user for additional information or to make a choice.

The ellipsis version of the string seems to me to be suitable for a command name in a command palette also.

We can discuss all of the above but then we should also codify it in a best-practices document like the Apple HIG, at least once single-action menu item commands are a thing.

See jni/zarpaint#20 for some context.

• npe2 version: 0.1.2
• Python version: 3.9.6
• Operating System: Ubuntu 18.04

• npe2 version: 0.2.1
• Python version: 3.9.11
• Operating System: macOS 12.3

Description

Working in a clean conda environment, @nclack and I ran into an issue running npe2 validate . on a test plugin

What I Did

(plugin) czirwc1macos2994:napari-fits-reader bnelson$ npe2 validate .
Traceback (most recent call last):
  File "/Users/bnelson/anaconda3/envs/plugin/bin/npe2", line 5, in <module>
    from npe2.cli import main
  File "/Users/bnelson/anaconda3/envs/plugin/lib/python3.9/site-packages/npe2/cli.py", line 6, in <module>
    import typer
  File "/Users/bnelson/anaconda3/envs/plugin/lib/python3.9/site-packages/typer/__init__.py", line 12, in <module>
    from click.termui import get_terminal_size as get_terminal_size
ImportError: cannot import name 'get_terminal_size' from 'click.termui' (/Users/bnelson/anaconda3/envs/plugin/lib/python3.9/site-packages/click/termui.py)

just split on : ? I guess using regex to validate even more sure, but if it's invalid it will anyway fail just after.

Originally posted by @Carreau in #124 (comment)

napari's plugin install dialog allows plugins to be:

• enabled/disabled
• installed/uninstalled

I believe (un)install can be handled entirely on the napari side. When a plugin is uninstalled, I think we require napari to be restarted for the changes to take effect. That system should work for npe2 plugins as well.

However, some functionality should be added to npe2 to support enable/disable.

Some requirements:

• it should be possible to get a list of all plugins that includes both the enabled and disabled
• iter_compatible_* functions should omit disabled plugins
• deactivate() should be invoked for plugins transitioning from the enabled to disabled state

I would love thoughts and input on the requirements. The description of this issue can be updated to keep them accurate.

Reference: Napari plugin install dialog

• npe2 version:
• Python version:
• Operating System:

Description

test_render_docs imports main from from _docs.render, whcih itself uses urlopen with "https://github.com/napari/npe2/releases/latest/download/schema.json" as a parameter.

I think this might be a problem in other cases, like packaging on linux, or other system, and in general we should not get the schema from latest napari from the test suite.

I would bundle the schema and have a separate github action step that check the schema is up to date, but not rely in having an internat connection during the tests.

What I Did

$ cd /mnt/train_with_no_wifi
$ pytest

Test suite fails.

Proposal:

• User drags file into napari
• UI pops up showing a group of radioboxes with all readers that can read that file
  • for npe2, we can use iter_compatible_readers
  • for npe1, we could also mock this functionality by seeing which readers actually return a function
• there's a checkbox that says "remember this settings", and tells them where to change in (in the preferences somewhere).

cc @DragaDoncila @nclack

whether it should get handled on the napari side (before passing paths to npe2) or on the npe2 side, the npe2 reader in io_utils is not currently handling a list of strings properly.

with a contribution of:

readers:
    ...
    filename_patterns: ['*.ext']

iter_compatible_readers(['f1.ext', f2.ext']) will return None

a list of strings is what will be passed when "open as stack" is true.

Originally, I think we we're going to use this to help distinguish plugins that had similar names. But if we end up requiring plugin name is a python package name (see #37) then that's not necessary.

We've ended up using this more as a list of authors. When missing it's populating this from the Author field of the package metadata when it's missing.

Type in docstring does not match type in signature, maybe we can go "ModuleType"

We can always change later to not mess up the 3 PR chains.

Originally posted by @Carreau in #124 (comment)

From today's discussion,

we likely want a activate at startup event, except if a plugin requests that instead of enabling it by default, we should on next start ask "this plugin want to activate on startup, this can have consequences on the speed of napari stability and startup, to allow them to do so, go to preferences > plugins > allow blahblah blah".

We could also have a blessed list hardcoded but let's see later for how to do that.
Either PR against napari, or crypto key ?

Description

npe2 has a command line tool that will validate a plugin author has a well-defined manifest.

Additional validation for plugin developers would help them double check they've fulfilled functional requirements for their plugin.

This issue proposes to add features to

• do smoke testing on the callables where that's possible
• check type annotations on callables when provided

A flag, something like npe2 validate --full, should trigger these more expensive checks.

For reference, see comments at the end of napari/napari#3617.

@uschmidt83 notes that vertical stretch is not part of the widget schema.

We should re-evaluate that kwarg and determine if it should be part of the schema, or handled on the napari side. If I recall correctly, it was always a bit of a "workaround" keyword, so let's take another look.

Description

Task: Incorporate changes from a review of the schema.

The review aimed to identify unused fields and other cleanup items. These aren't listed here, but will be identified as todo items added in the pr associated with this issue.

One of the cleanup items is the regex around command id's which should change to be like a python package name (rather than like a python identifier).

This task requires:

• updating npe2
• review of related docs to incorporate changes
• review of the npe2 cookiecutter to incorporate changes
• update the npe2 convert utility to incorporate changes (in particular around command id)

please add bullet points here for things that need to be done before napari depends on npe2 and it is "active"

• publish on pypi, and it will also need to be on conda-forge for the napari package

• docs done. See napari/napari#3818

  • intro guide drafted
  • manifest spec drafted
  • migration guide drafted

• do final audit of all fields in the schema, making sure they are A) very clearly defined in the field description, and B) provide very clear errors when not correct. See #48

• basic napari install dialog support napari/napari#3694

non critical

• test the napari integration code better.
• get tests on the npe2 side close to 100% (we're doing ok, but now's a great time to get full coverage) #70
• Reader call order priority... how to deal with priority across npe1/npe2. See #41.
• napari preferences > plugins panel. user defined sort order
• npe2 conversion cli tool done
• move to napari team space
• update readme
• #47
• autocomplete support for schema #50

future

• #36

I ran npe2 convert on affinder at https://github.com/jni/affinder/tree/1b35d64bb8eb57b842ba15162ccbd57048f04280

(Side note: I expected that I would run npe2 convert on my code, not on installed plugins. npe2 convert . gave me a meaningful error but I kinda would have wanted it to just look at a repo rather than an installed package. Maybe that's harder though.)

1. Running npe2 convert affinder > napari.yaml gives a warning, Invalid schema 'napari:builtins.yaml', and no other feedback. It actually succeeded, so a success message should probably also be printed on stderr?
2. npe2 convert puts the napari.yaml in the current directory, but it actually needs to go in the package directory and get added to the manifest, right? That can probably be done automatically?

was just playing around with writer spec and tried to save a 3D file as a jpeg ... unsurprisingly it failed, ValueError: Image must be 2D (grayscale, RGB, or RGBA). ... but it seems like the kind of thing we could catch with when statements.

Description

When plugins are migrated to npe2, a change of the plugin name is often required. This is a consequence of the choice to force plugin name to be the package name.

Napari users rely on specifying the plugin name for some operations, e.g.:

layers.save(path, plugin='svg')

But after migration that plugin will be named 'napari-svg' and the lookup will fail. Code relying on that 'svg' name will break when the npe2 version of that plugin is installed.

We could add an alias field in the top-level part of the manifest that allows plugin authors to specify an alternative name for these kinds of lookups. For example:

name: napari-svg
alias: svg     # <--- new field
display_name: napari SVG
contributions:
   ...

The alias wouldn't be unique. The lookup would return the first plugin with a matching name or alias. If a user wanted to guarantee their lookup addressed a specific plugin they could specify the actual plugin name, e.g. 'napari-svg'.

paths is there to allow discovery of plugins that are not in sys.path (by temporarily adding the specified paths to sys.path, just for the duration of discovery) ...
while it works for discovery, it's rather error prone later because of all of our usage of delayed imports and python_name. Unless the manifest somehow remembers that it came from somewhere outside of sys.path, it will break at import time, and I think it's a YAGNI feature anyway

even once napari adds npe2 to dependencies, plugins are going to have a tricky time supporting various napari versions unless we come up with a way to allow them to have both entry points.

I think this came up in the past and I suggested that plugins only implement one of the two entry points, but forgot that this means they will no longer be compatible with napari < 0.4.13.

I think this will require some communication between plugin managers. Specifically, if npe2's plugin manager sees a plugin, npe1's plugin manager shouldn't register it.

Yaml allows new lines.

    - command: my_plugin.affinder
      title: Affinder
      python_name: | 
        npe2_tester.module.sub.sub:Affinder
        
      # why do we both a command id and python_name?
      # entertain possibility of *requiring* python_name

npe parse gives:

...
  - command: my_plugin.affinder
    python_name: 'npe2_tester.module.sub.sub:Affinder

      '
    title: Affinder

and validate as OK, which I'm sure is not what we want.

Description

semver was added as a dependency in #49 for parsing and comparing engine versions as an expediency.

It should be replaced with a custom parser. We don't need to pull in a dependency for this.

it's come up multiple times that we need a way to know, given some instance of a Contribution, which plugin provided that instance. There was briefly a get_manifest_for_writer method, that would index and perform that lookup specifically for writer contributions. I removed that since it was a) not necessary in that specific case, but mostly b) it was not general enough.

We should think about how we might do this more generally. We could have a base Contribution class that holds a plugin pointer, which gets populated during PluginManifest creation? (but which is otherwise empty for one-off Contribution instances?)

Description

Task: Simplify the interface that napari uses to save layers with npe2.

In #3, the npe2 api that napari uses consists of iter_compatible_writers() and npe2.save_layers(). The way this currently works means napari still has to do a fair amount of logic, e.g. generating extension strings for the "save file" dialog and selecting the writer to use in the viewer.layers.save() implementation.

Consolidating functionality on the npe2 side should simplify things, ideally to the point where napari doesn't have to be aware of the WriterContribution object at all.

See comments:
tlambert03#3 (comment)_

Some stuff we discussed in the meeting today is how to help plugins provide translation strings

in zulip previously, I had shown how vscode does this:

# main manifest file
contributes:
  commands:
    - command: my_plugin.hello_world
      title: '%my_plugin.hello_world.title%'
# my_plugin.nls.en.yaml
my_plugin.hello_world.title : Hello!

@goanpeca pointed out that this key-based method has some downsides and is tough to combine with the "string-as-key" method that we currently use. Instead, we could just identify certain fields in the manifest like "display_text" or "title" etc that we offer to translate.

class Manifest(BaseModel):
    some_field: str = Field(..., extra={'translatable': True})  # or whatever ... 

In addition we will scan the plugin code itself for trans._() calls.

Exactly how that all gets to crowdin and how plugin translation packs make their way back into napari at runtime is still a little foggy to me.

Description

cookiecutter is pretty limited. It's a pretty limited cli frontend in front of a jinja templating engine.

We could think about creating our own front-end - maybe keeping the jinja template for parts but using the npe2 machinery for manifest creation in particular. It seems super nice to me to have an npe2 create command. The npe2 command becomes something like a one-stop-shop for related tooling.

Also, we could leverage the type information in the manifest to automate some of the ui. ui could be a cli or a gui.

There was some discussion around this in the 2021 Dec 9 community meeting.

@goanpeca @tlambert03 @sofroniewn

Description

Base on errors that I meet working with napari I strongly suggest switching from the global plugin_manager object to the getter function (named ex. get_plugin_manager()).

Maybe it will be worth blocking calling it from module level (some try of this is present in #3427)

In this function the error catching is overly broad, any other error appear in the CLI as Could not find manifest for 'npe2_tester' as either a package name or a file, hiding potential bugs in npe2 tester. I think we should be more restrictive into both what we catch, and/or raise a more narrow exception (maybe custom exceptions? )

def _from_package_or_name(
        cls, package_or_filename: Union[Path, str]
    ) -> PluginManifest:
        
        try:
            return PluginManifest.from_file(package_or_filename)
        except ValidationError:
            raise
        except (FileNotFoundError, ValueError):
            try:
                return PluginManifest.from_distribution(str(package_or_filename))
            except ValidationError:
                raise
            except Exception as e:
                raise ValueError(
                    f"Could not find manifest for {package_or_filename!r} as either a "
                    "package name or a file.."
                ) from e

Description

Maybe following decision in cibuildwheel there should be change from toml to tomli or mayby rtoml even to get performance gain as parsing manifest will be important part of every napari start?

https://pypi.org/project/tomli/
https://pypi.org/project/rtoml/
pypa/cibuildwheel#534

@gselzer raised an important point on zulip. Here is a specific challenge for plugins using the JVM, but it hints at a broader challenge for plugins that need to share some external initialization:

from @gselzer
Our use case for napari-imagej centers around JVM startup; if multiple plugins want to add JARs to the java runtime before it starts up, they would need a way to do that before any of those plugins actually start the JVM. napari-imagej does its best to defer JVM startup, but if another plugin wants to add additional JARs or if another plugin starts the JVM before napari-imagej does, then one plugin will be unable to run. Ideally, any plugin that wants JARs included in the runtime should be able to declare them in some eager initialization that could run at e.g. plugin discovery time.

@ctrueden proposed something like "kernel space" plugins (such as a napari-jvm plugin) that act as aggregators for related functionality, and resources that must be shared:

One way to protect napari a bit more from rogue plugins could be to have these "kernel-space" plugins act as aggregators for related functionality. What I'm envisioning for Java-dependent plugins would be that you'd have a "Java plugin" for napari, which has some napari-startup-time initialization taking care of preparing the Java runtime. Then the actual Java-dependent plugins like napari-imagej would NOT have startup-time initialization, but rather use a new java_libraries: YAML field or something, which the Java plugin itself consumes. So you'd have a situation then where Java-dependent plugins could declare metadata that affects how the Java plugin prepares the Java runtime, but in a safer way than giving those plugins direct eager code execution.

I like that general idea, as it's extensible for arbitrary other resources. One question becomes whether napari-jvm would be in some way a "blessed" plugin – that is, how does everyone agree on what the kernel-space aggregator is? And does npe2 need to add the java_libraries field to the npe2 schema? or does napari-jvm register a key during it's special on-startup activation event that other plugins can declare support for?

thanks @gselzer and @ctrueden for raising. Curious to hear @nclack's thoughts on this particular issue.

in sample data, it's possible that someone will ship a tif file or something in their package. Before, they could use Path(__file__).parent... to point to it, but not in a static manifest,

One proposal is to use ${package}:

sample_data:
  - display_name: Some image
    key: some_image
    uri: ${package}/data/some_image.tif

where ${package} is to be expanded to the top_level directory of the package (found, for instance, with https://docs.python.org/3/library/importlib.html#importlib.util.find_spec or other means)

@Carreau, @nclack ?

Another good thing to do before release is to get our schema (once we finish cleanup and declare an initial version) onto https://www.schemastore.org/json/, where various vscode (e.g.) plugins can find it to provide autocompletion and hinting and stuff if someone is editing napari.yaml

Description

I wanted to track fields that will probably go unused or may be redundant.

These include:

• categories
• command.short_title (see tlambert03#3 (comment))

Presentation only:

• license
• preview

Putting these up for discussion. Please add things I missed. Some of these may just need a use case.

autogenerate_from_command is a bit verbose, but hard to confuse :)

that's fine, but i would have suggested autogenerate as being clear in context and a little more friendly, unless you think we'd ever have other autogenerate_from_.... fields, but those are likely to be incompatible with a bool on autogenerate_from_command

Originally posted by @sofroniewn in #51 (comment)

• npe2 version: 0.1.dev116+gdd529bf
• Python version: Python 3.9.7
• Operating System: MacOS Big Sur 11.6

Description

There are some inconsistencies in the way the name key of the manifest is defined/parsed by npe2.

The docs currently mention that it should be a valid package name (or at least that is the intention as I understood from @nclack). However, based on the command validation regex, it looks like they should be module names instead?

e.g. this manifest snippet is validated as ok by npe2 validate

name: nag-demo
display_name: Workshop Demo
license: BSD-3-Clause
entry_point: nag_demo

but if I add a reader command referencing this name:

name: nag-demo
display_name: Workshop Demo
license: BSD-3-Clause
entry_point: nag_demo

contributions:
  commands:
  - id: nag-demo.get_reader
    python_name: nag_demo._reader:napari_get_reader
    title: NAG Reader
  readers:
  - command: nag-demo.get_reader
    accepts_directories: false  
    filename_patterns: ['*.xfc']

then npe2 validate gives me:

% npe2 validate src/nag_demo/napari.yaml
🅇 Invalid! 1 validation error for PluginManifest
contributions -> commands -> 0 -> id
  string does not match regex "^((([a-zA-Z_][a-zA-Z_0-9]+)\.)*([a-zA-Z_][a-zA-Z_0-9]+))$" (type=value_error.str.regex; pattern=^((([a-zA-Z_][a-zA-Z_0-9]+)\.)*([a-zA-Z_][a-zA-Z_0-9]+))$)

I can change the name (and associated commands) nag-demo to nag_demo (the module name) and this works fine. However, I can also change the name to any random word I want and that also still works e.g.

name: foobar
display_name: Workshop Demo
license: BSD-3-Clause
entry_point: nag_demo

contributions:
  commands:
  - id: foobar.get_reader
    python_name: nag_demo._reader:napari_get_reader
    title: NAG Reader
  readers:
  - command: foobar.get_reader
    accepts_directories: false  
    filename_patterns: ['*.xfc']

gives us

% npe2 validate src/nag_demo/napari.yaml
✔ Manifest for 'Workshop Demo' valid!

Questions/issue

• the validator tool should error on a name key that can't be used as part of a command
• Is it intended behaviour that the name can be any identifier i.e. it doesn't have to relate in any way to the package name or module name? Are there best practice guidelines for what it should be?
• Is the manifest name supposed to be any valid package name (in which case we need to modify behaviour to accept -) or any valid module name?

Description

I discussed this in separate conversations w @tlambert03, @sofroniewn and in the plugin working group meeting.

One of the nice things about migrating these is that it makes it simple for people to integrate functions like these into napari without adding dependencies to their package (no npe2, magicgui, napari etc...just the manifest file)

The proposal is to migrate napari_experimental_provide_function hooks to be npe2 widget contributions. It sounds like something like this might work:

name: myplugin
contributions:
  commands:
    - id: myplugin.func
      python_name: myplugin:my_typed_function
      title: Open dock widget for myplugin function
  widgets:
    - command: myplugin.func
      name: Functionality
      type: magicgui                #<-- new field 

Implementation should cover:

• schema support
• npe2 conversion tool support
• npe2 doc update
• npe2 cookiecutter update

It might be nice to add a couple decorators like this:

import npe2

@npe2.widget(display_name="My Widget")
class MyWidget(QWidget):
    ...

@npe2.reader(filename_patterns=["*.tif"])
def read_tiff(path): ...

these would be no-op at runtime, but could allow this:

npe2 compile-manifest .

would would build a manifest for you out of the contributions...

this could also be used in a pre-commit hook, to make sure that the manifest agrees (for instance, if you rename or move a function). basically just an opt-in feature to make it easier to build a manifest and/or keep it in sync with your code

In converting over to using only npe2 interface in napari, I came across an issue where it was very difficult to get get_writer to pick the lossless writer by default when writing an image with write_layer_data_with_plugins:

        # build in a temporary directory and then move afterwards,
        # it makes cleanup easier if an exception is raised inside.
        with TemporaryDirectory(dir=path) as tmp:
            # Loop through data for each layer
            for layer_data_tuple in layer_data:
                _, meta, _ = layer_data_tuple
                # Get hook caller according to layer type

                # Create full path using name of layer
                full_path = abspath_or_url(os.path.join(tmp, meta['name']))
                # Write out data using first plugin found or named plugin if provided
                written.extend(
                    io_utils.write(
                        path=full_path,
                        layer_data=[layer_data_tuple],
                        plugin_name=plugin_name,
                    )
                )
            for fname in os.listdir(tmp):
                shutil.move(os.path.join(tmp, fname), path)

The tiff writer does appear first in the list of single image writers, but there's a few things in iter_compatible writers that mangle the order. Ultimately, I needed to remove these two lines from _writer_key in order to get it to choose the earlier listed tiff writer instead of the lossy writer:

            # 3. then sort by the number of listed extensions
            #    (empty set of extensions goes last)
            ext_len = len(writer.filename_extensions)

            # 4. finally group related extensions together
            exts = writer.filename_extensions

But, obviously, that's just a hack. @nclack, we didn't particularly anticipate this internal usage, but do you have any thoughts on how we might update our write_layer_data_with_plugins to maintain the current behavior while using npe2?