Bugfix: Fix regression from #5739 for passing plugin name and reader plus add test

[psobolewskiPhD]

Fixes/Closes

Issue with 0.4.18rc2 reported on zulip by @adamltyson
https://napari.zulipchat.com/#narrow/stream/212875-general/topic/reader.20plugin.20issue.20with.200.2E4.2E18

Closes #6012

Description

In merged commit: 6847eee
the method of checking whether a plugin is npe2 returns None if a reader is passed in addition to the plugin name, e.g. my-plugin.some_reader vs. my-plugin
This is a regression for the case of plugins with multiple readers, as reported on zulip:
https://napari.zulipchat.com/#narrow/stream/212875-general/topic/reader.20plugin.20issue.20with.200.2E4.2E18

This PR just changes the logic of the comparison so use startswith so just the plugin name is compared, rather than the entire string. This fixes the regression.
Also, I add a test for this condition that fails on main, but passes with this fix.

References

See: https://napari.zulipchat.com/#narrow/stream/212875-general/topic/reader.20plugin.20issue.20with.200.2E4.2E18

Type of change

• Bug-fix (non-breaking change which fixes an issue)

How has this been tested?

• example: added a test that passes now, but fails on main

Final checklist:

• My PR is the minimum possible work for the desired functionality

[codecov]

Codecov Report

Merging #6013 (158e453) into main (49e235a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 158e453 differs from pull request most recent head c7a8e34. Consider uploading reports for the commit c7a8e34 to get more accurate results

@@           Coverage Diff           @@
##             main    #6013   +/-   ##
=======================================
  Coverage   91.56%   91.56%           
=======================================
  Files         573      573           
  Lines       49809    49813    +4     
=======================================
+ Hits        45606    45612    +6     
+ Misses       4203     4201    -2     

Impacted Files	Coverage Δ
napari/plugins/_npe2.py	91.83% <100.00%> (ø)
napari/plugins/_tests/test_npe2.py	100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

[Czaki]

Keeping npe1_... names in code make it harder to read. But everything looks ok.

[DragaDoncila]

As mentioned in the comment, startswith is just not strong enough here. I'd like us to have an explicit check

[DragaDoncila]

This is super danger zone and is going to lead to the same issues as were fixed in napari/npe2#297. I suggest we explicitly perform a split on the '.' here and check the full plugin name:

    # do nothing if `plugin` is not an npe2 reader
    if plugin:
        # user might have passed 'plugin.reader_contrib' as the command
        plugin_name = plugin.split('.')[0]
        if plugin_name not in get_readers():
            return None

I don't know if it should be our job to check down to the contribution level here - I've left that for npe2 to check - note that without napari/npe2#301, npe2 in this case may just use a different contribution... But I just don't really know if adding the check here is right...

[psobolewskiPhD]

Makes sense. I implemented your suggestion, just using partition('.') because it seems safer in case of no delimiter and we only care about whats before the . anyways?
Tests pass with npe2 0.7.0 and my test case with napari builtin works.

[DragaDoncila]

Thanks @psobolewskiPhD I've played with this all night and it seems to work like a charm!

[imagesc-bot]

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/error-with-brainreg-installation/89547/3

[Virginia9733]

Hi there, I think I encounted the same issue with:https://forum.image.sc/t/error-with-brainreg-installation/89547/3

ModuleNotFoundError Traceback (most recent call last)
File ~\miniconda3\envs\napari-env\lib\site-packages\npe2_command_registry.py:32, in CommandHandler.resolve(self=CommandHandler(id='brainreg.Register', function=...ame='brainreg.napari.register:brainreg_register'))
31 try:
---> 32 self.function = utils.import_python_name(self.python_name)
self.function = None
self.python_name = 'brainreg.napari.register:brainreg_register'
self = CommandHandler(id='brainreg.Register', function=None, python_name='brainreg.napari.register:brainreg_register')
utils = <module 'npe2.manifest.utils' from 'C:\Users\muhang\miniconda3\envs\napari-env\lib\site-packages\npe2\manifest\utils.py'>
33 except Exception as e:

File ~\miniconda3\envs\napari-env\lib\site-packages\npe2\manifest\utils.py:254, in import_python_name(python_name='brainreg.napari.register:brainreg_register')
252 module_name, funcname = match.groups() # type: ignore [union-attr]
--> 254 mod = import_module(module_name)
module_name = 'brainreg.napari.register'
255 return getattr(mod, funcname)

File ~\miniconda3\envs\napari-env\lib\importlib_init_.py:126, in import_module(name='brainreg.napari.register', package=None)
125 level += 1
--> 126 return _bootstrap._gcd_import(name[level:], package, level)
level = 0
name = 'brainreg.napari.register'
name[level:] = 'brainreg.napari.register'
package = None
_bootstrap = <module '_frozen_importlib' (frozen)>

File :1050, in _gcd_import(name='brainreg.napari.register', package=None, level=0)

File :1027, in find_and_load(name='brainreg.napari.register', import=)

File :1006, in find_and_load_unlocked(name='brainreg.napari.register', import=)

File :688, in _load_unlocked(spec=ModuleSpec(name='brainreg.napari.register', load...b\site-packages\brainreg\napari\register.py'))

File :883, in exec_module(self=<_frozen_importlib_external.SourceFileLoader object>, module=<module 'brainreg.napari.register' from 'C:\Use...b\site-packages\brainreg\napari\register.py'>)

File :241, in call_with_frames_removed(f=, *args=(<code object at 0x000001DCCA416810, fil...te-packages\brainreg\napari\register.py", line 1>, {'BrainGlobeAtlas': <class 'brainglobe_atlasapi.bg_atlas.BrainGlobeAtlas'>, 'Dict': typing.Dict, 'Enum': <enum 'Enum'>, 'List': typing.List, 'Tuple': typing.Tuple, 'builtins': {'ArithmeticError': <class 'ArithmeticError'>, 'AssertionError': <class 'AssertionError'>, 'AttributeError': <class 'AttributeError'>, 'BaseException': <class 'BaseException'>, 'BlockingIOError': <class 'BlockingIOError'>, 'BrokenPipeError': <class 'BrokenPipeError'>, 'BufferError': <class 'BufferError'>, 'BytesWarning': <class 'BytesWarning'>, 'ChildProcessError': <class 'ChildProcessError'>, 'ConnectionAbortedError': <class 'ConnectionAbortedError'>, ...}, 'cached': r'C:\Users\muhang\miniconda3\envs\napari-env\lib\s...inreg\napari_pycache\register.cpython-310.pyc', 'doc': None, 'file': r'C:\Users\muhang\miniconda3\envs\napari-env\lib\site-packages\brainreg\napari\register.py', 'loader': <_frozen_importlib_external.SourceFileLoader object>, ...}), **kwds={})

File ~\miniconda3\envs\napari-env\lib\site-packages\brainreg\napari\register.py:13
12 from brainglobe_atlasapi.list_atlases import descriptors, utils
---> 13 from brainglobe_napari_io.brainmapper.brainmapper_reader_dir import (
14 load_registration,
15 )
16 from fancylog import fancylog

ModuleNotFoundError: No module named 'brainglobe_napari_io'

The above exception was the direct cause of the following exception:

RuntimeError Traceback (most recent call last)
File ~\miniconda3\envs\napari-env\lib\site-packages\napari_qt\menus\plugins_menu.py:105, in PluginsMenu._add_plugin_actions.._add_toggle_widget(key=('brainreg', 'Atlas Registration'), hook_type='dock')
102 return
104 if hook_type == 'dock':
--> 105 self._win.add_plugin_dock_widget(*key)
key = ('brainreg', 'Atlas Registration')
self._win = <napari._qt.qt_main_window.Window object at 0x000001DCBFCE4BE0>
self = <napari._qt.menus.plugins_menu.PluginsMenu object at 0x000001DCC72B24D0>
106 else:
107 self._win._add_plugin_function_widget(*key)

File ~\miniconda3\envs\napari-env\lib\site-packages\napari_qt\qt_main_window.py:876, in Window.add_plugin_dock_widget(self=<napari._qt.qt_main_window.Window object>, plugin_name='brainreg', widget_name='Atlas Registration', tabify=False)
873 Widget = None
874 dock_kwargs = {}
--> 876 if result := _npe2.get_widget_contribution(plugin_name, widget_name):
widget_name = 'Atlas Registration'
plugin_name = 'brainreg'
_npe2 = <module 'napari.plugins._npe2' from 'C:\Users\muhang\miniconda3\envs\napari-env\lib\site-packages\napari\plugins\_npe2.py'>
877 Widget, widget_name = result
879 if Widget is None:

File ~\miniconda3\envs\napari-env\lib\site-packages\napari\plugins_npe2.py:136, in get_widget_contribution(plugin_name='brainreg', widget_name='Atlas Registration')
134 if contrib.plugin_name == plugin_name:
135 if not widget_name or contrib.display_name == widget_name:
--> 136 return contrib.get_callable(), contrib.display_name
contrib = WidgetContribution(command='brainreg.Register', display_name='Atlas Registration', autogenerate=False)
contrib.display_name = 'Atlas Registration'
137 widgets_seen.add(contrib.display_name)
138 if widget_name and widgets_seen:

File ~\miniconda3\envs\napari-env\lib\site-packages\npe2\manifest\contributions_widgets.py:49, in WidgetContribution.get_callable(self=WidgetContribution(command='brainreg.Register', ...ay_name='Atlas Registration', autogenerate=False), _registry=None)
46 def get_callable(
47 self, _registry: Optional[CommandRegistry] = None
48 ) -> Callable[..., Widget]:
---> 49 func = super().get_callable()
50 if self.autogenerate:
51 try:

File ~\miniconda3\envs\napari-env\lib\site-packages\npe2\manifest\utils.py:71, in Executable.get_callable(self=WidgetContribution(command='brainreg.Register', ...ay_name='Atlas Registration', autogenerate=False), _registry=<npe2._command_registry.CommandRegistry object>)
68 from npe2._plugin_manager import PluginManager
70 _registry = PluginManager.instance().commands
---> 71 return _registry.get(self.command)
_registry = <npe2._command_registry.CommandRegistry object at 0x000001DCBBCDBE80>
self.command = 'brainreg.Register'
self = WidgetContribution(command='brainreg.Register', display_name='Atlas Registration', autogenerate=False)

File ~\miniconda3\envs\napari-env\lib\site-packages\npe2_command_registry.py:138, in CommandRegistry.get(self=<npe2._command_registry.CommandRegistry object>, id='brainreg.Register')
136 if id not in self._commands: # sourcery skip
137 raise KeyError(f"command {id!r} not registered")
--> 138 return self._commands[id].resolve()
id = 'brainreg.Register'
self._commands = {'brainreg.Register': CommandHandler(id='brainreg.Register', function=None, python_name='brainreg.napari.register:brainreg_register'), 'brainreg.SampleData': CommandHandler(id='brainreg.SampleData', function=None, python_name='brainreg.napari.sample_data:load_test_brain')}
self = <npe2._command_registry.CommandRegistry object at 0x000001DCBBCDBE80>
self._commands[id] = CommandHandler(id='brainreg.Register', function=None, python_name='brainreg.napari.register:brainreg_register')

File ~\miniconda3\envs\napari-env\lib\site-packages\npe2_command_registry.py:34, in CommandHandler.resolve(self=CommandHandler(id='brainreg.Register', function=...ame='brainreg.napari.register:brainreg_register'))
32 self.function = utils.import_python_name(self.python_name)
33 except Exception as e:
---> 34 raise RuntimeError(
self.python_name = 'brainreg.napari.register:brainreg_register'
self = CommandHandler(id='brainreg.Register', function=None, python_name='brainreg.napari.register:brainreg_register')
35 f"Failed to import command at {self.python_name!r}: {e}"
36 ) from e
38 return self.function

RuntimeError: Failed to import command at 'brainreg.napari.register:brainreg_register': No module named 'brainglobe_napari_io'

[adamltyson]

Hi @Virginia9733, could you try updating your version of napari and brainreg (pip install napari brainreg -U)? If this doesn't work, please feel free to reach out to the BrainGlobe team over on the image.sc forum, as I don't think this is a napari issue anymore.

cc @alessandrofelder

[Virginia9733]

It works, thank you!
Also, may I ask, does brainreg only accept lightsheet or 3D whole brain imaging? Side scanning images was not supported?

[adamltyson]

Also, may I ask, does brainreg only accept lightsheet or 3D whole brain imaging? Side scanning images was not supported?

Yes, it only supports 3D images. We are building a new tool to handle registration of 2D and 3D data but it's still early days.

We're always happy to help with any BrainGlobe questions, but please use one of our contact channels. This is the napari issue tracker and there's likely to be many people getting unnecessary notifications!