Context-aware plugin relevancy

[kecnry]

Description

This pull request implements infrastructure for plugins to determine when they are relevant/irrelevant based on the state of the application as a whole (data, viewers, units, etc). When a plugin determines itself to be irrelevant, then:

• the plugin itself will not appear in the tray, or will be removed if it already existed
• the plugin user API will no longer be listed in viz.plugins
• any existing UI instances (non-tray) of the plugin are replaced with the specified message (in the same way as the disabled message)
• TBD: any existing user API instances currently will continue to function. Should these instead raise an error or have the message in the repr?
• the plugin will be considered not "active" meaning any live preview and auto-updating logic should be paused, even if UI instances remain open

This is distinct from the existing "disabled" capabilities which only replaces the content with a message, but leaves the plugin in the list of options. There are cases where that might be less intrusive (change in units, etc), whereas this is designed more for the use-case of a more freeform configuration. For example: lcviz should only show the slice plugin if there is cube-like data (and this could then allow us to consider merging cubeviz, specviz2d, and specviz down the road).

Change log entry

• Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
  list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
  should add a no-changelog-entry-needed label.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

• Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
• Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
• Do the proposed changes follow the STScI Style Guides?
• Are tests added/updated as required? If so, do they follow the STScI Style Guides?
• Are docs added/updated as required? If so, do they follow the STScI Style Guides?
• Did the CI pass? If not, are the failures related?
• Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone. Bugfix milestone also needs an accompanying backport label.
• After merge, any internal documentations need updating (e.g., JIRA, Innerspace)?

[pllim]

Would this also be useful for downstream to configure their own setup like in #2696 ?

[kecnry]

Sure, I wasn't planning to make this public API (since once you make it irrelevant, there is no longer public API access), but the code snippet I put in the issue would definitely be simplified by this without having to do a list comprehension. Something like:

cubeviz.plugins['Slice']._obj.irrelevant_msg = 'anything here will get rid of it'

[kecnry]

Re-opening this for consideration now that we'll need this downstream for lcviz soon and also would be useful for dev work on the DQ plugin.

[codecov]

Codecov Report

Attention: Patch coverage is 54.54545% with 5 lines in your changes are missing coverage. Please review.

Project coverage is 88.70%. Comparing base (5afa49f) to head (9edd445).
Report is 6 commits behind head on main.

❗ Current head 9edd445 differs from pull request most recent head 12de8a1. Consider uploading reports for the commit 12de8a1 to get more accurate results

Files	Patch %	Lines
jdaviz/core/template_mixin.py	54.54%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2711      +/-   ##
==========================================
+ Coverage   88.67%   88.70%   +0.03%     
==========================================
  Files         108      108              
  Lines       16266    16305      +39     
==========================================
+ Hits        14424    14464      +40     
+ Misses       1842     1841       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pllim]

Maybe add explanation on when devs should consider something "irrelevant" or not. Do we still need "disabled" then? What are the differences when devs decide which one to use?

[bmorris3]

The distinction with disabled is meaningful. "irrelevant" extends upon the “disabled” plugin feature, which marks a plugin as disabled with a message in the UI, but does not “remove” it from the plugin tray or API. "irrelevant" additionally removes plugins from the UI and public API. The plugin won’t be visible in viz.plugins. But if you already assigned the plugin user API to a variable, that will still work.

[pllim]

So once it is marked as irrelevant, can it become relevant again later in the same session? I think this was discussed before but I don't remember and none of those discussions are documented in the diff.

Is the only use for this to hide a WIP plugin?

[bmorris3]

Is the only use for this to hide a WIP plugin?

It can have more general uses. We could also use it to put plugins away depending on the state of the app or viewer.

[kecnry]

This is a traitlet, so we can respond to anything we want to toggle its "relevancy", yes. Disabled leaves the plugin in the tray but just replaces the UI with a message (which is useful for cases where you want to tell the user WHY something disappeared). Imagine merging specviz and cubeviz, its obvious that the slice plugin is irrelevant without cube data, but hiding spectral analysis because of a unit conversion switch probably deserves showing a message. We can clarify a pattern for when to use each as they are used more often - and the under-the-hood implementation is very similar, so should be easy to migrate from one to the other when warranted.

[kecnry]

(I'm not really sure where to document this in the docs and I think it'll be a bit liquid for a while until we get a clear idea of how we want to use this... for now, there at least is a good papertrail in this discussion - and the PR description, so thanks for brining it up)

[pllim]

Can we clarify the pattern now as you envisioned and clean it up later? We cannot guarantee new devs would be able to dig up this conversation here. A few sentences in the dev docs at least would be searchable in RTD.

[kecnry]

ok, done

[bmorris3]

Looks good enough that I can't find a comment that is_relevant.

[pllim]

Thanks! I guess we can clean this up if we find bugs when actually trying to use it.