Clarify valid manifest name keys

[DragaDoncila]

• 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?

[tlambert03]

hey @DragaDoncila. I believe the goal is that the name must be the package name, but normalized to a python identifier. so dashes will be replaced by underscores. (pypi still treats those as unique). I definitely imagined that we would get around to all of these things, with nicer error message, before anyone actually used them, but it looks like we're under a time crunch now

[tlambert03]

basically... none of this is ready for mass consumption.

[DragaDoncila]

hey @DragaDoncila. I believe the goal is that the name must be the package name, but normalized to a python identifier.

Ah ok perfect so I think then the "change" required would be to validate the name key as a Python identifier rather than package name, and also against the name listed in setup.cfg:metadata?

I definitely imagined that we would get around to all of these things, with nicer error message, before anyone actually used them, but it looks like we're under a time crunch now

Completely understand - I'm only testing things now with the plugin workshop in mind. If we need more manpower on npe2 I'm happy to help get some of these small issues across the line

[tlambert03]

thank you @DragaDoncila. I added a note about this to the pre-release checklist #32

I agree with proposed changes. note that we could technically do this for the user (coerce the name at discovery), but I think it's best to just error: so that they are aware of the key. The best place to do this is somewhere in the manifest creation, when we have both the validated manifest, and the distribution object itself. Like in _populate_missing_metadata

or in from_distribution or from_entrypoint

[nclack]

I'm closing this. It's mostly addressed by #44. The rest will be done in #48.