META: Decouple plugin-manager from napari

[jaimergp]

Other dev teams have expressed interest in using our plugin manager in their ecosystem. To that end, we offered to provide a plugin-manager UI that is uncoupled from napari as much as possible. The end goal would be a napari-plugin-manager that depends and extends a hypothetical, application agnostic plugin-manager-dialog (name tbd).

To do:

These will become their own items as needed

• Investigate coupled areas and design API for plugin-manager-dialog Initial attempt to create base GUI classes #113
  • Abstract stylesheets in generic base class
  • Abstract plugin list sources in generic base class
  • Provide subclass for napari itself
• Assess code coverage and improve if necessary
• Refactor napari-plugin-manager into two packages (still in the same repo) following the design proposed above. These two steps can happen iteratively (implementation informs design and viceversa)
• Decide maintenance and release model: two repositories, fully independent? One repository, coupled releases? Single repo but uncoupled releases?

[dalthviz]

Gave this an initial check and from the UI part (widgets definitions under qt_plugin_dialog for the most part) got a list of methods that use/have napari related logic:

• Custom rendering for plugins items:
  • PluginListItem class: (setup_ui, _handle_npe2_plugin, _on_enabled_checkbox)
• Plugins list:
  • QPluginList class: (handle_action, tag_outdated, tag_unavailable)
• Plugins dialog:
  • QtPluginDialog class: (_fetch_available_plugins, _add_to_installed, _add_installed, _handle_yield, _update_theme, _setup_shortcuts, setup_ui, _on_installer_all_finished, _add_items)

Also, there are some widgets that are being used which come from napari like QtToolTipLabel. Maybe their usage/creation should be encapsualted on a method to override, right?

Some other general elements that probably should be encapsulated into a method to override/should be marked with something like NotImplementedError is the translations handling (when widgets display text it is being first passed to the napari trans._ function).

Could it make sense to start doing small PRs (like to encapsulate the translation logic or the QtToolTipLabel usage) or maybe I should give a more in depth check to the code first? Also, before start doing any big PR changing things, probably any pending major work related with the UI should be merged? Like #51? What do you think @jaimergp ?

[jaimergp]

Thank you for the analysis, Daniel!

I agree that ideally #51 would go in first, but maybe we can start small already, by proposing the base class + subclass split? If @goanpeca is not available this week, you can also push to #51 and finish it if you think that's easier.

[Czaki]

I do not see here any topic about generalization of plugin API server, like https://github.com/napari/npe2api

Did you plan to only document the REST API required by generalized plugin manager?

[dalthviz]

Thanks for the feedback! I would say in my initial idea a definition of what info is needed for the plugins will be done but I don't think that documentation would follow a REST API structure. I would say the work I was thinking of is going be closer to a well defined abstract class which has methods that need to be implemented.

Following that, I was thinking on a base class definition which requires the developer to implement several things like a way to query plugins when extending it (which in the case of the napari class would use the current logic using the npe2api web service to get the plugins info). To be more precise, related with the plugins info query, the base plugin dialog class would have a _fetch_available_plugins method raising a NotImplementedError. Then, the napari specific class which extends that class, would implement that method using the current logic at

napari-plugin-manager/napari_plugin_manager/qt_plugin_dialog.py

Lines 1101 to 1115 in dbcd362

	def _fetch_available_plugins(self, clear_cache: bool = False):
	get_settings()
	if clear_cache:
	cache_clear()
	self.worker = create_worker(iter_napari_plugin_info)
	self.worker.yielded.connect(self._handle_yield)
	self.worker.started.connect(self.working_indicator.show)
	self.worker.finished.connect(self.working_indicator.hide)
	self.worker.finished.connect(self._add_items_timer.start)
	self.worker.start()
	pm2 = npe2.PluginManager.instance()
	pm2.discover()

However, other implementations could extend the base class and make the query method to query things from a file or define a constant/variable with the info needed without the need of querying a web service.

I think that's the scope of this work (at least from the GUI part) but maybe I'm missing something? Maybe the scope here is also to define the API a web service that retrieves plugin info needs to follow to work like the npe2api service? Also, maybe creating a pluginapi kind of project is worthy then? 🤔

[jaimergp]

However, other implementations could extend the base class and make the query method to query things from a file or define a constant/variable with the info needed without the need of querying a web service.

Yep, that's what I had in mind too. napari happens to have the npe2api service (and we can document what we are doing there if someone wants to reuse the same approach for their stuff), but in principle users should define their own list population logic. In the future, if folks are really interested in the npe2api-like stuff, we could think of providing a 2nd level base class that streamlines the API querying but I think that's a second step, not the first one.

[dalthviz]

Continuing with the work here, I was checking how things would look dividing the code into two packages. However, while doing that, I noticed that the qt_package_installer module, although defines some level of abstraction, seems like has still some level of coupling with napari releated things (mostly around constrains definitions/file generation). Should I try to divide that module also into a base_qt_package_installer for the base clases and a qt_package_installer for napari related logic?

Also, thinking on the two packages division, there are a couple of PRs open which probably should be updated and merged before proceeding with the package division, right?

What do you think @jaimergp ?