Npe2 enable/disable support

[tlambert03]

Description

companion PR to napari/npe2#101

turns the enable/disable checkbox back on for npe2 plugins. (will require the next version of npe2)

This PR includes:

• updates to the qt_plugin_dialog to enable the checkboxes on npe2 plugins
• slight change to the way plugins are initialized in plugins/__init__ (to no longer "disabled" npe2 plugins)
• a better way to skip npe2 plugins during discovery (iter_available) in the old plugins/_plugin_manager.py
• an improved semver Version class (pretty much vendored from npe2), could potentially depend on that later and remove here)
• new logic to "migrate" a napari settings object from an old schema version to a new schema version. This will likely be broadly useful eventually as we update the settings. Here, it is used to remove npe2 plugins from the "disabled plugins" only once (during migration from 0.3.0 to 0.4.0).
• tests for the above

Type of change

• Bug-fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

References

How has this been tested?

• example: the test suite for my feature covers cases x, y, and z
• example: all tests pass with my change

Final checklist:

• My PR is the minimum possible work for the desired functionality
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• If I included new strings, I have used trans. to make them localizable.
  For more information see our translations guide.

[nclack]

Thanks!

One question - what determines the initial enable/disable status? napari_svg was disabled the first time I spun this up and I'm not sure why. It made me wonder about the interaction with settings.

[tlambert03]

One question - what determines the initial enable/disable status? napari_svg was disabled the first time I spun this up and I'm not sure why. It made me wonder about the interaction with settings.

this is an unfortunate consequence of the workaround we previously used, wherein we "set to blocked" all plugins that came from npe2, so that npe1 didn't detect them. Our persistence layer doesn't discriminate, and happily saved that preference. This PR contains a better way to skip npe2 plugin discovery by the npe1 plugin engine, but we'll have to figure out how to deal with that previous decision now

[nclack]

This PR contains a better way to skip npe2 plugin discovery by the npe1 plugin engine, but we'll have to figure out how to deal with that previous decision now

Previously no one could disable their npe2 plugins so the very first time we come across them they should be enabled. We just need a way of detecting and migrating the plugin settings forward. It requires some thought but seems doable?

[tlambert03]

@nclack, this seems to be working now. before I push on, would be good to get your thoughts. this adds a new disabled_plugins_npe2 field to the settings, and will basically just always remove a field from disabled_plugins if it is detected to be an npe2 plugin on this line here. (but will not modify anything it finds in settings.disabled_plugins_npe2). so basically, the first time a plugin transitions to npe2, it will be re-enabled until they disable it again as an npe2 plugin

[nclack]

This works but is there a way to avoid having two disabled_plugins fields? It looks like you needed to keep two separate lists for the two plugin engines here. Is that right?

[tlambert03]

yeah, that's what I ended up using. If you can think of a way to keep a single field, that'd be great! :) but it wasn't clear to me how we would determine whether something we found in that one list was an npe2 plugin that someone had intended to disable, or an npe2 plugin that had ended up in that list as a side effect of not wanting the npe1 plugin manager to pay attention to it

[tlambert03]

(I guess we could encode it in the string itself (napari-svg[npe2] ... but it feels like we'd need to deal with that in a lot of places... forever 😂)

[tlambert03]

@nclack have another look now. After our chat, seemed like the main way to accomplish this was to introduce the concept of migrations (in this case, a migration would just remove any npe2 plugins from the set of disabled_plugins). Would appreciate a somewhat close look here to make sure I'm on the right track, and happy to zoom about it again

[nclack]

looks good to me. I love the way migrations are done. There should probably be a test though.

[nclack]

We discussed this in the plugin working group. I think this is fine but we may want to follow up to address backward migrations. Migrations aren't really the point of this pr - the approach here is a good start.

I tested running main after upgrading the settings w this branch. It predictably complains the first time - the message is printed to console, not an exception.

> napari -vv                                                                                                                                                                                                                 Validation errors in config file(s).
The following fields have been reset to the default value:

schema_version
  value is not a valid tuple (type=type_error.tuple)

Second run there's no message. I think this is acceptable. It works for the problem addressed here, but more complicated migrations will need a proper undo step imo.

[codecov]

Codecov Report

Merging #4086 (02f2b7e) into main (0338649) will increase coverage by 0.00%.
The diff coverage is 84.87%.

@@           Coverage Diff            @@
##             main    #4086    +/-   ##
========================================
  Coverage   84.13%   84.13%            
========================================
  Files         596      598     +2     
  Lines       49648    49822   +174     
========================================
+ Hits        41770    41918   +148     
- Misses       7878     7904    +26     

Impacted Files	Coverage Δ
napari/plugins/_npe2.py	89.21% <9.09%> (-9.69%)	⬇️
napari/_qt/dialogs/qt_plugin_dialog.py	65.55% <46.66%> (-0.50%)	⬇️
napari/viewer.py	88.00% <75.00%> (-1.37%)	⬇️
napari/plugins/__init__.py	83.87% <78.26%> (+3.87%)	⬆️
napari/settings/_fields.py	90.21% <86.79%> (+29.65%)	⬆️
napari/settings/_tests/test_migrations.py	93.47% <93.47%> (ø)
napari/settings/_migrations.py	95.91% <95.91%> (ø)
napari/plugins/_plugin_manager.py	80.76% <100.00%> (-5.00%)	⬇️
napari/settings/__init__.py	95.45% <100.00%> (ø)
napari/settings/_base.py	91.38% <100.00%> (+0.12%)	⬆️
... and 9 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0338649...02f2b7e. Read the comment docs.

[tlambert03]

@sofroniewn and @jni, just wanted to draw your attention to one of the things being added in this PR, and that is the "new migration logic" mentioned in the original post (this file). It's minimal at this point, but it provides a mechanism for us to update/migrate the napari settings.yaml file as we evolve the settings schema. In general, it should be silent, and will only run once when a user updates napari to a new version that has modifies the settings schema. I tried to make it such that in the worst case scenario, it fails (and reverts back to the last working settings) with a warning that the user might need to manually call napari --reset.

[tlambert03]

otherwise, this PR is ready to go, and is required for #4015

[sofroniewn]

LGTM, let's get this in - feel free to merge when ready @tlambert03 or i'll merge it tomorrow if still open

[sofroniewn]

It's minimal at this point, but it provides a mechanism for us to update/migrate the napari settings.yaml file as we evolve the settings schema. In general, it should be silent, and will only run once when a user updates napari to a new version that has modifies the settings schema.

exciting!!