allow v2.3 in metadataversion, fallback to any string

[tlambert03]

someone recently showed me an issue validating the manifest of a napari plugin,

ValidationError: 1 validation error for PackageMetadata
metadata_version
  unexpected value; permitted: '1.0', '1.1', '1.2', '2.0', '2.1', '2.2' (type=value_error.const; given=2.3; permitted=('1.0', '1.1', '1.2', '2.0', '2.1', '2.2'))

What happened is that PEP685 https://peps.python.org/pep-0685/ added a new spec (v2.3), and some build backends have begun to implement it. For example, hatchling (which this package was using) bumped their default version to v2.3 5 days ago: pypa/hatch@1a9c30d

This PR just relaxes the validation of metadata_version to allow for any string.

@napari/core-devs, this is something that would be important to get in soon and push a new release. Packages that use setuptools will not have this issue, but anything using hatchling after 1.22.1 (released on Mar 15) will begin to see this

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (595b088) to head (57f7cca).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #344   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         2750      2750           
=========================================
  Hits          2750      2750           

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

[jni]

Thanks @tlambert03!

@Czaki @nclack @andy-sweet @aganders3 do any of you know why the napari headless tests would be failing with "no Qt bindings found"? here:

_____________________________ test_plugin_actions ______________________________
    import os
    import sys
    from pathlib import Path
    from warnings import warn
    
    from napari.utils.translations import trans
    
    try:
>       from qtpy import API_NAME, QT_VERSION, QtCore

[...]

E           ImportError: No Qt bindings could be found.
E           
E           napari requires either PyQt5 (default) or PySide2 to be installed in the environment.
E           
E           With pip, you can install either with:
E             $ pip install -U 'napari[all]'  # default choice
E             $ pip install -U 'napari[pyqt5]'
E             $ pip install -U 'napari[pyside2]'
E           
E           With conda, you need to do:
E             $ conda install -c conda-forge pyqt
E             $ conda install -c conda-forge pyside2
E           
E           Our heuristics suggest you are using 'pip' to manage your packages.

(We may or may not want to merge this without fixing this issue first since it seems totally unrelated. But the failing tests might block release.)

[DragaDoncila]

pretty confident this is a side effect of napari/#4991. I'll take a look at it tomorrow morning at the latest.

[tlambert03]

yep, it is. testing napari/plugins now requires qt:

napari/plugins/_tests/test_npe2.py:186:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
napari/plugins/__init__.py:34: in _initialize_plugins
    _npe2.on_plugins_registered(set(_npe2pm.iter_manifests()))
napari/plugins/_npe2.py:341: in on_plugins_registered
    _safe_register_qt_actions(mf)
napari/plugins/_npe2.py:367: in _safe_register_qt_actions
    from napari._qt._qplugins import _register_qt_actions

[DragaDoncila]

Yeah I thought that was supposed to be guarded but obviously we cooked something!

[aganders3]

A clue perhaps is that this line is specifically looking for ModuleNotFoundError but the Qt guard I think will raise a more general ImportError:
https://github.com/napari/napari/blob/b3e15c512ea611e3a8c98f1cba0a326890d2fdb2/napari/plugins/_npe2.py#L368

[tlambert03]

this is the test that used to test this on the napari side to make sure that you could run napari tests without qt:

https://github.com/napari/napari/blob/b3e15c512ea611e3a8c98f1cba0a326890d2fdb2/tox.ini#L117-L119

not sure where it is run these days

[DragaDoncila]

Based on the workflow file and the run on 4991 I think headless tests are now only run on Ubuntu 20.04 with python=3.9, while npe2 uses Ubuntu 22.04 python=3.10. If this could lead to different errors thrown by Qt (ImportError vs. ModuleNotFoundError) then that would explain why the workflow on the PR didn't catch this. Probably a sign we should bring headless tests back across multiple platforms/python versions. FWIW I tried running napari headless tests locally on my Ubuntu 22.04 machine with python=3.10 but all passed 🤷‍♀️ .

As for the fix, I think we should be catching the more general ImportError in this case, since the whole point of that function is to fail gracefully if there's no qt. I'll open PR in napari.

[aganders3]

Also note in napari's tox env for headless there is this line
py310-linux-headless-no_cov: commands_pre[0]> pip uninstall -y pyautogui pytest-qt qtpy pyqt5 pyside2 pyside6 pyqt6

https://github.com/napari/napari/blob/f4641213513a13b76195e3c8bb910cbcba60ede3/tox.ini#L111

Maybe this is missed when running headless tests here in npe2?

Uninstalling qtpy would cause a ModuleNotFoundError instead (properly caught).

[Czaki]

Should we provide a validator that checks if this string is a valid version number?

[tlambert03]

I assume you mean is it of the general pattern X.Y? You can if you want to, but I'm not sure it matters (and could bring back the future proof problem).

[Czaki]

So why we just don't annotate with plain str?

[tlambert03]

@Czaki, you can do whatever you want. with my code above, I tried to indicate that normally this should be MetadataVersion, but yes we allow any string. along with the provided comment it gives the reader a bit more context. If you want to remove all that and make it just str, be my guest

[DragaDoncila]

Uninstalling qtpy would cause a ModuleNotFoundError instead (properly caught).

Makes sense, thanks for catching that @aganders3!

@Czaki @jni I think it's super odd that we have qtpy in our required dependencies, but we uninstall it explicitly for headless tests. As a user I would expect that running pip install napari and then running viewer = napari.Viewer() doesn't fail. However right now, I have to uninstall qtpy myself explicitly before getting headless mode to work. I think we should update our workflow file to not uninstall qtpy when testing headless mode, and catch the broader ImportError. I will update napari #6764 appropriately.

[jni]

Since the fix for the failing CI is in napari, I suggest we merge and release this. @DragaDoncila what do you think?

[DragaDoncila]

@jni agreed, I'll merge now and push a tag