DynamicPlugin is not able to re-register commands after 'disable' then 'enable'

[lucyleeow]

• npe2 version: 0.7.0
• Python version: 3.9
• Operating System: Linux

Description

The fixture @tmp_plugin.contribute.widget does not seem to register the corresponding command contribution?

What I Did

The following test:

def test_plugin(
    make_napari_viewer,
    tmp_plugin: DynamicPlugin,
):
    """Check plugin menu items correct after a plugin changes state."""
    app = get_app()
    pm = tmp_plugin.plugin_manager

    @tmp_plugin.contribute.widget(display_name='Widget 1')
    def widget1():
        return DummyWidget()

    # Configures `app`, registers actions and initializes plugins
    make_napari_viewer()

    # Disable plugin
    pm.disable(tmp_plugin.name)

    # Enable plugin
    pm.enable(tmp_plugin.name)

will give the following error

        # Enable plugin
>       pm.enable(tmp_plugin.name)

napari/_qt/menus/_tests/test_plugins_menu.py:130:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../miniconda3/envs/napari-dev/lib/python3.9/site-packages/npe2/_plugin_manager.py:438: in enable
    self.events.enablement_changed({plugin_name}, {})
../../miniconda3/envs/napari-dev/lib/python3.9/site-packages/psygnal/_signal.py:967: in __call__
    return self.emit(  # type: ignore
../../miniconda3/envs/napari-dev/lib/python3.9/site-packages/psygnal/_signal.py:935: in emit
    self._run_emit_loop(args)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <SignalInstance 'enablement_changed' on <SignalGroup 'PluginManagerEvents' with 3 signals on <npe2._pytest_plugin.TestPluginManager object at 0x7f6076cd8910>>>, args = ({'tmp_plugin'}, {})

    def _run_emit_loop(self, args: tuple[Any, ...]) -> None:
        # allow receiver to query sender with Signal.current_emitter()
        with self._lock:
            with Signal._emitting(self):
                for caller in self._slots:
                    try:
                        caller.cb(args)
                    except Exception as e:
>                       raise EmitLoopError(
                            slot_repr=repr(caller), args=args, exc=e
                        ) from e
E                       psygnal._exceptions.EmitLoopError: calling <psygnal._weak_callback._StrongFunction object at 0x7f606b2f5f10> with args=({'tmp_plugin'}, {}) caused KeyError: "command 'tmp_plugin.widget1' not registered".

../../miniconda3/envs/napari-dev/lib/python3.9/site-packages/psygnal/_signal.py:982: EmitLoopError

[lucyleeow]

cc @DragaDoncila

[lucyleeow]

The contribution is first registered via the fixture tmp_plugin.contribute.widget, via ContributionDecorator.__call__. The resulting CommandRegistry item for the above widget contribution looks something like this:

{'tmp_plugin.widget2': CommandHandler(id='tmp_plugin.widget2', function=<function test_plugin.<locals>.widget2 at 0x7f17c327d040>, python_name=None)}

Note that python_name is None.

On disable, commands belonging to this plugin are removed from the CommandRegistry. When we enable the plugin we run on_plugin_enablement_change which is connected to event enablement_changed in _initialize_plugins. This runs _register_manifest_actions, which for widget plugins runs:

1. get_widget_contribution which calls:
2. WidgetContribution.get_callable, which calls
3. Executable.get_callable, which calls
4. CommandRegistry.get

CommandRegistry.get looks in the plugin managers CommandRegistry (pm._contrib._commands) and does not find the widget command (because it has been removed during 'disable') so it does PluginManager.activate, which calls CommandRegistry.register_manifest. register_manifest will only register contributions where the python_name is truthy, so it does not get registered. Thus back in CommandRegistry.get , we cannot find the command and raise a KeyError.

Note it seems that for all types of contributions added via DynamicPlugin fixture, the command is not re-added to the CommandRegistry when you 'disable' then 'enable'

[lucyleeow]

I think we've sorted out what is happening now.

Some background; PluginManager keeps track of contributions in the following 2 attributes:

• _contrib, a _ContributionsIndex - gets updated when index_contributions is run, which happens on plugin registration and enable.
• commands (_command_registry), a CommandRegistry - gets updated when activate is run. This happens when we get the callable (i.e. resolving the python_name) for a command id in CommandRegistry.get, i.e., when a command is actually called. All the commands for a plugin are added to the CommandRegistry when this happens.

The original error was because we were getting the callable (in get_widget_contribution) when creating the plugins menu - which we should not be doing, we want to only get callables when we run the command. We were getting the callable too early because we originally thought we had to do more complicated return annotation for the widget action callback for processors to work. This is not the case so we can fix it such that we never get a callable before we want to run it.

The problem remains though that you will not be able to execute a command added by a DynamicPlugin ContributionDecorator after 'disable' then 'enable'. Initial registration works because the decorator passes the defined function to CommandRegistry.register (instead of a resolve-able python name string). However, disable removes commands from both _ContributionsIndex and CommandRegistry. On enable, commands are added back to _ContributionsIndex but not added back to CommandRegistry on activate. This is because activate calls CommandRegistry.register_manifest which will only register contributions where the python_name is truthy, and since python_name for our contribution is None, no DynamicPlugin decorator added contributions get added back, and we cannot execute any commands.

This is not ideal and prevents proper testing, even though unfortunately we currently never test that enable works properly.

We may be able to fix this by over-writing the activate function.