NAP-6: Updates after discussion and review #312
Conversation
Co-authored-by: alisterburt <alisterburt@gmail.com>
github-actions
bot
added
the
documentation
docs/naps/6-contributable-menus.md
Outdated
| to a much greater extent. This should include contribution-level enablement control | ||
| for plugins, as well as building their own custom menus, whether this be a brand new top-level |
There was a problem hiding this comment.
Naive question, how would users control plugin contribution enablement..?
There was a problem hiding this comment.
I imagine, a tree selection widget like this one:
I think there should be some internal npe2 data structure for this, but the UI itself should itself be a plugin. (Maybe built-ins.)
There was a problem hiding this comment.
Would this be separate to our plugin manager (i.e. the install/uninstall/enable/disable dialog)?
(plugin manager has several meanings in napari: #321 (comment) 🙈 )
There was a problem hiding this comment.
I'm ambivalent about it, and would leave that decision to whoever ends up implementing it.
On the one hand, it can be an independent dialog — enablement of contributions only needs npe2 and stuff already installed, and doesn't need to depend on conda, pip, etc. So you eliminate a lot of complexity. Additionally, I personally would never use the plugin manager (I directly install things with micromamba as needed), but I would probably use the enablement UI because the alternative, I think, is editing some random yaml or something? 😂 Which I'm not super excited about.
On the other hand, I can imagine a user being in the enablement UI and thinking hmm, I really don't use this plugin, I should remove it. And they would probably be grateful if that can be done in the same widget rather than have to launch a totally different interface.
|
Ok so I finally read NAP-6. Better late than never???? 😅😅😅😅😅 Thank you @DragaDoncila for your patience with me. 😅 And thanks for writing the original NAP and for this update! Here are my current comments:
That's it from me! It's not a tiny list but I think this is very close. ❤️ Thanks! |
I think this ties in with the wider discussion about what the manifest should look like: napari/napari#6227 With submenus in particular, you either have to give the full path (i.e. parent menu) every time you use it, or you have a submenu section that defines submenu and its parent, and then use the shorter name every time you use the submenu. With an updated manifest, having a submenu section may not look as bad? |
|
Thanks @lucyleeow and @jni I've made some updates would appreciate another pass. Also @jni some responses inline.
Yep you're not wrong, @psobolewskiPhD has brought this up too. I don't know what exactly the distinction should be there. I would look to other tools - I know Photoshop also has
Yeah I don't love it.. in a world where
I've added a couple more examples. You're welcome to suggest your own 😛 Otherwise definitely let me know which ones you can't think of an example for. W.r.t the visualization one specifically, Lorenzo and I did have this discussion.
Fixed, I like the shorter imperatives as well. I think I had it like that at one stage but then changed it...
I think I/O at top level is pretty strange compared to
Added a sentence!
🙈
Boo 😜 Still need to do this and update the TODOs. |
But Photoshop doesn't seem to have a Layer > Edit menu. (screenshot from the web.) Reading the example text, "The items in this submenu allow you to change the data of your layer", I would consider renaming this to Layer > Modify, or Layer > In-Place > ... I would also say "to change the data and metadata of your layer". Finally, I'm quite apprehensive about supporting/encouraging in-place filters at all. I might suggest that we should remove filters from the example and focus instead on metadata...
My gut feeling is that it is worth updating the schema. I'm not 100% sure about Lucy's idea that this would imply always typing in the full menu path. I think it's possible to say that if the tail of the ID is unique, you can just use that ID; if there is ambiguity, you must use the full path. So, you'd have to write "layer/edit", but "segment" would automatically point to with "layer/segment". And also the parent path would be implied in the Honestly, I can easily imagine that this approach could devolve into a mess, but my gut feeling is that it's worth trying it out.
I know we don't have auto-actions/purely functional inputs, but how far are those? Can we use those examples in the NAP? I would like something like "Another example would be a function that automatically sets the contrast limits based on properties of the data". And toggle visibility surely would be in this menu?
I'm trying to say that "Acquire" is a subset of "I/O". So maybe File > Acquire, File > Load? Anyway, a side benefit of my menus proposal above is that people can just contribute to "acquire" and we can actually move acquire around and everything would Just Work! Getting there! |
Are you talking about over hauling the manifest template and addressing napari/napari#6227 ?
My bad, I was already thinking in terms of a 'new' manifest schema where for each command you list all its 'contribution' types. At least this is format we think we want the manifest data to be represented as, internally in napari, from discussions with @DragaDoncila. Something like: |
Co-authored-by: Lucy Liu <jliu176@gmail.com>
Co-authored-by: Lucy Liu <jliu176@gmail.com>
Sure yeah, I guess we need to take a closer look at what exactly the separation is that they've made, and see where it makes sense for us, and maybe add the
I think people will absolutely want to be editing their data and their metadata potentially so idk, unless you want to make some blanket rule like "every action returns a new layer" I'd rather have a dedicated spot for that stuff to go...
I'm open to that with the caveat that it will delay things by potentially some weeks. I've uploaded a tentative new manifest here for us to have a look at and make sure we're talking about the same thing. In this one, as @lucyleeow has mentioned, the menus are attached directly to their commands, but we keep a
You know nothing, Jon Snow. Functional inputs are totally possible, and if they return layer type things those get added to the viewer, and if they request layer type things those get provided afaik. The
I've added that example, and I've expanded the section about 'what do menu contributions do' to talk about them.
Unless it goes in top level edit?
Let's go with
This I love, and I've kept it in the proposed manifest above. |
Just realized it's going to be messy having the keys be just the menu name with absolutely no prefix, because of potential clashes with other plugin submenus etc. So I think it should be |
We control the contributable menus, so we can make sure there are no name collisions there. We can also decide on a resolution order or raising if a name clashes and is not fully qualified. So, if napari-special has an edit menu, they should refer to it as napari-special/edit, or it will end up in our own menu. |
Yeah, but I think explicit is better than implicit, and it's nice to have the menus either start with |
no, it is a nightmare to type "napari" or the plugin name umpteen times. That is one of my manifest gripes, you'll recall, and moving it from command-id to the menus is really poor consolation... If we can come up with a shortcut for the plugin's own menu, that would be fine — so it's either nothing (napari contributable), or
I would find that super weird. Maybe a View top level menu? Also, we could/should rename Layer > Edit to Layer > Modify. BUT ALSO,
We don't have such a blanket rule and some widgets might want to have a toggle for in-place processing. So I actually don't think that they should all go in one place. For example, scikit-image morphology remove_small_objects could work both in place or return a copy, and users should be able to decide. I don't think these should be two separate commands. |
but are they possible from plugins??? |
yes!!! 😜 |
Yeah I think for widgets it's a bit different because I feel the user will probably be making choices and clicking buttons of their own volition. With commands I'm worried, because if someone clicks on something and expects a new layer but it actually edits the layer, that's a problem - especially since we don't have undo. So for me that was kinda the guideline - put stuff in this edit menu and people should expect that it works on the layer data. But that's where I think we need to have more specific contribution types that guarantee things to the user about what's going to happen once they click this button. Happy to think of other ways to organize them, with the key goal being I don't want users to experience trauma at the hands of an overzealous command. |
That's exactly what I was thinking. Since there are other sections (sample URI, theme) as well as submenu, I don't mind there being a submenu section. Edit - in-place vs new layer
and
@jni when you say these shouldn't be two separate commands, do you mean that this would be one (contributable) menu item but there would be a pop-up asking what the user wants to do (in-place to return new layer?). Or do you think there should be 2 menu items? How do you see this working? Menu paths - full vs partialFirst, I have forgotten, but have we decided if a plugin should be able to contribute to another plugins menu?
@jni is it really that bad, and it would be for a few submenu items? I think a plugin should be able to specify which menu an item goes into, which I don't think will be fixed by having a conflict resolution order. How do you propose a plugin be able to specify whether an item goes into napari menu or it's own menu, or even another plugins menu? |
Well, for this specific example there is an extra parameter (the minimum object size) needed, so you will need a widget one way or another — and that widget would have a boolean "in-place" that would be off by default. For cases where you only need a boolean you could instead present two buttons "run in-place" "run and create new layer".
ie, we make napari a privileged set of menus, everything else must be specified, but with a shortcut for self. |
Two buttons - are you talking about when the action is a widget? Is it possible that the action is just a menu item? In that case should there be two menu items? |
|
I'm envisioning that in the future, if, say, a command requires an integer, selecting that command from the menu will bring up a short-lived pop-up to select the value, then you click "run" and the dialog disappears and the command runs, rather than a full-on docked widget, which is more suited to long-lasting interactions, rather than single runs of a thing. (The converse could also happen: don't use a dialog, dock the widget, but by default hitting "run" disappears the widget.) In this world, having a single tick-box and a run button is not as good as having two buttons, "run ticked" and "run unticked". |
I think I prefer the dialog, which is what I was thinking above.
I this world - in the world where the menu item opens a docked widget? I may have missed it, but what did we decide about commands that can become a widget or be a menu item? |
in the the world where a menu item will open a (probably magicgui-generated) dialog if we can't automatically determine the value of all the arguments to the function from the current context.
I don't know that it's been decided, but the macOS convention is that you have an action normally and if you hold down Option the menu item changes from menu-opt.mp4I think currently most plugins are widgets one way or another, so we can just leave everything as a widget and work with the community to have most actions be contextual, and holding option for specifying parameters. Dock Widgets would be their own thing. |
Oh the buttons would be in the pop up dialog? That makes more sense |
|
Since this PR does not affect the NAP status, and we've reached something close to a stable state, I'm going to merge so the document can be in published form for (hopefully final) discussion and acceptance. 😊 Massive thanks to Draga for all the work so far and @lucyleeow for the discussion — which has helped make the document and our own brains clearer! 😊 |
# References and relevant issues # Description This PR is an alpha implementation of NAP-6. It adds all proposed menus from NAP-6 (see docs PR for [latest status](napari/docs#312)) and enables plugins to add commands and widgets to the proposed menus. Right now it's in pretty early stages, and not everything is implemented - ~~I'll be opening a tracking issue soon~~ see the [tracking issue](#7012). To try it out, you can just install from this branch, but most menus will be empty. I've added actions for the new layer buttons to `File -> New Layer`, but haven't yet populated other built-in stuff. You can also run the script below to add actions on the fly. Finally, you can[ install this branch](https://github.com/DragaDoncila/napari-demo/tree/menu-contrib) of `napari-demo` to see a plugin declare some menus. ```python import napari from napari._app_model import get_app from app_model.types import Action viewer = napari.Viewer() def large_points() -> napari.layers.Points: return napari.layers.Points(size=40) actions = [ Action( id='napari-example.merge_labels', title='Merge Labels... (napari-example)', callback=lambda: napari.utils.notifications.show_info("Not implemented..."), menus=[{'id': 'napari/layers/annotate'}] ), Action( id='napari-example.large_points', title='Large Points (napari-example)', callback=large_points, menus=[{'id': 'napari/file/new_layer'}] ) ] app = get_app() app.register_actions(actions) napari.run() ``` @jni thinks that this is ~ready for an alpha rc, but there's still no tests or docs really yet. --------- Co-authored-by: Juan Nunez-Iglesias <jni@fastmail.com> Co-authored-by: napari-bot <napari-bot@users.noreply.github.com>
Description
This PR updates NAP-6 after feedback on #77, discussion at the napari hackathon (notes here but also linked in doc), and discussions in community meetings in recent weeks.
Still to be decided
npe2allows users to define new submenus as well as menu items. Should plugin developers be allowed to add new submenus to contributable menus? Decision: yes, but not to the top levelLayersmenu, and the context menu shouldn't actually be contributable as it's already very long.TracksandClassificationsubmenus are as well defined as the rest. Classification at least I'm not even sure how to describe. Should we removeTracksandClassificationfrom the initial list of contributable menus? Decision: No, we like both!