Skip to content

Commit

Permalink
Use full plugin_name when finding chosen reader rather than `star…
Browse files Browse the repository at this point in the history
…tswith` (#297)

Prior to this PR, the wrong plugin might be chosen when multiple
compatible readers are available with similar names, and the user chose
to use a plugin with a shorter name. As a concrete example, if a user
chose explicitly to read a file with `napari`, but a plugin with a
longer name that starts with `napari` was also compatible, e.g.
`napari-tifffile`, the user's choice **may** have been ignored,
depending on which plugin came up first in the comparison.

To avoid this, this PR changes the comparison from
`rdr.command.startswith` to strict comparison on `rdr.plugin_name`. I've
added a test that always passes in this branch, and only sometimes
passes on `main` (😬) but I'm not sure if it's worth keeping it or if we
are all just happy to accept the logic 🤷‍♀️

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Juan Nunez-Iglesias <jni@fastmail.com>
  • Loading branch information
3 people authored Jun 16, 2023
1 parent 3486638 commit 31d1c90
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/npe2/io_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def _read(
_pm = PluginManager.instance()

for rdr in _pm.iter_compatible_readers(paths):
if plugin_name and not rdr.command.startswith(plugin_name):
if plugin_name and rdr.plugin_name != plugin_name:
continue
read_func = rdr.exec(
kwargs={"path": paths, "stack": stack, "_registry": _pm.commands}
Expand Down
29 changes: 29 additions & 0 deletions tests/test__io_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,35 @@ def test_read_with_no_plugin():
read(["some.nope"], stack=False)


def test_read_uses_correct_passed_plugin(tmp_path):
pm = PluginManager()
long_name = "gooby-again"
short_name = "gooby"
long_name_plugin = DynamicPlugin(long_name, plugin_manager=pm)
short_name_plugin = DynamicPlugin(short_name, plugin_manager=pm)

path = "something.fzzy"
mock_file = tmp_path / path
mock_file.touch()

@long_name_plugin.contribute.reader(filename_patterns=["*.fzzy"])
def get_read_long(path=mock_file):
raise ValueError(
f"Uhoh, {long_name} was chosen, but given plugin was {short_name}"
)

@short_name_plugin.contribute.reader(filename_patterns=["*.fzzy"])
def get_read(path=mock_file):
def read(paths):
return [(None,)]

return read

# "gooby-again" isn't used even though given plugin starts with the same name
# if an error is thrown here, it means we selected the wrong plugin
io_utils._read(["some.fzzy"], plugin_name=short_name, stack=False, _pm=pm)


def test_read_return_reader(uses_sample_plugin):
data, reader = read_get_reader("some.fzzy")
assert data == [(None,)]
Expand Down

0 comments on commit 31d1c90

Please sign in to comment.