Document command ID naming conventions #405
Conversation
|
|
||
| * use `.` to separate words | ||
| * start with `napari` for builtin commands or the plugin name for plugin commands | ||
| * `napari.window` prefix indicates the `napari` command is part of a GUI menu |
There was a problem hiding this comment.
Is this only for napari commands? We don't have this convention for plugins, or should we?
There was a problem hiding this comment.
Also I noticed that our layer actions do not have window but they are part of a menu
There was a problem hiding this comment.
| * `napari.window` prefix indicates the `napari` command is part of a GUI menu | |
| * the `napari.window` prefix indicates the `napari` command is part of a GUI menu |
There was a problem hiding this comment.
I don't think anyone has thought about it deeply before (?), so we can discuss. Maybe it's meant to indicate a menubar menu item?
I'm also not sure why command ids aren't just package:fully.qualified.python.name?
There was a problem hiding this comment.
Honestly I am just following what was left by Talley.
Sometimes we use the same callback, e.g., a lot of help menu items just open a URL. I guess in that case we can append an extra word.
I am not opposed to fully qualified python name:
- It would probably be equally long as current system - (which is fine though shorter would be nice just formatting style wise)
- It would follow plugin naming convention
- No extra info about where this command is (i.e., which menu)
- Is it slightly confusing c.f. the plugin manifest
python_namewhich uses:before the final function name?
There was a problem hiding this comment.
I don't agree with fully qualifying the python name. If we move forward with allowing actions to take kwargs, then multiple actions might share the same name. Similarly we sometimes wrap actions in napari stuff, and we may not have a fully qualified python name. I much prefer having the command ID give me a sense of where the command lives in the viewer, rather than in the code.
There was a problem hiding this comment.
I will say I don't love napari.window either. I think it should be napari.menus.menu for stuff that lives in a menu (including a context menu I guess?) and then just napari.callback_name for stuff that doesn't live in menus. I think everything does now maybe but longer term
There was a problem hiding this comment.
I guess the only pro for python name is that it would follow what is happening in plugin manifests, though maybe we want to update that? With NAP-6, maybe we want the command ID to indicate where in the menu the action will live?
There was a problem hiding this comment.
Meh I mean I super don't care if we have different convention internally vs. for plugins tbh. And also, for plugins, cant the make their command ID whatever they want anyway? As long as it starts with their plugin name?
@Czaki I'm not sure what you mean sorry, can you expand? |
|
I think @Czaki is responding to my suggestion that IDs should be a fully qualified Python path. That ties the IDs to implementation details, which is indeed problematic. So I'm open to scrapping that idea and coming up with a different system. Any suggestions? |
Do you not like the current system, which I think is like this:
? |
|
The PR is documenting the current state of things so we think it's worth it going in as is. @jni and I both think the broader discussion about what we want the convention to be warrants discussion, but we will open an issue in |
|
Sounds OK to me, thanks! |
References and relevant issues
Follow up from: napari/napari#6777 (review)
Description
Naming conventions.
Note the
windowprefix was noted by Talley: https://github.com/napari/napari/pull/4826/files#r954915004Open to discussion and changes.