Proof of Concept approach for plugins providing actions

[sofroniewn]

Here is a potential approach for allowing plugins to providing actions such as keybindings that could then be registered by the action manager that @tlambert03 and I came up with. The big goal was to remove to explicit calls to viewer.bind_key and the need for the plugin to clean up its own keybindings and have the "bind" and "unbind" be done by napari through the actions manager, so the shortcut can be changed etc.

The main idea is that there is a new hook_specification napari_register_actions that allows a list of actions to be passed. This approach was a little more inspired by how we used to think about providing defaults with the actions themselves, but we should really follow the APIs that @Carreau has already been developing and using in napari, but now just be able to pass plugin dock widgets to these functions and register shortcuts to them.

What we wrote could look something like this

from napari_animation._qt import AnimationWidget


@napari_hook_implementation
def napari_register_actions():
    return [
        (AnimationWidget, "Alt-f", AnimationWidget._capture_keyframe_callback)
        (AnimationWidget, "Alt-r", AnimationWidget._replace_keyframe_callback)
        (AnimationWidget, "Alt-d", AnimationWidget._capture_keyframe_callback)
        (AnimationWidget, "Alt-a", lambda w: w.animation.key_frames.select_next())
        (AnimationWidget, "Alt-b", lambda w: w.animation.key_frames.select_previous())
        # ('napari.Viewer', 'alt-f', viewer_function) # To register action to viewer (not used in this repo, but illustrative)
        # ('napari.layers.Points', 'alt-p', points_function) # To register action to points layer (not used in this repo, but illustrative)
    ]

We will need to think about how napari can receive these actions, and how/ when they will get called, but that is on the napari side, not the plugin side.

Curious what @Carreau thinks about this. Might probably be good to have a dedicated meeting with @Carreau @ppwadhwa @goanpeca too to discuss this as we start thinking about it more!! cc @jni @alisterburt

[alisterburt]

this looks really interesting, definitely a goal we want to push for!
I'm not sure why some of the actions are lambda functions whilst others are the callback method itself - is this a bind to the instance vs. class thing?

[sofroniewn]

I'm not sure why some of the actions are lambda functions whilst others are the callback method itself - is this a bind to the instance vs. class thing?

It was just using what was there before, wasn't thought out beyond that. We could standardize and make it a little more readable

[tlambert03]

to expand on that a bit:
we're imagining that the third item in the list is a callback that will get called with an instance of the first item in the list. Calling AnimationWidget._capture_keyframe_callback(widget_instance) is the same thing as calling widget_instance._capture_keyframe_callback(), so it could have been expressed either using the unbound method, or using a lambda function: lambda w: w._capture_keyframe_callback().
The last two methods (select_next and select_previous) do not, however, accept an argument, so a lambda is necessary to "consume" the argument that the action manager is going to provide. (but, they could have all been expressed with a lambda too ... perhaps it would have been clearer, but this is also educational :)

[alisterburt]

very useful @tlambert03 - made me realise a few things, thanks!