Add check for passed reader contribution

[DragaDoncila]

Following the report here , this PR updates the _read function to check if a specific reader contribution of the form plugin_name.reader_contribution was passed as plugin_name. This worked before #297 (though it was not part of the intended public API), but will not work with npe2>0.7.0 without this change.

Note that this PR does not throw a specific error if the given plugin doesn't exist - we can add a check for that but it's actually non-trivial since the only public reader method atm is iter_compatible_readers. We arguably should never be passing a non-existent plugin this level deep so maybe that's ok.

No doubt napari tests will now fail, but I will go and make those updates separately...

cc @Czaki @psobolewskiPhD

[codecov]

Codecov Report

Merging #301 (659a68e) into main (5199738) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #301   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         2790      2811   +21     
=========================================
+ Hits          2790      2811   +21     

Impacted Files	Coverage Δ
src/npe2/io_utils.py	100.00% <100.00%> (ø)

[DragaDoncila]

Will update this with an additional test and fix pre-commit checks if people are happy with this direction

[jni]

Ok I was a little hasty with my approval, mostly to indicate that I am happy with the direction, but in a longer review with @Czaki during community meeting we discussed the following issues:

• The codecov failure was confusing but the indirect changes tab shows that there is now an unreachable ValueError clause in lines 194-198.
• This failure (and my failure to see it) suggests that the nested logic is too complicated now. Ideally, the code should aim to have a flat structure, e.g. if plugin_name and plugin not in _pm.available_plugins ... elif plugin_name and not contrib: ... elif plugin_name and contrib etc.
• the plugin "nope" used in testing doesn't exist, but that's not clear from the name: it could also be a test plugin that does exist but never returns a reader, or has no valid contributions. Instead it would be good to change the name to something clearer, like "non-existent-plugin".
• Using split without the maxsplit= argument is a bit risky because users might include more than one . in the plugin+contribution string, even if that's not valid, and this might result in a confusing error message down the line. It's probably safer to use maxsplit=1 to find the plugin, contrib split.

Just had a chat with @DragaDoncila who says fixes are incoming though! 😊

[DragaDoncila]

mypy was complaining and I made it stop but idk if it was the right way to do it

[aganders3]

Looks good to me, but I humbly submit some unsolicited nitpicky readability suggestions.

[DragaDoncila]

Thank you @aganders3 those were great! So much neater now 😊 I've added more tests and I think the only remaining failure is napari_tests but the failure isn't user facing I'm 99.9% sure, but rather just changed logic. I don't know how to make 100% sure though or what the best order of events is.

[DragaDoncila]

Idk what this coverage failure is all of a sudden 🥲

[aganders3]

Coverage is just failing to upload. I was hoping it was a transient error but maybe a token expired?

[nclack]

LGTM! Left a couple suggestions that you can take or leave.

[DragaDoncila]

@nclack I've updated with your suggestions and I think this can go in. Note that #6016 will contain the fix for the currently failing napari test. We shouldn't release npe2 until I've added comprehensive tests in #6016 to make sure we're not breaking a use case with the changes here.