Exception from core.imports.RequirementCache

[jwnimmer-tri]

🐛 Bug

An empty egg directory on sys.path can crash RequirementCache probing.

My impression of the purpose of RequirementCache is to be a exception-free way to probe for available requirements. If the named requirement is not present, then it's __bool__ operator should just return False and never throw.

However, in one unusual corner case (of an empty egg directory on the path) the RequirementCache._check_requirement function can raise a TypeError.

This line

utilities/src/lightning_utilities/core/imports.py

Line 130 in 2549f80

pkg_version = Version(_version(req.name))

calls the packaging.version.Version constructor with the return value from importlib.metadata.version. The Version constructor is type-annotated to not accept None, but the return value of importlib.metadata.version apparently can be None (it's not documented), so the call to the Version constructor will crash with a TypeError.

Exception trace

  File "venv/site-packages/lightning_utilities/core/imports.py", line 130, in _check_requirement
    pkg_version = Version(_version(req.name))
  File "venv/site-packages/packaging/version.py", line 200, in __init__
    match = self._regex.search(version)
TypeError: expected string or bytes-like object

I imagine that a good fix would be to split up Version(_version(req.name)) into two different statements, with a None check inbetween.

Possibly the root cause is actually that importlib shouldn't be allowing the None to be returned (rather raising ModuleNotFoundError or something), but that seems somewhat difficult to fix quickly, since it's part of the standard library. Dealing with the problem at the point-of-use in this library seems like it might be a better answer.

To Reproduce

Steps to reproduce the behavior...

Code sample

jwnimmer@call-cps:~/tmp$ python3 -m venv env
jwnimmer@call-cps:~/tmp$ env/bin/pip install lightning_utilities
jwnimmer@call-cps:~/tmp$ mkdir env/lib/python3.10/site-packages/matplotlib
jwnimmer@call-cps:~/tmp$ mkdir env/lib/python3.10/site-packages/matplotlib-3.5.1.egg-info
jwnimmer@call-cps:~/tmp$ env/bin/python3 -c 'from lightning_utilities.core.imports import RequirementCache; print(bool(RequirementCache("matplotlib")))'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/jwnimmer/tmp/env/lib/python3.10/site-packages/lightning_utilities/core/imports.py", line 198, in __bool__
    self._check_available()
  File "/home/jwnimmer/tmp/env/lib/python3.10/site-packages/lightning_utilities/core/imports.py", line 164, in _check_available
    self._check_requirement()
  File "/home/jwnimmer/tmp/env/lib/python3.10/site-packages/lightning_utilities/core/imports.py", line 130, in _check_requirement
    pkg_version = Version(_version(req.name))
  File "/home/jwnimmer/tmp/env/lib/python3.10/site-packages/packaging/version.py", line 200, in __init__
    match = self._regex.search(version)
TypeError: expected string or bytes-like object

Expected behavior

The __bool__ should never raise an exception.

Additional context

Environment details

• OS (e.g., Linux): Ubuntu 22.04
• Python version: CPython 3.10 via Ubuntu 22.04

We were able to work around the bug by deleting the mistaken empty directories from our venv to avoid the hazard.

[Borda]

If the named requirement is not present, then it's __bool__ operator should just return False and never throw.

that is a good point, would you be interested in sending PR?
cc: @awaelchli

[jwnimmer-tri]

I am able to make a PR, I guess I just wasn't sure what would be the most preferred change. I could do the very narrowest change and add the proposed check for None, or I could add a new catch-all except Exception as ex: to _check_requirement that turns anything unexpected into self.available = False?