diff --git a/qiskit/exceptions.py b/qiskit/exceptions.py index 048f79dfbc62..e26d4097fa90 100644 --- a/qiskit/exceptions.py +++ b/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. diff --git a/qiskit/qpy/exceptions.py b/qiskit/qpy/exceptions.py index 476e39eaa337..c6cdb4303a62 100644 --- a/qiskit/qpy/exceptions.py +++ b/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.""" diff --git a/qiskit/test/base.py b/qiskit/test/base.py index 1dde885bd67f..cfba5bf610d7 100644 --- a/qiskit/test/base.py +++ b/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", diff --git a/qiskit/utils/lazy_tester.py b/qiskit/utils/lazy_tester.py index 4f0d8e5a37aa..d3e5cd971185 100644 --- a/qiskit/utils/lazy_tester.py +++ b/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 diff --git a/releasenotes/notes/lazy-testers-warn-on-import-errors-95a9bdaacc9c3d2b.yaml b/releasenotes/notes/lazy-testers-warn-on-import-errors-95a9bdaacc9c3d2b.yaml new file mode 100644 index 000000000000..0a3167cc1f66 --- /dev/null +++ b/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. diff --git a/test/python/utils/test_lazy_loaders.py b/test/python/utils/test_lazy_loaders.py index 614bea15f04d..7a8c16eda2fe 100644 --- a/test/python/utils/test_lazy_loaders.py +++ b/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."""