Warn on import failures within optional dependencies

qiskit/exceptions.py

@@ -15,7 +15,10 @@
 Top-level exceptions (:mod:`qiskit.exceptions`)
 ===============================================
 
-All Qiskit-related errors raised by Qiskit are subclasses of the base:
+Exceptions
+==========
+
+All Qiskit-related exceptions raised by Qiskit are subclasses of the base:
 
 .. autoexception:: QiskitError
 
@@ -42,6 +45,22 @@
 
 .. autoexception:: QiskitUserConfigError
 .. autoexception:: InvalidFileError
+
+
+Warnings
+========
+
+Some particular features of Qiskit may raise custom warnings.  In general, Qiskit will use built-in
+Python warnings (such as :exc:`DeprecationWarning`) when appropriate, but warnings related to
+Qiskit-specific functionality will be subtypes of :exc:`QiskitWarning`.
+
+.. autoexception:: QiskitWarning
+
+Related to :exc:`MissingOptionalLibraryError`, in some cases an optional dependency might be found,
+but fail to import for some other reason.  In this case, Qiskit will continue as if the dependency
+is not present, but will raise :exc:`OptionalDependencyImportWarning` to let you know about it.
+
+.. autoexception:: OptionalDependencyImportWarning
 """
 
 from typing import Optional
@@ -95,3 +114,13 @@ def __str__(self) -> str:
 
 class InvalidFileError(QiskitError):
     """Raised when the file provided is not valid for the specific task."""
+
+
+class QiskitWarning(UserWarning):
+    """Common subclass of warnings for Qiskit-specific warnings being raised."""
+
+
+class OptionalDependencyImportWarning(QiskitWarning):
+    """Raised when an optional library raises errors during its import."""
+
+    # Not a subclass of `ImportWarning` because those are hidden by default.

qiskit/qpy/exceptions.py

@@ -12,7 +12,7 @@
 
 """Exception for errors raised by the QPY module."""
 
-from qiskit.exceptions import QiskitError
+from qiskit.exceptions import QiskitError, QiskitWarning
 
 
 class QpyError(QiskitError):
@@ -28,6 +28,6 @@ def __str__(self):
         return repr(self.message)
 
 
-class QPYLoadingDeprecatedFeatureWarning(UserWarning):
+class QPYLoadingDeprecatedFeatureWarning(QiskitWarning):
     """Visible deprecation warning for QPY loading functions without
     a stable point in the call stack."""

qiskit/test/base.py

@@ -28,6 +28,7 @@
 import unittest
 from unittest.util import safe_repr
 
+from qiskit.exceptions import QiskitWarning
 from qiskit.tools.parallel import get_platform_parallel_default
 from qiskit.utils import optionals as _optionals
 from qiskit.circuit import QuantumCircuit
@@ -201,6 +202,8 @@ def setUpClass(cls):
             setup_test_logging(cls.log, os.getenv("LOG_LEVEL"), filename)
 
         warnings.filterwarnings("error", category=DeprecationWarning)
+        warnings.filterwarnings("error", category=QiskitWarning)
+
         allow_DeprecationWarning_modules = [
             "test.python.pulse.test_builder",
             "test.python.pulse.test_block",

qiskit/utils/lazy_tester.py

@@ -13,14 +13,16 @@
 """Lazy testers for optional features."""
 
 import abc
+import collections
 import contextlib
 import functools
 import importlib
 import subprocess
 import typing
+import warnings
 from typing import Union, Iterable, Dict, Optional, Callable, Type
 
-from qiskit.exceptions import MissingOptionalLibraryError
+from qiskit.exceptions import MissingOptionalLibraryError, OptionalDependencyImportWarning
 from .classtools import wrap_method
 
 
@@ -284,12 +286,43 @@ def __init__(
         super().__init__(name=name, callback=callback, install=install, msg=msg)
 
     def _is_available(self):
-        try:
-            for module, names in self._modules.items():
+        failed_modules = {}
+        failed_names = collections.defaultdict(list)
+        for module, names in self._modules.items():
+            try:
                 imported = importlib.import_module(module)
-                for name in names:
-                    getattr(imported, name)
-        except (ImportError, AttributeError):
+            except ModuleNotFoundError as exc:
+                failed_parts = exc.name.split(".")
+                target_parts = module.split(".")
+                if failed_parts == target_parts[: len(failed_parts)]:
+                    # If the module that wasn't found is the one we were explicitly searching for
+                    # (or one of its parents), then it's just not installed.
+                    return False
+                # Otherwise, we _did_ find the module, it just didn't import, which is a problem.
+                failed_modules[module] = exc
+                continue
+            except ImportError as exc:
+                failed_modules[module] = exc
+                continue
+            for name in names:
+                try:
+                    _ = getattr(imported, name)
+                except AttributeError:
+                    failed_names[module].append(name)
+        if failed_modules or failed_names:
+            package_description = f"'{self._name}'" if self._name else "optional packages"
+            message = (
+                f"While trying to import {package_description},"
+                " some components were located but raised other errors during import."
+                " You might have an incompatible version installed."
+                " Qiskit will continue as if the optional is not available."
+            )
+            for module, exc in failed_modules.items():
+                message += "".join(f"\n - module '{module}' failed to import with: {exc!r}")
+            for module, names in failed_names.items():
+                attributes = f"attribute '{names[0]}'" if len(names) == 1 else f"attributes {names}"
+                message += "".join(f"\n - '{module}' imported, but {attributes} couldn't be found")
+            warnings.warn(message, category=OptionalDependencyImportWarning)
             return False
         return True
 

releasenotes/notes/lazy-testers-warn-on-import-errors-95a9bdaacc9c3d2b.yaml

@@ -0,0 +1,14 @@
+---
+features:
+  - |
+    A new warning base class, :exc:`.QiskitWarning`, was added.  While Qiskit will continue to use
+    built-in Python warnings (such as :exc:`DeprecationWarning`) when those are most appropriate,
+    for cases that are more specific to Qiskit, the warnings will be subclasses of :exc:`.QiskitWarning`.
+  - |
+    :exc:`.QPYLoadingDeprecatedFeatureWarning` is now a subclass of :exc:`.QiskitWarning`.
+  - |
+    The optional-functionality testers (:mod:`qiskit.utils.optionals`) will now distinguish an
+    optional dependency that was completely not found (a normal situation) with one that was found,
+    but triggered errors during its import.  In the latter case, they will now issue an
+    :exc:`.OptionalDependencyImportWarning` telling you what happened, since it might indicate a
+    failed installation or an incompatible version.

test/python/utils/test_lazy_loaders.py

@@ -12,12 +12,17 @@
 
 """Tests for the lazy loaders."""
 
+from __future__ import annotations
+
+import importlib.abc
+import importlib.util
 import sys
+import warnings
 from unittest import mock
 
 import ddt
 
-from qiskit.exceptions import MissingOptionalLibraryError
+from qiskit.exceptions import MissingOptionalLibraryError, OptionalDependencyImportWarning
 from qiskit.test import QiskitTestCase
 from qiskit.utils import LazyImportTester, LazySubprocessTester
 
@@ -49,6 +54,29 @@ def mock_availability_test(feature):
     return mock.patch.object(type(feature), "_is_available", wraps=feature._is_available)
 
 
+def patch_imports(mapping: dict[str, importlib.abc.Loader]):
+    """Patch the import system so that the given named modules will skip the regular search system
+    and instead be loaded by the given loaders.
+
+    Already imported modules will not be affected; this should use uniquely named modules."""
+
+    class OverrideLoaders(importlib.abc.MetaPathFinder):
+        """A metapath finder that will simply return an explicit loader for specific modules."""
+
+        def __init__(self, mapping: dict[str, importlib.abc.Loader]):
+            self.mapping = mapping
+
+        def find_spec(self, fullname, path, target=None):
+            """Implementation of the abstract (but undefined) method."""
+            del path, target  # ABC parameters we don't need.
+            if (loader := self.mapping.get(fullname)) is not None:
+                return importlib.util.spec_from_loader(fullname, loader)
+            return None
+
+    new_path = [OverrideLoaders(mapping)] + sys.meta_path
+    return mock.patch.object(sys, "meta_path", new_path)
+
+
 @ddt.ddt
 class TestLazyDependencyTester(QiskitTestCase):
     """Tests for the lazy loaders.  Within this class, we parameterise the test cases with
@@ -71,6 +99,23 @@ def test_evaluates_correctly_false(self, test_generator):
         if test_generator():
             self.fail("did not evaluate false")
 
+    def test_submodule_import_detects_false_correctly(self):
+        """Test that a lazy import of a submodule where the parent is not available still generates
+        a silent failure."""
+
+        # The idea here is that the base package is what will fail the import, and the corresponding
+        # `ImportError.name` won't be the same as the full path we were trying to import.  We want
+        # to make sure that the "was it found and failed to import?" handling is correct in this
+        # case.
+        def checker():
+            return LazyImportTester("_qiskit_module_does_not_exist_.submodule")
+
+        # Just in case something else is allowing the warnings, but they should be forbidden by
+        # default.
+        with warnings.catch_warnings(record=True) as log:
+            self.assertFalse(checker())
+        self.assertEqual(log, [])
+
     @ddt.data(available_importer, available_process, unavailable_importer, unavailable_process)
     def test_check_occurs_once(self, test_generator):
         """Check that the test of availability is only performed once."""
@@ -324,6 +369,59 @@ class Module:
                 vars(type(mock_module))[attribute].assert_called()
             vars(type(mock_module))["unaccessed_attribute"].assert_not_called()
 
+    def test_warns_on_import_error(self):
+        """Check that the module raising an `ImportError` other than being not found is warned
+        against."""
+
+        # pylint: disable=missing-class-docstring,missing-function-docstring,abstract-method
+
+        class RaisesImportErrorOnLoad(importlib.abc.Loader):
+            def __init__(self, name):
+                self.name = name
+
+            def create_module(self, spec):
+                raise ImportError("sentinel import failure", name=self.name)
+
+            def exec_module(self, module):
+                pass
+
+        dummy = f"{__name__}_{type(self).__name__}_test_warns_on_import_error".replace(".", "_")
+        tester = LazyImportTester(dummy)
+        with patch_imports({dummy: RaisesImportErrorOnLoad(dummy)}):
+            with self.assertWarnsRegex(
+                OptionalDependencyImportWarning,
+                rf"module '{dummy}' failed to import with: .*sentinel import failure.*",
+            ):
+                self.assertFalse(tester)
+
+    def test_warns_on_internal_not_found_error(self):
+        """Check that the module raising an `ModuleNotFoundError` for some module other than itself
+        (such as a module trying to import parts of Terra that don't exist any more) is caught and
+        warned against, rather than silently caught as an expected `ModuleNotFoundError`."""
+
+        # pylint: disable=missing-class-docstring,missing-function-docstring,abstract-method
+
+        class ImportsBadModule(importlib.abc.Loader):
+            def create_module(self, spec):
+                # Doesn't matter what, we just want to return any module object; we're going to
+                # raise an error during "execution" of the module.
+                return sys
+
+            def exec_module(self, module):
+                del module  # ABC parameter we don't care about.
+                import __qiskit__some_module_that_does_not_exist
+
+        dummy = f"{__name__}_{type(self).__name__}_test_warns_on_internal_not_found_error".replace(
+            ".", "_"
+        )
+        tester = LazyImportTester(dummy)
+        with patch_imports({dummy: ImportsBadModule()}):
+            with self.assertWarnsRegex(
+                OptionalDependencyImportWarning,
+                rf"module '{dummy}' failed to import with: ModuleNotFoundError.*__qiskit__",
+            ):
+                self.assertFalse(tester)
+
     def test_import_allows_attributes_failure(self):
         """Check that the import tester can accept a dictionary mapping module names to attributes,
         and that these are recognised when they are missing."""
@@ -334,7 +432,8 @@ def test_import_allows_attributes_failure(self):
         }
 
         feature = LazyImportTester(name_map)
-        self.assertFalse(feature)
+        with self.assertWarnsRegex(UserWarning, r"'builtins' imported, but attribute"):
+            self.assertFalse(feature)
 
     def test_import_fails_with_no_modules(self):
         """Catch programmer errors with no modules to test."""