Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When should the NapariApplication be instantiated and its actions registered? #6778

Open
DragaDoncila opened this issue Mar 27, 2024 · 4 comments
Labels

Comments

@DragaDoncila
Copy link
Contributor

DragaDoncila commented Mar 27, 2024

🧰 Task

When is app created?

When we initialize a napari.Viewer, we must also instantiate the NapariApplication object (which is our app-model app). The application is created when the get_app class method is first called.

Currently, this first call happens during execution of the intialize_plugins function, which is called first thing on Viewer.__init__ and registers npe2 plugin actions.

Is it strange that the NapariApplication, which is so critical to the Viewer and its function, is created in this little function rather than explicitly before we register manifest actions etc.? It seemed to @lucyleeow and I that it was a little hidden and potentially could lead to issues? So wanted to open for discussion.

When are actions registered?*

* I say actions here for brevity but these steps also include registering providers and processors.

  • Internal non-qt actions: as soon as we build the app, on NapariApplication.__init__. This seems normal to me.
  • Internal qt actions: on QtMainWindow.__init__. Also seems normal to me I guess - not sure if there's meaning to moving it up or down in the function.
  • Non-qt plugin actions: on initialize_plugins as above (which is up first on Viewer.__init__). This means it happens after internal non-qt actions and before internal/plugin qt actions. I think this is also ok.
  • Qt plugin actions: immediately after non-qt plugin actions, if qt is available. This means it's before qt actions, providers and processors are registered with the viewer, even though some plugin actions (if executed) may request the Viewer provider. @lucyleeow and I thought maybe this was kinda weird, and we should potentially consider splitting this out so that qt plugin actions are only registered after init_qactions?

Just a final note: in practice none of this seems to have caused any issues, and it may never. Everything is registered and available by the time the Viewer is returned. We just wanted to explicitly consider whether this is the right order of operations and whether there might be any potentially unintended consequences of the current setup down the line.

@lucyleeow
Copy link
Contributor

Thinking about this, where else do we get the app via get_app (which instantiates NapariApplication if one does not already exsit)? It would be good to decouple init of the app to plugins. I recall we were interested in being able to use napari standalone, without any plugins (napari-lite?).

@jni
Copy link
Member

jni commented Mar 28, 2024

Is it strange that the NapariApplication, which is so critical to the Viewer and its function, is created in this little function rather than explicitly before we register manifest actions etc.?

From my reading of the get_app classmethod, I think this is fine — it's written to work "just in time" so that it either returns the napari application if it exists, or it creates it otherwise. This is written this way specifically so that we don't have to plan when it gets instantiated, it just gets instantiated on demand the first time it's needed. If that happens to be in initialize_plugins, so be it! And:

It would be good to decouple init of the app to plugins. I recall we were interested in being able to use napari standalone, without any plugins (napari-lite?).

If at some point we want to implement a plugin-less app, we just need to call get_app where it is needed, and that might happen before plugin init, and that's fine, at that point the plugin init get_app will just get the already-created application.

immediately after non-qt plugin actions, if qt is available. This means it's before qt actions, providers and processors are registered with the viewer, even though some plugin actions (if executed) may request the Viewer provider. @lucyleeow and I thought maybe this was kinda weird, and we should potentially consider splitting this out so that qt plugin actions are only registered after init_qactions?

this is definitely a bit weird, but I don't know enough about the architecture to comment intelligently about whether it's worth rejiggering it.

@DragaDoncila
Copy link
Contributor Author

whether it's worth rejiggering it.

Rejigging it would be trivial, we'd just move the safe_register_qt_actions function call out of where it currently is and below init_qactions. The downside to it, I guess, would be that plugin stuff gets registered now in two different spots, rather than in the one nice place. I don't see that as a super big downside, because at least this way there's a more explicit split between plugin actions that are qt dependent and ones that aren't. But I don't have very strong feelings - @lucyleeow?

@lucyleeow
Copy link
Contributor

From my reading of the get_app classmethod, I think this is fine — it's written to work "just in time" so that it either returns the napari application if it exists, or it creates it otherwise.

Yeah this seems reasonable. I checked and if we don't init plugins, the app would be first instantiated when building the menu bar (in qt main window) (just FYI, this doesn't change anything)

Rejigging it would be trivial, we'd just move the safe_register_qt_actions function call out of where it currently is and below init_qactions

No strong opinion either but I think it makes sense to move it. We make an effort to separate non-qt and qt napari actions so we may as well do the same for plugins. It would also avoid us having to use the safe register of plugin qt actions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants