Improve reading error for selected plugins

[DragaDoncila]

When a plugin is selected by the user to read a file, we want to raise specific error messages when that reader fails to read, for whatever reason.

Prior to this PR:

• if a selected reader refused to read the file i.e. the get_reader function returned None, the error message would say that the plugin returned no data
• if a selected reader returned a null_layer_sentinel, no error would be surfaced in napari. I think this is a holdover from olden days when we would cascade readers, so we didn't want the null_layer_sentinel to throw an error. I think there's probably good reason to remove the null_layer_sentinel entirely, but that's a discussion for another day.

With this PR:

• if a selected reader refused to read the file, we get "Reader {plugin_name!r} was selected to open {paths!r}, but refused the file."
• if a selected reader tried to read the file and returned null_layer_sentinel, we get "Reader {plugin_name!r} was selected to open {paths!r}, but returned no data."

As an aside, if a plugin is not selected, we get a semi-generic "No readers returned data" but I think this is also ok as, from the napari side, it's ~impossible for npe2 reading to get called without a plugin selected - either we use the preferred one, the user specifically selects one, or we explicitly pass the only one available.

Note that this brings the _is_null_layer_sentinel function to npe2, and it previously lived in napari. I think this is ok, and if we're happy with this PR, we can update its usage in napari to import from here.

Note also that this function checks specifically for a list (same as with napari/#7851), not a container sequence, but I actually think this is also ok and maybe we should update our typing in other places to simply expect a list, since many other container sequences wouldn't be useful anyway: a dict would likely fail somewhere weirdly, a set would lead to unexpected layer orderings... I guess a generator would be ok? But I don't think anyone is returning anything except a list.

[DragaDoncila]

ok these tests are unexpectedly failing cause they just passed locally. I think I did something weird. Please hold...

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (3089116) to head (3ba1e3f).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #377   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         2785      2794    +9     
=========================================
+ Hits          2785      2794    +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[DragaDoncila]

Ok this remaining error is totally not my fault, I will address in separate PR. Also this will probs break napari tests, so I'll take a look at that too. @napari/core-devs this might be a bigger change than we want to mess with right now, so I'm happy to close the PR if we want to punt and keep the existing behaviour (silent failure on selected reader returning no data), but would appreciate your insights, especially if you play with this stuff often.

[psobolewskiPhD]

If you saw the original version, I was in the wrong env, naturally.
ignore me.
(btw my issues with mp4 without pyav/ffmpeg are also resolved so that could have been an env issue too 😅 )

[brisvag]

is it always list or can it be like a tuple?

[DragaDoncila]

So I think technically speaking it could be a tuple, but this function was reproduced from napari, so it's what we've always used to validate the return type of readers. I'd say we keep it like this for now, and change it later if need be. Looking at the docs for readers, we always refer to a list also.

[DragaDoncila]

It might still be worth getting this PR in, but I'm not going to include it in v0.7.10 as it's going to potentially hold up napari/napari#8622 and I can't figure out how to easily test it until we merge napari/napari#8509 (lots of unrelated pydantic errors), which we can't do until we release v0.7.10.

[psobolewskiPhD]

Should we still check if layer_data isn't None/False before this return?

[TimMonko]

I'm pushing this change, and adding a test to see if that is the behavior we want. I think that what you say makes sense

[DragaDoncila]

I think this change is ok. Technically the reader should never be returning falsy things, but many probably do, so I think this is more defensive.