From bbfe154aaa244ea57c0c80f65f1e79211644c3b2 Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Wed, 28 Jun 2023 09:57:29 +1000 Subject: [PATCH 01/22] Add check for passed reader contribution, rearrange and add more error messages --- src/npe2/io_utils.py | 27 ++++++++++++++++++++------- tests/test__io_utils.py | 15 ++++++++++++++- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index a6462528..71c40940 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -154,9 +154,22 @@ def _read( if _pm is None: _pm = PluginManager.instance() - for rdr in _pm.iter_compatible_readers(paths): - if plugin_name and rdr.plugin_name != plugin_name: - continue + rdrs = list(_pm.iter_compatible_readers(paths)) + if plugin_name: + # user might have passed 'plugin.reader_contrib' as the command + plugin, contrib = tuple(plugin_name.split('.')) if '.' in plugin_name else (plugin_name, None) + chosen_compatible_readers = list(filter(lambda rdr: rdr.plugin_name == plugin, rdrs)) + if contrib: + chosen_compatible_readers = list(filter(lambda rdr: rdr.command.split('.')[1] == contrib, chosen_compatible_readers)) + if not chosen_compatible_readers: + raise ValueError(f"Given reader '{plugin_name}' is not a compatible reader for {paths}. " + + (f"Available readers for {paths} are: {[rdr.command for rdr in rdrs]}" if rdrs else \ + "No compatible readers are available." + )) + else: + chosen_compatible_readers = rdrs + + for rdr in chosen_compatible_readers: read_func = rdr.exec( kwargs={"path": paths, "stack": stack, "_registry": _pm.commands} ) @@ -166,10 +179,10 @@ def _read( return (layer_data, rdr) if return_reader else layer_data if plugin_name: - raise ValueError( - f"Plugin {plugin_name!r} was selected to open " - + f"{paths!r}, but returned no data." - ) + raise ValueError( + f"Plugin {plugin_name!r} was selected to open " + + f"{paths!r}, but returned no data." + ) raise ValueError(f"No readers returned data for {paths!r}") diff --git a/tests/test__io_utils.py b/tests/test__io_utils.py index bde64d22..0f3eb6c5 100644 --- a/tests/test__io_utils.py +++ b/tests/test__io_utils.py @@ -23,9 +23,14 @@ def test_read(uses_sample_plugin): def test_read_with_unknown_plugin(uses_sample_plugin): # no such plugin name.... skips over the sample plugin & error is specific - with pytest.raises(ValueError, match="Plugin 'nope' was selected"): + with pytest.raises(ValueError, match="Given reader 'nope' is not a compatible reader"): read(["some.fzzy"], plugin_name="nope", stack=False) +def test_read_with_unknown_plugin_no_readers(uses_sample_plugin): + with pytest.raises(ValueError, match="Given reader 'nope' is not a compatible reader"): + read(["some.nope"], plugin_name="nope", stack=False) + # assert 'No compatible readers are available.' in str(e) + def test_read_with_no_plugin(): # no plugin passed and none registered @@ -61,6 +66,14 @@ def read(paths): # 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_with_reader_contribution_plugin(uses_sample_plugin): + assert read(["some.fzzy"], stack=False, plugin_name=f'{SAMPLE_PLUGIN_NAME}.some_reader') == [(None,)] + + # if the wrong contribution is passed we get useful error message + with pytest.raises(ValueError, match="Given reader 'my-plugin.not_a_reader' is not a compatible reader") as e: + read(["some.fzzy"], stack=False, plugin_name=f'{SAMPLE_PLUGIN_NAME}.not_a_reader') + assert 'Available readers for' in str(e) + def test_read_return_reader(uses_sample_plugin): data, reader = read_get_reader("some.fzzy") From 34df97e23c8f06297b2fc61db5f56da2c1acde43 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 28 Jun 2023 00:01:45 +0000 Subject: [PATCH 02/22] style: [pre-commit.ci] auto fixes [...] --- src/npe2/io_utils.py | 35 ++++++++++++++++++++++++----------- tests/test__io_utils.py | 25 +++++++++++++++++++------ 2 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 71c40940..b5e6d1f4 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -157,15 +157,28 @@ def _read( rdrs = list(_pm.iter_compatible_readers(paths)) if plugin_name: # user might have passed 'plugin.reader_contrib' as the command - plugin, contrib = tuple(plugin_name.split('.')) if '.' in plugin_name else (plugin_name, None) - chosen_compatible_readers = list(filter(lambda rdr: rdr.plugin_name == plugin, rdrs)) + plugin, contrib = ( + tuple(plugin_name.split(".")) if "." in plugin_name else (plugin_name, None) + ) + chosen_compatible_readers = list( + filter(lambda rdr: rdr.plugin_name == plugin, rdrs) + ) if contrib: - chosen_compatible_readers = list(filter(lambda rdr: rdr.command.split('.')[1] == contrib, chosen_compatible_readers)) + chosen_compatible_readers = list( + filter( + lambda rdr: rdr.command.split(".")[1] == contrib, + chosen_compatible_readers, + ) + ) if not chosen_compatible_readers: - raise ValueError(f"Given reader '{plugin_name}' is not a compatible reader for {paths}. " - + (f"Available readers for {paths} are: {[rdr.command for rdr in rdrs]}" if rdrs else \ - "No compatible readers are available." - )) + raise ValueError( + f"Given reader '{plugin_name}' is not a compatible reader for {paths}. " + + ( + f"Available readers for {paths} are: {[rdr.command for rdr in rdrs]}" + if rdrs + else "No compatible readers are available." + ) + ) else: chosen_compatible_readers = rdrs @@ -179,10 +192,10 @@ def _read( return (layer_data, rdr) if return_reader else layer_data if plugin_name: - raise ValueError( - f"Plugin {plugin_name!r} was selected to open " - + f"{paths!r}, but returned no data." - ) + raise ValueError( + f"Plugin {plugin_name!r} was selected to open " + + f"{paths!r}, but returned no data." + ) raise ValueError(f"No readers returned data for {paths!r}") diff --git a/tests/test__io_utils.py b/tests/test__io_utils.py index 0f3eb6c5..29974692 100644 --- a/tests/test__io_utils.py +++ b/tests/test__io_utils.py @@ -23,11 +23,16 @@ def test_read(uses_sample_plugin): def test_read_with_unknown_plugin(uses_sample_plugin): # no such plugin name.... skips over the sample plugin & error is specific - with pytest.raises(ValueError, match="Given reader 'nope' is not a compatible reader"): + with pytest.raises( + ValueError, match="Given reader 'nope' is not a compatible reader" + ): read(["some.fzzy"], plugin_name="nope", stack=False) + def test_read_with_unknown_plugin_no_readers(uses_sample_plugin): - with pytest.raises(ValueError, match="Given reader 'nope' is not a compatible reader"): + with pytest.raises( + ValueError, match="Given reader 'nope' is not a compatible reader" + ): read(["some.nope"], plugin_name="nope", stack=False) # assert 'No compatible readers are available.' in str(e) @@ -66,13 +71,21 @@ def read(paths): # 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_with_reader_contribution_plugin(uses_sample_plugin): - assert read(["some.fzzy"], stack=False, plugin_name=f'{SAMPLE_PLUGIN_NAME}.some_reader') == [(None,)] + assert read( + ["some.fzzy"], stack=False, plugin_name=f"{SAMPLE_PLUGIN_NAME}.some_reader" + ) == [(None,)] # if the wrong contribution is passed we get useful error message - with pytest.raises(ValueError, match="Given reader 'my-plugin.not_a_reader' is not a compatible reader") as e: - read(["some.fzzy"], stack=False, plugin_name=f'{SAMPLE_PLUGIN_NAME}.not_a_reader') - assert 'Available readers for' in str(e) + with pytest.raises( + ValueError, + match="Given reader 'my-plugin.not_a_reader' is not a compatible reader", + ) as e: + read( + ["some.fzzy"], stack=False, plugin_name=f"{SAMPLE_PLUGIN_NAME}.not_a_reader" + ) + assert "Available readers for" in str(e) def test_read_return_reader(uses_sample_plugin): From 9cb6771998f67c6a963853dc197c2ef3e205a253 Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Wed, 28 Jun 2023 20:43:39 +1000 Subject: [PATCH 03/22] Tests passing --- src/npe2/io_utils.py | 75 ++++++++++++++++++++++++++--------------- tests/test__io_utils.py | 28 +++++++++------ 2 files changed, 65 insertions(+), 38 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index b5e6d1f4..6b77662c 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -154,33 +154,7 @@ def _read( if _pm is None: _pm = PluginManager.instance() - rdrs = list(_pm.iter_compatible_readers(paths)) - if plugin_name: - # user might have passed 'plugin.reader_contrib' as the command - plugin, contrib = ( - tuple(plugin_name.split(".")) if "." in plugin_name else (plugin_name, None) - ) - chosen_compatible_readers = list( - filter(lambda rdr: rdr.plugin_name == plugin, rdrs) - ) - if contrib: - chosen_compatible_readers = list( - filter( - lambda rdr: rdr.command.split(".")[1] == contrib, - chosen_compatible_readers, - ) - ) - if not chosen_compatible_readers: - raise ValueError( - f"Given reader '{plugin_name}' is not a compatible reader for {paths}. " - + ( - f"Available readers for {paths} are: {[rdr.command for rdr in rdrs]}" - if rdrs - else "No compatible readers are available." - ) - ) - else: - chosen_compatible_readers = rdrs + chosen_compatible_readers = get_compatible_readers_by_choice(plugin_name, paths, _pm) for rdr in chosen_compatible_readers: read_func = rdr.exec( @@ -193,11 +167,56 @@ def _read( if plugin_name: raise ValueError( - f"Plugin {plugin_name!r} was selected to open " + f"Reader {plugin_name!r} was selected to open " + f"{paths!r}, but returned no data." ) raise ValueError(f"No readers returned data for {paths!r}") +def get_compatible_readers_by_choice(plugin_name, paths, _pm): + compat_readers = list(_pm.iter_compatible_readers(paths)) + compat_reader_names = sorted([rdr.plugin_name for rdr in compat_readers]) + helper_error_message = ( + f"Available readers for {paths!r} are: {compat_reader_names!r}." + if compat_reader_names + else f"No compatible readers are available for {paths!r}." + ) + + # check whether plugin even exists. + if plugin_name: + try: + # note that get_manifest works with a full command e.g. my-plugin.my-reader + _pm.get_manifest(plugin_name) + except KeyError: + raise ValueError(f"Given reader {plugin_name!r} does not exist. {helper_error_message}") + + if not len(compat_reader_names): + raise ValueError(helper_error_message) + + # user didn't make a choice and we have some readers to try, return them + if not plugin_name: + return compat_readers + + # user made a choice and it exists, but it may not be a compatible reader + plugin, contrib = ( + tuple(plugin_name.split(".", maxsplit=1)) if "." in plugin_name else (plugin_name, None) + ) + chosen_compatible_readers = list( + filter(lambda rdr: rdr.plugin_name == plugin, compat_readers) + ) + if contrib: + chosen_compatible_readers = list( + filter( + lambda rdr: rdr.command.split(".")[1] == contrib, + chosen_compatible_readers, + ) + ) + # the user's choice is not compatible with the paths. let them know what is + if not chosen_compatible_readers: + raise ValueError( + f"Given reader {plugin_name!r} is not a compatible reader for {paths!r}. " + + helper_error_message + ) + return chosen_compatible_readers @overload def _write( diff --git a/tests/test__io_utils.py b/tests/test__io_utils.py index 29974692..ea359523 100644 --- a/tests/test__io_utils.py +++ b/tests/test__io_utils.py @@ -23,24 +23,29 @@ def test_read(uses_sample_plugin): def test_read_with_unknown_plugin(uses_sample_plugin): # no such plugin name.... skips over the sample plugin & error is specific + paths = ["some.fzzy"] + chosen_reader = "not-a-plugin" with pytest.raises( - ValueError, match="Given reader 'nope' is not a compatible reader" - ): - read(["some.fzzy"], plugin_name="nope", stack=False) - + ValueError, match=f"Given reader {chosen_reader!r} does not exist." + ) as e: + read(paths, plugin_name=chosen_reader, stack=False) + assert f"Available readers for {paths!r} are: {[SAMPLE_PLUGIN_NAME]!r}" in str(e) def test_read_with_unknown_plugin_no_readers(uses_sample_plugin): + paths = ["some.nope"] + chosen_reader = "not-a-plugin" with pytest.raises( - ValueError, match="Given reader 'nope' is not a compatible reader" - ): - read(["some.nope"], plugin_name="nope", stack=False) - # assert 'No compatible readers are available.' in str(e) + ValueError, match=f"Given reader {chosen_reader!r} does not exist." + ) as e: + read(paths, plugin_name=chosen_reader, stack=False) + assert 'No compatible readers are available' in str(e) def test_read_with_no_plugin(): # no plugin passed and none registered - with pytest.raises(ValueError, match="No readers returned"): - read(["some.nope"], stack=False) + paths = ["some.nope"] + with pytest.raises(ValueError, match=f"No compatible readers are available"): + read(paths, stack=False) def test_read_uses_correct_passed_plugin(tmp_path): @@ -50,6 +55,9 @@ def test_read_uses_correct_passed_plugin(tmp_path): long_name_plugin = DynamicPlugin(long_name, plugin_manager=pm) short_name_plugin = DynamicPlugin(short_name, plugin_manager=pm) + long_name_plugin.register() + short_name_plugin.register() + path = "something.fzzy" mock_file = tmp_path / path mock_file.touch() From 1cc61229f38144b0cabe79498c31841c048b8c18 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 28 Jun 2023 10:44:05 +0000 Subject: [PATCH 04/22] style: [pre-commit.ci] auto fixes [...] --- src/npe2/io_utils.py | 24 ++++++++++++++++-------- tests/test__io_utils.py | 5 +++-- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 6b77662c..524e0a57 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -154,7 +154,9 @@ def _read( if _pm is None: _pm = PluginManager.instance() - chosen_compatible_readers = get_compatible_readers_by_choice(plugin_name, paths, _pm) + chosen_compatible_readers = get_compatible_readers_by_choice( + plugin_name, paths, _pm + ) for rdr in chosen_compatible_readers: read_func = rdr.exec( @@ -172,14 +174,15 @@ def _read( ) raise ValueError(f"No readers returned data for {paths!r}") + def get_compatible_readers_by_choice(plugin_name, paths, _pm): compat_readers = list(_pm.iter_compatible_readers(paths)) compat_reader_names = sorted([rdr.plugin_name for rdr in compat_readers]) helper_error_message = ( - f"Available readers for {paths!r} are: {compat_reader_names!r}." - if compat_reader_names - else f"No compatible readers are available for {paths!r}." - ) + f"Available readers for {paths!r} are: {compat_reader_names!r}." + if compat_reader_names + else f"No compatible readers are available for {paths!r}." + ) # check whether plugin even exists. if plugin_name: @@ -187,7 +190,9 @@ def get_compatible_readers_by_choice(plugin_name, paths, _pm): # note that get_manifest works with a full command e.g. my-plugin.my-reader _pm.get_manifest(plugin_name) except KeyError: - raise ValueError(f"Given reader {plugin_name!r} does not exist. {helper_error_message}") + raise ValueError( + f"Given reader {plugin_name!r} does not exist. {helper_error_message}" + ) if not len(compat_reader_names): raise ValueError(helper_error_message) @@ -195,10 +200,12 @@ def get_compatible_readers_by_choice(plugin_name, paths, _pm): # user didn't make a choice and we have some readers to try, return them if not plugin_name: return compat_readers - + # user made a choice and it exists, but it may not be a compatible reader plugin, contrib = ( - tuple(plugin_name.split(".", maxsplit=1)) if "." in plugin_name else (plugin_name, None) + tuple(plugin_name.split(".", maxsplit=1)) + if "." in plugin_name + else (plugin_name, None) ) chosen_compatible_readers = list( filter(lambda rdr: rdr.plugin_name == plugin, compat_readers) @@ -218,6 +225,7 @@ def get_compatible_readers_by_choice(plugin_name, paths, _pm): ) return chosen_compatible_readers + @overload def _write( path: str, diff --git a/tests/test__io_utils.py b/tests/test__io_utils.py index ea359523..92c2df20 100644 --- a/tests/test__io_utils.py +++ b/tests/test__io_utils.py @@ -31,6 +31,7 @@ def test_read_with_unknown_plugin(uses_sample_plugin): read(paths, plugin_name=chosen_reader, stack=False) assert f"Available readers for {paths!r} are: {[SAMPLE_PLUGIN_NAME]!r}" in str(e) + def test_read_with_unknown_plugin_no_readers(uses_sample_plugin): paths = ["some.nope"] chosen_reader = "not-a-plugin" @@ -38,13 +39,13 @@ def test_read_with_unknown_plugin_no_readers(uses_sample_plugin): ValueError, match=f"Given reader {chosen_reader!r} does not exist." ) as e: read(paths, plugin_name=chosen_reader, stack=False) - assert 'No compatible readers are available' in str(e) + assert "No compatible readers are available" in str(e) def test_read_with_no_plugin(): # no plugin passed and none registered paths = ["some.nope"] - with pytest.raises(ValueError, match=f"No compatible readers are available"): + with pytest.raises(ValueError, match="No compatible readers are available"): read(paths, stack=False) From a61f1a586e9b07c683bcc0cd2b163c0ef8b39741 Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Wed, 28 Jun 2023 20:56:04 +1000 Subject: [PATCH 05/22] Fix contribution existence check --- src/npe2/io_utils.py | 2 ++ tests/test__io_utils.py | 16 +++++++++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 524e0a57..211ef716 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -189,6 +189,8 @@ def get_compatible_readers_by_choice(plugin_name, paths, _pm): try: # note that get_manifest works with a full command e.g. my-plugin.my-reader _pm.get_manifest(plugin_name) + if '.' in plugin_name: + _pm.get_command(plugin_name) except KeyError: raise ValueError( f"Given reader {plugin_name!r} does not exist. {helper_error_message}" diff --git a/tests/test__io_utils.py b/tests/test__io_utils.py index 92c2df20..e57c0676 100644 --- a/tests/test__io_utils.py +++ b/tests/test__io_utils.py @@ -80,19 +80,25 @@ def read(paths): # 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_fails_with_chosen_plugin(): + pm = PluginManager() + plugin_name = 'always-fails' + + def test_read_with_reader_contribution_plugin(uses_sample_plugin): - assert read( - ["some.fzzy"], stack=False, plugin_name=f"{SAMPLE_PLUGIN_NAME}.some_reader" - ) == [(None,)] + paths = ["some.fzzy"] + chosen_reader = f"{SAMPLE_PLUGIN_NAME}.some_reader" + assert read(paths, stack=False, plugin_name=chosen_reader) == [(None,)] # if the wrong contribution is passed we get useful error message + chosen_reader = f"{SAMPLE_PLUGIN_NAME}.not_a_reader" with pytest.raises( ValueError, - match="Given reader 'my-plugin.not_a_reader' is not a compatible reader", + match=f"Given reader {chosen_reader!r} does not exist.", ) as e: read( - ["some.fzzy"], stack=False, plugin_name=f"{SAMPLE_PLUGIN_NAME}.not_a_reader" + paths, stack=False, plugin_name=chosen_reader ) assert "Available readers for" in str(e) From 0a9506aab50db4d1009dda6fc02d6b6df54d598f Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Wed, 28 Jun 2023 21:14:51 +1000 Subject: [PATCH 06/22] Add more tests --- tests/test__io_utils.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/test__io_utils.py b/tests/test__io_utils.py index e57c0676..ee723076 100644 --- a/tests/test__io_utils.py +++ b/tests/test__io_utils.py @@ -80,10 +80,21 @@ def read(paths): # 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_fails_with_chosen_plugin(): +def test_read_fails(): pm = PluginManager() plugin_name = 'always-fails' + plugin = DynamicPlugin(plugin_name, plugin_manager=pm) + plugin.register() + @plugin.contribute.reader(filename_patterns=["*.fzzy"]) + def get_read(path): + return None + + with pytest.raises(ValueError, match=f"Reader {plugin_name!r} was selected"): + io_utils._read(["some.fzzy"], plugin_name=plugin_name, stack=False, _pm=pm) + + with pytest.raises(ValueError, match=f"No readers returned data"): + io_utils._read(["some.fzzy"], stack=False, _pm=pm) def test_read_with_reader_contribution_plugin(uses_sample_plugin): From 91a5f6c2ddbcfa1440e96b81b2a28e57bff3f412 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 28 Jun 2023 11:15:20 +0000 Subject: [PATCH 07/22] style: [pre-commit.ci] auto fixes [...] --- src/npe2/io_utils.py | 2 +- tests/test__io_utils.py | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 211ef716..4a77e87b 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -189,7 +189,7 @@ def get_compatible_readers_by_choice(plugin_name, paths, _pm): try: # note that get_manifest works with a full command e.g. my-plugin.my-reader _pm.get_manifest(plugin_name) - if '.' in plugin_name: + if "." in plugin_name: _pm.get_command(plugin_name) except KeyError: raise ValueError( diff --git a/tests/test__io_utils.py b/tests/test__io_utils.py index ee723076..0cee021a 100644 --- a/tests/test__io_utils.py +++ b/tests/test__io_utils.py @@ -80,9 +80,10 @@ def read(paths): # 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_fails(): pm = PluginManager() - plugin_name = 'always-fails' + plugin_name = "always-fails" plugin = DynamicPlugin(plugin_name, plugin_manager=pm) plugin.register() @@ -92,8 +93,8 @@ def get_read(path): with pytest.raises(ValueError, match=f"Reader {plugin_name!r} was selected"): io_utils._read(["some.fzzy"], plugin_name=plugin_name, stack=False, _pm=pm) - - with pytest.raises(ValueError, match=f"No readers returned data"): + + with pytest.raises(ValueError, match="No readers returned data"): io_utils._read(["some.fzzy"], stack=False, _pm=pm) @@ -108,9 +109,7 @@ def test_read_with_reader_contribution_plugin(uses_sample_plugin): ValueError, match=f"Given reader {chosen_reader!r} does not exist.", ) as e: - read( - paths, stack=False, plugin_name=chosen_reader - ) + read(paths, stack=False, plugin_name=chosen_reader) assert "Available readers for" in str(e) From 03407f3dea0f11e850400525e54b1e1b11254d79 Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Wed, 28 Jun 2023 21:16:40 +1000 Subject: [PATCH 08/22] Appease the ruff gods --- src/npe2/io_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 211ef716..84b190df 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -194,7 +194,7 @@ def get_compatible_readers_by_choice(plugin_name, paths, _pm): except KeyError: raise ValueError( f"Given reader {plugin_name!r} does not exist. {helper_error_message}" - ) + ) from None if not len(compat_reader_names): raise ValueError(helper_error_message) From 4a59c52d82ff701e102e7cdf091c327df076d04d Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Wed, 28 Jun 2023 21:25:15 +1000 Subject: [PATCH 09/22] Add docs --- src/npe2/io_utils.py | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 1846f3e4..071d624a 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -154,6 +154,8 @@ def _read( if _pm is None: _pm = PluginManager.instance() + # get readers compatible with paths and chosen plugin - raise errors if + # choices are invalid or there's nothing to try chosen_compatible_readers = get_compatible_readers_by_choice( plugin_name, paths, _pm ) @@ -175,8 +177,33 @@ def _read( raise ValueError(f"No readers returned data for {paths!r}") -def get_compatible_readers_by_choice(plugin_name, paths, _pm): - compat_readers = list(_pm.iter_compatible_readers(paths)) +def get_compatible_readers_by_choice( + plugin_name: Union[str, None], + paths: Union[str, Sequence[str]], + pm: PluginManager +): + """Returns compatible readers filtered by validated plugin choice. + + Checks that plugin_name is an existing plugin (and command if + a specific contribution was passed), and that it is compatible + with paths. Raises ValueError if given plugin doesn't exist, + it is not compatible with the given paths, or no compatible + readers can be found for paths (if no plugin was chosen). + + Args: + plugin_name: _description_ + paths: _description_ + _pm: _description_ + + Raises: + ValueError: If the given reader doesn't exist + ValueError: If there are no compatible readers + ValueError: If the given reader is not compatible + + Returns: + _description_ + """ + compat_readers = list(pm.iter_compatible_readers(paths)) compat_reader_names = sorted([rdr.plugin_name for rdr in compat_readers]) helper_error_message = ( f"Available readers for {paths!r} are: {compat_reader_names!r}." @@ -188,9 +215,9 @@ def get_compatible_readers_by_choice(plugin_name, paths, _pm): if plugin_name: try: # note that get_manifest works with a full command e.g. my-plugin.my-reader - _pm.get_manifest(plugin_name) + pm.get_manifest(plugin_name) if "." in plugin_name: - _pm.get_command(plugin_name) + pm.get_command(plugin_name) except KeyError: raise ValueError( f"Given reader {plugin_name!r} does not exist. {helper_error_message}" From 8267446133a1d81b189e42f2902a5e9b6851d165 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 28 Jun 2023 11:25:35 +0000 Subject: [PATCH 10/22] style: [pre-commit.ci] auto fixes [...] --- src/npe2/io_utils.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 071d624a..5055b08a 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -178,9 +178,7 @@ def _read( def get_compatible_readers_by_choice( - plugin_name: Union[str, None], - paths: Union[str, Sequence[str]], - pm: PluginManager + plugin_name: Union[str, None], paths: Union[str, Sequence[str]], pm: PluginManager ): """Returns compatible readers filtered by validated plugin choice. From 1c8524a5f4bc82b7851cb0f268ebf7ba6027c796 Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Wed, 28 Jun 2023 21:29:35 +1000 Subject: [PATCH 11/22] Finish docs --- src/npe2/io_utils.py | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 071d624a..0e3661ef 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -190,18 +190,28 @@ def get_compatible_readers_by_choice( it is not compatible with the given paths, or no compatible readers can be found for paths (if no plugin was chosen). - Args: - plugin_name: _description_ - paths: _description_ - _pm: _description_ - - Raises: - ValueError: If the given reader doesn't exist - ValueError: If there are no compatible readers - ValueError: If the given reader is not compatible - - Returns: - _description_ + Parameters + ---------- + plugin_name: Union[str, None] + name of chosen plugin, or None + paths: Union[str, Sequence[str]] + paths to read + pm: PluginManager + plugin manager instance to check for readers + + Raises + ------ + ValueError + If the given reader doesn't exist + ValueError + If there are no compatible readers + ValueError + If the given reader is not compatible + + Returns + ------- + compat_readers : List[ReaderContribution] + Compatible readers for plugin choice """ compat_readers = list(pm.iter_compatible_readers(paths)) compat_reader_names = sorted([rdr.plugin_name for rdr in compat_readers]) From 3ca4a98a7e684dbdcb7e67f33d2b78841e774dca Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 28 Jun 2023 11:30:44 +0000 Subject: [PATCH 12/22] style: [pre-commit.ci] auto fixes [...] --- src/npe2/io_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 75758ffe..cefe0298 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -198,7 +198,7 @@ def get_compatible_readers_by_choice( plugin manager instance to check for readers Raises - ------ + ------ ValueError If the given reader doesn't exist ValueError From 7f88cdd046e3aea5c5cf736f41c0d03cea31d5f5 Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Thu, 29 Jun 2023 01:18:04 +1000 Subject: [PATCH 13/22] Only show command name if user passed a command --- src/npe2/io_utils.py | 15 +++++++++------ src/npe2/manifest/_bases.py | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 75758ffe..733ce7f4 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -156,7 +156,7 @@ def _read( # get readers compatible with paths and chosen plugin - raise errors if # choices are invalid or there's nothing to try - chosen_compatible_readers = get_compatible_readers_by_choice( + chosen_compatible_readers = _get_compatible_readers_by_choice( plugin_name, paths, _pm ) @@ -177,7 +177,7 @@ def _read( raise ValueError(f"No readers returned data for {paths!r}") -def get_compatible_readers_by_choice( +def _get_compatible_readers_by_choice( plugin_name: Union[str, None], paths: Union[str, Sequence[str]], pm: PluginManager ): """Returns compatible readers filtered by validated plugin choice. @@ -198,7 +198,7 @@ def get_compatible_readers_by_choice( plugin manager instance to check for readers Raises - ------ + ------ ValueError If the given reader doesn't exist ValueError @@ -211,8 +211,11 @@ def get_compatible_readers_by_choice( compat_readers : List[ReaderContribution] Compatible readers for plugin choice """ + passed_contrib = plugin_name and ("." in plugin_name) compat_readers = list(pm.iter_compatible_readers(paths)) - compat_reader_names = sorted([rdr.plugin_name for rdr in compat_readers]) + compat_reader_names = sorted( + {(rdr.command if passed_contrib else rdr.plugin_name) for rdr in compat_readers} + ) helper_error_message = ( f"Available readers for {paths!r} are: {compat_reader_names!r}." if compat_reader_names @@ -224,7 +227,7 @@ def get_compatible_readers_by_choice( try: # note that get_manifest works with a full command e.g. my-plugin.my-reader pm.get_manifest(plugin_name) - if "." in plugin_name: + if passed_contrib: pm.get_command(plugin_name) except KeyError: raise ValueError( @@ -241,7 +244,7 @@ def get_compatible_readers_by_choice( # user made a choice and it exists, but it may not be a compatible reader plugin, contrib = ( tuple(plugin_name.split(".", maxsplit=1)) - if "." in plugin_name + if passed_contrib else (plugin_name, None) ) chosen_compatible_readers = list( diff --git a/src/npe2/manifest/_bases.py b/src/npe2/manifest/_bases.py index 630b75bf..d5e7d7c7 100644 --- a/src/npe2/manifest/_bases.py +++ b/src/npe2/manifest/_bases.py @@ -78,7 +78,7 @@ def from_file(cls, path: Union[Path, str]): try: import tomllib except ImportError: - import tomli as tomllib # type: ignore [no-redef] + import tomli as tomllib loader = tomllib.load elif path.suffix.lower() in (".yaml", ".yml"): From b9405f1a988c53649abe88d23099932f04083f0d Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop <17995243+DragaDoncila@users.noreply.github.com> Date: Thu, 29 Jun 2023 12:27:16 +1000 Subject: [PATCH 14/22] Update src/npe2/io_utils.py Co-authored-by: Ashley Anderson --- src/npe2/io_utils.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 733ce7f4..13fbe3fd 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -247,16 +247,11 @@ def _get_compatible_readers_by_choice( if passed_contrib else (plugin_name, None) ) - chosen_compatible_readers = list( - filter(lambda rdr: rdr.plugin_name == plugin, compat_readers) - ) - if contrib: - chosen_compatible_readers = list( - filter( - lambda rdr: rdr.command.split(".")[1] == contrib, - chosen_compatible_readers, - ) - ) + chosen_compatible_readers = [ + rdr for rdr in compat_readers + if rdr.plugin_name == plugin + and (not passed_contrib or rdr.command == plugin_name) + ] # the user's choice is not compatible with the paths. let them know what is if not chosen_compatible_readers: raise ValueError( From 67175487feb5ad010a85afcbe35216accf96d1ce Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 29 Jun 2023 02:27:43 +0000 Subject: [PATCH 15/22] style: [pre-commit.ci] auto fixes [...] --- src/npe2/io_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 13fbe3fd..340c8103 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -248,7 +248,8 @@ def _get_compatible_readers_by_choice( else (plugin_name, None) ) chosen_compatible_readers = [ - rdr for rdr in compat_readers + rdr + for rdr in compat_readers if rdr.plugin_name == plugin and (not passed_contrib or rdr.command == plugin_name) ] From c32d6f005624e1b4b72b8f4fcfc2257b88e845de Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop <17995243+DragaDoncila@users.noreply.github.com> Date: Thu, 29 Jun 2023 12:29:24 +1000 Subject: [PATCH 16/22] Update src/npe2/io_utils.py Co-authored-by: Ashley Anderson --- src/npe2/io_utils.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 340c8103..bab300a6 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -242,11 +242,7 @@ def _get_compatible_readers_by_choice( return compat_readers # user made a choice and it exists, but it may not be a compatible reader - plugin, contrib = ( - tuple(plugin_name.split(".", maxsplit=1)) - if passed_contrib - else (plugin_name, None) - ) + plugin, _, contrib = plugin_name.partition(".") chosen_compatible_readers = [ rdr for rdr in compat_readers From ddfe5e8315b787d7b2fec9066746719c27d79e2d Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Thu, 29 Jun 2023 12:40:37 +1000 Subject: [PATCH 17/22] Bring back no-redef --- src/npe2/manifest/_bases.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/npe2/manifest/_bases.py b/src/npe2/manifest/_bases.py index d5e7d7c7..630b75bf 100644 --- a/src/npe2/manifest/_bases.py +++ b/src/npe2/manifest/_bases.py @@ -78,7 +78,7 @@ def from_file(cls, path: Union[Path, str]): try: import tomllib except ImportError: - import tomli as tomllib + import tomli as tomllib # type: ignore [no-redef] loader = tomllib.load elif path.suffix.lower() in (".yaml", ".yml"): From 5b233aff6d9915d555e7db5da1059ffb25a6af3b Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Thu, 29 Jun 2023 13:04:30 +1000 Subject: [PATCH 18/22] Add more tests --- src/npe2/io_utils.py | 3 ++- tests/test__io_utils.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index bab300a6..5a4d1943 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -234,7 +234,8 @@ def _get_compatible_readers_by_choice( f"Given reader {plugin_name!r} does not exist. {helper_error_message}" ) from None - if not len(compat_reader_names): + # no choice was made and there's no readers to try + if not plugin_name and not len(compat_reader_names): raise ValueError(helper_error_message) # user didn't make a choice and we have some readers to try, return them diff --git a/tests/test__io_utils.py b/tests/test__io_utils.py index 0cee021a..99b35093 100644 --- a/tests/test__io_utils.py +++ b/tests/test__io_utils.py @@ -98,6 +98,21 @@ def get_read(path): io_utils._read(["some.fzzy"], stack=False, _pm=pm) +def test_read_with_incompatible_reader(uses_sample_plugin): + paths = ["some.notfzzy"] + chosen_reader = f"{SAMPLE_PLUGIN_NAME}" + with pytest.raises( + ValueError, match=f"Given reader {chosen_reader!r} is not a compatible reader" + ): + read(paths, stack=False, plugin_name=chosen_reader) + + +def test_read_with_no_compatible_reader(): + paths = ["some.notfzzy"] + with pytest.raises(ValueError, match="No compatible readers are available"): + read(paths, stack=False) + + def test_read_with_reader_contribution_plugin(uses_sample_plugin): paths = ["some.fzzy"] chosen_reader = f"{SAMPLE_PLUGIN_NAME}.some_reader" From 2592a1d35a969e4de3f5d9d4ac9e68d1baa3bf68 Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Thu, 29 Jun 2023 13:14:35 +1000 Subject: [PATCH 19/22] Add one more test --- tests/test__io_utils.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test__io_utils.py b/tests/test__io_utils.py index 99b35093..e0b79027 100644 --- a/tests/test__io_utils.py +++ b/tests/test__io_utils.py @@ -128,6 +128,28 @@ def test_read_with_reader_contribution_plugin(uses_sample_plugin): assert "Available readers for" in str(e) +def test_available_readers_show_commands(uses_sample_plugin): + paths = ["some.fzzy"] + chosen_reader = "not-a-plugin.not-a-reader" + with pytest.raises( + ValueError, + match=f"Given reader {chosen_reader!r} does not exist.", + ) as e: + read(paths, stack=False, plugin_name=chosen_reader) + assert "Available readers " in str(e) + assert f"{SAMPLE_PLUGIN_NAME}.some_reader" in str(e) + + chosen_reader = "not-a-plugin" + with pytest.raises( + ValueError, + match=f"Given reader {chosen_reader!r} does not exist.", + ) as e: + read(paths, stack=False, plugin_name=chosen_reader) + assert "Available readers " in str(e) + assert f"{SAMPLE_PLUGIN_NAME}.some_reader" not in str(e) + assert f"{SAMPLE_PLUGIN_NAME}" in str(e) + + def test_read_return_reader(uses_sample_plugin): data, reader = read_get_reader("some.fzzy") assert data == [(None,)] From ab0a105a7f50b4afd4da85965a0a887c6b06c1a1 Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop <17995243+DragaDoncila@users.noreply.github.com> Date: Wed, 12 Jul 2023 13:21:32 +1000 Subject: [PATCH 20/22] Update src/npe2/io_utils.py Co-authored-by: Nathan Clack --- src/npe2/io_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 5a4d1943..104a83e1 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -243,7 +243,7 @@ def _get_compatible_readers_by_choice( return compat_readers # user made a choice and it exists, but it may not be a compatible reader - plugin, _, contrib = plugin_name.partition(".") + plugin, _, _ = plugin_name.partition(".") chosen_compatible_readers = [ rdr for rdr in compat_readers From a9a9129f87e232c6e42877244461a5246401f872 Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop <17995243+DragaDoncila@users.noreply.github.com> Date: Wed, 12 Jul 2023 13:50:45 +1000 Subject: [PATCH 21/22] Update src/npe2/io_utils.py Co-authored-by: Nathan Clack --- src/npe2/io_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 104a83e1..24f08b48 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -159,6 +159,7 @@ def _read( chosen_compatible_readers = _get_compatible_readers_by_choice( plugin_name, paths, _pm ) + assert chosen_compatible_readers, "Expected an exception before this point" for rdr in chosen_compatible_readers: read_func = rdr.exec( From 59ea5542901df2c6c79defaf5b496276001a55e5 Mon Sep 17 00:00:00 2001 From: Draga Doncila Pop Date: Wed, 12 Jul 2023 14:05:35 +1000 Subject: [PATCH 22/22] Update assertion message and add tests --- src/npe2/io_utils.py | 4 +++- tests/test__io_utils.py | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/npe2/io_utils.py b/src/npe2/io_utils.py index 24f08b48..2ba68594 100644 --- a/src/npe2/io_utils.py +++ b/src/npe2/io_utils.py @@ -159,7 +159,9 @@ def _read( chosen_compatible_readers = _get_compatible_readers_by_choice( plugin_name, paths, _pm ) - assert chosen_compatible_readers, "Expected an exception before this point" + assert ( + chosen_compatible_readers + ), "No readers to try. Expected an exception before this point." for rdr in chosen_compatible_readers: read_func = rdr.exec( diff --git a/tests/test__io_utils.py b/tests/test__io_utils.py index e0b79027..ee64df4d 100644 --- a/tests/test__io_utils.py +++ b/tests/test__io_utils.py @@ -1,5 +1,6 @@ # extra underscore in name to run this first from pathlib import Path +from unittest.mock import patch import pytest @@ -128,6 +129,13 @@ def test_read_with_reader_contribution_plugin(uses_sample_plugin): assert "Available readers for" in str(e) +def test_read_assertion_with_no_compatible_readers(uses_sample_plugin): + paths = ["some.noreader"] + with patch("npe2.io_utils._get_compatible_readers_by_choice", return_value=[]): + with pytest.raises(AssertionError, match="No readers to try."): + read(paths, stack=False) + + def test_available_readers_show_commands(uses_sample_plugin): paths = ["some.fzzy"] chosen_reader = "not-a-plugin.not-a-reader"