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

META: Decouple plugin-manager from napari #59

Open
4 tasks
jaimergp opened this issue Jun 18, 2024 · 5 comments
Open
4 tasks

META: Decouple plugin-manager from napari #59

jaimergp opened this issue Jun 18, 2024 · 5 comments
Assignees

Comments

@jaimergp
Copy link
Contributor

jaimergp commented Jun 18, 2024

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?
@jaimergp jaimergp changed the title Decouple plugin-manager from napari META: Decouple plugin-manager from napari Jun 18, 2024
@dalthviz
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor

Czaki commented Oct 28, 2024

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
Copy link
Member

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

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
Copy link
Contributor Author

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.

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

No branches or pull requests

3 participants