diff --git a/docs/changes/newsfragments/8252.breaking b/docs/changes/newsfragments/8252.breaking new file mode 100644 index 00000000000..c102b9d4061 --- /dev/null +++ b/docs/changes/newsfragments/8252.breaking @@ -0,0 +1,14 @@ +Importing the short hand aliases (such as ``Parameter``, ``Measurement``, +``Instrument``, ``Monitor`` and ``Station``) from the top level ``qcodes`` +namespace is now deprecated and emits a ``QCoDeSDeprecationWarning``. Import +these names from their respective submodules (for example ``qcodes.parameters``, +``qcodes.dataset`` or ``qcodes.station``) instead. The submodules themselves +(such as ``qcodes.dataset`` and ``qcodes.instrument``) remain accessible from the +top level ``qcodes`` namespace and are not deprecated. + +Similarly, importing the parameter classes (such as ``Parameter``, ``ManualParameter`` and +``DelegateParameter``) from ``qcodes.instrument`` is now deprecated and emits a +``QCoDeSDeprecationWarning``. Import these names from ``qcodes.parameters`` +instead. The names remain available from ``qcodes.instrument`` at runtime for +backwards compatibility but are hidden from static type checkers, so importing +them from ``qcodes.instrument`` is reported as an unknown attribute. diff --git a/docs/changes/newsfragments/8252.underthehood b/docs/changes/newsfragments/8252.underthehood new file mode 100644 index 00000000000..458eefbc90f --- /dev/null +++ b/docs/changes/newsfragments/8252.underthehood @@ -0,0 +1,10 @@ +Importing the top level ``qcodes`` namespace no longer eagerly imports the +``dataset``, ``instrument``, ``parameters``, ``monitor``, ``station`` and +``validators`` submodules. The discouraged short hand aliases (such as +``qcodes.Parameter`` and ``qcodes.Measurement``) as well as ``qcodes.validators`` +are still available at runtime via a lazy module level ``__getattr__``, but they +are intentionally hidden from static type checkers and are now reported as +unknown attributes rather than typed as ``Any``; import the names from their +respective submodules to retain type information. This breaks a large import +cycle spanning the parameters, dataset and instrument packages that triggered an +internal error in ``mypy`` 2.2. diff --git a/docs/examples/Parameters/Legacy_Parameters.ipynb b/docs/examples/Parameters/Legacy_Parameters.ipynb index e0f078b70f8..27929fa737f 100644 --- a/docs/examples/Parameters/Legacy_Parameters.ipynb +++ b/docs/examples/Parameters/Legacy_Parameters.ipynb @@ -32,7 +32,7 @@ } ], "source": [ - "from qcodes.instrument import ArrayParameter\n", + "from qcodes.parameters import ArrayParameter\n", "\n", "\n", "class ArrayCounter(ArrayParameter):\n", diff --git a/src/qcodes/__init__.py b/src/qcodes/__init__.py index e6f0b8d4674..fd2abac5b89 100644 --- a/src/qcodes/__init__.py +++ b/src/qcodes/__init__.py @@ -1,14 +1,8 @@ """Set up the main qcodes namespace.""" -# ruff: noqa: F401, E402 -# This module still contains a lot of short hand imports -# since these imports are discouraged and they are officially -# added elsewhere under their respective submodules we cannot add -# them to __all__ here so silence the warning. - -# config +import importlib import warnings -from typing import Any +from typing import TYPE_CHECKING, Any import qcodes._version import qcodes.configuration as qcconfig @@ -22,58 +16,6 @@ conditionally_start_all_logging() -import atexit - -import qcodes.validators -from qcodes.dataset import ( - Measurement, - ParamSpec, - SQLiteSettings, - experiments, - get_guids_by_run_spec, - initialise_database, - initialise_or_create_database_at, - initialised_database_at, - load_by_counter, - load_by_guid, - load_by_id, - load_by_run_spec, - load_experiment, - load_experiment_by_name, - load_last_experiment, - load_or_create_experiment, - new_data_set, - new_experiment, -) -from qcodes.instrument import ( - ChannelList, - ChannelTuple, - Instrument, - InstrumentChannel, - IPInstrument, - VisaInstrument, - find_or_create_instrument, -) -from qcodes.monitor import Monitor -from qcodes.parameters import ( - ArrayParameter, - CombinedParameter, - DelegateParameter, - Function, - ManualParameter, - MultiParameter, - Parameter, - ParameterWithSetpoints, - ScaledParameter, - SweepFixedValues, - SweepValues, - combine, -) -from qcodes.station import Station - -# ensure to close all instruments when interpreter is closed -atexit.register(Instrument.close_all) - if config.core.import_legacy_api: warnings.warn( "`core.import_legacy_api` and `gui.plotlib` config option has no effect " @@ -81,3 +23,109 @@ "Please avoid setting this in your `qcodesrc.json` config file.", QCoDeSDeprecationWarning, ) + +# The following names are re-exported for backwards compatibility as short hand +# for the objects in their respective submodules. Importing them from the top +# level ``qcodes`` namespace is deprecated (import them from the submodule +# instead); they are provided lazily via a module level ``__getattr__`` that +# emits a ``QCoDeSDeprecationWarning``. Importing them eagerly here would also +# pull ``qcodes.dataset``, ``qcodes.instrument``, ``qcodes.parameters``, +# ``qcodes.monitor`` and ``qcodes.station`` into a single large import cycle at +# type-check time (which triggers an internal error in mypy >= 2.2). +_LAZY_NAME_TO_MODULE = ( + { + name: "qcodes.dataset" + for name in ( + "Measurement", + "ParamSpec", + "SQLiteSettings", + "experiments", + "get_guids_by_run_spec", + "initialise_database", + "initialise_or_create_database_at", + "initialised_database_at", + "load_by_counter", + "load_by_guid", + "load_by_id", + "load_by_run_spec", + "load_experiment", + "load_experiment_by_name", + "load_last_experiment", + "load_or_create_experiment", + "new_data_set", + "new_experiment", + ) + } + | { + name: "qcodes.instrument" + for name in ( + "ChannelList", + "ChannelTuple", + "Instrument", + "InstrumentChannel", + "IPInstrument", + "VisaInstrument", + "find_or_create_instrument", + ) + } + | { + name: "qcodes.parameters" + for name in ( + "ArrayParameter", + "CombinedParameter", + "DelegateParameter", + "Function", + "ManualParameter", + "MultiParameter", + "Parameter", + "ParameterWithSetpoints", + "ScaledParameter", + "SweepFixedValues", + "SweepValues", + "combine", + ) + } + | { + "Monitor": "qcodes.monitor", + "Station": "qcodes.station", + } +) + + +# These public submodules are exposed lazily so that ``import qcodes`` stays +# cheap and does not force the submodules (and their dependencies) to be +# imported. Accessing them is not deprecated. The submodules that the deprecated +# short hands above live in are included here so that ``qcodes.dataset`` and +# friends keep working as submodule accessors once the deprecated short hands are +# eventually removed. +_LAZY_SUBMODULES = frozenset( + { + "validators", + "dataset", + "instrument", + "parameters", + "monitor", + "station", + } +) + + +# The lazy ``__getattr__`` is intentionally hidden from static type checkers via +# ``if not TYPE_CHECKING`` so that these discouraged short hands are reported as +# unknown attributes (rather than typed as ``Any``); import the names from their +# respective submodules to get proper type information. +if not TYPE_CHECKING: + + def __getattr__(name: str) -> Any: + if name in _LAZY_SUBMODULES: + return importlib.import_module(f"{__name__}.{name}") + module_name = _LAZY_NAME_TO_MODULE.get(name) + if module_name is not None: + warnings.warn( + f"Importing {name!r} from the top level {__name__!r} namespace " + f"is deprecated. Import it from {module_name!r} instead.", + QCoDeSDeprecationWarning, + stacklevel=2, + ) + return getattr(importlib.import_module(module_name), name) + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/src/qcodes/instrument/__init__.py b/src/qcodes/instrument/__init__.py index 5713b58b592..90e88e97dca 100644 --- a/src/qcodes/instrument/__init__.py +++ b/src/qcodes/instrument/__init__.py @@ -1,19 +1,9 @@ -# left here for backwards compatibility -# but not part of the api officially -from qcodes.parameters import ( # noqa: F401 - ArrayParameter, - CombinedParameter, - DelegateParameter, - Function, - ManualParameter, - MultiParameter, - Parameter, - ParameterWithSetpoints, - ScaledParameter, - SweepFixedValues, - SweepValues, - combine, -) +import atexit +import importlib +import warnings +from typing import TYPE_CHECKING, Any + +from qcodes.utils import QCoDeSDeprecationWarning from .channel import ChannelList, ChannelTuple, InstrumentChannel, InstrumentModule from .instrument import Instrument, find_or_create_instrument @@ -21,6 +11,11 @@ from .ip import IPInstrument from .visa import VisaInstrument, VisaInstrumentKWArgs +# ensure that all instruments are closed when the interpreter is shut down. +# this is registered here rather than in ``qcodes.__init__`` so that importing +# the top level ``qcodes`` package does not eagerly import ``qcodes.instrument``. +atexit.register(Instrument.close_all) + __all__ = [ "ChannelList", "ChannelTuple", @@ -34,3 +29,43 @@ "VisaInstrumentKWArgs", "find_or_create_instrument", ] + +# The following parameter classes used to be re-exported from +# ``qcodes.instrument`` for backwards compatibility. They now live in +# ``qcodes.parameters`` and importing them from here is deprecated. They are +# provided lazily via a module level ``__getattr__`` that emits a deprecation +# warning; import them from ``qcodes.parameters`` instead. +_DEPRECATED_PARAMETER_NAMES = frozenset( + { + "ArrayParameter", + "CombinedParameter", + "DelegateParameter", + "Function", + "ManualParameter", + "MultiParameter", + "Parameter", + "ParameterWithSetpoints", + "ScaledParameter", + "SweepFixedValues", + "SweepValues", + "combine", + } +) + + +# The lazy ``__getattr__`` is intentionally hidden from static type checkers via +# ``if not TYPE_CHECKING`` so that importing these deprecated names from +# ``qcodes.instrument`` is reported as an unknown attribute; import them from +# ``qcodes.parameters`` instead. +if not TYPE_CHECKING: + + def __getattr__(name: str) -> Any: + if name in _DEPRECATED_PARAMETER_NAMES: + warnings.warn( + f"Importing {name!r} from {__name__!r} is deprecated. " + f"Import it from 'qcodes.parameters' instead.", + QCoDeSDeprecationWarning, + stacklevel=2, + ) + return getattr(importlib.import_module("qcodes.parameters"), name) + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/src/qcodes/instrument/instrument.py b/src/qcodes/instrument/instrument.py index 70ea6d203c3..2f52e6670df 100644 --- a/src/qcodes/instrument/instrument.py +++ b/src/qcodes/instrument/instrument.py @@ -180,7 +180,10 @@ def close(self) -> None: self.remove_instance(self) @classmethod - def close_all(cls) -> None: + def close_all( + cls, + log_status: bool = False, + ) -> None: """ Try to close all instruments registered in ``_all_instruments`` This is handy for use with atexit to @@ -190,15 +193,22 @@ def close_all(cls) -> None: Examples: >>> atexit.register(qc.Instrument.close_all()) + Args: + log_status: If True, log the status of closing each instrument. Set this to False + if you want to avoid logging during interpreter shutdown, which can cause errors. + """ - log.info("Closing all registered instruments") + if log_status: + log.info("Closing all registered instruments") for inststr in list(cls._all_instruments): try: inst: Instrument = cls.find_instrument(inststr) - log.info("Closing %s", inststr) + if log_status: + log.info("Closing %s", inststr) inst.close() except Exception: - log.exception("Failed to close %s, ignored", inststr) + if log_status: + log.exception("Failed to close %s, ignored", inststr) @classmethod def record_instance(cls, instance: Instrument) -> None: diff --git a/tests/test_import.py b/tests/test_import.py new file mode 100644 index 00000000000..d3e553ff278 --- /dev/null +++ b/tests/test_import.py @@ -0,0 +1,130 @@ +"""Tests for the lazily provided attributes on the top level ``qcodes`` +namespace and on ``qcodes.instrument``. + +The short hand aliases on ``qcodes`` (such as ``qcodes.Parameter``) and the +parameter classes re-exported from ``qcodes.instrument`` are provided lazily via +module level ``__getattr__`` hooks that are hidden from static type checkers. +Accessing them still works at runtime for backwards compatibility but is +deprecated: it emits a ``QCoDeSDeprecationWarning`` at runtime *and* is reported +as an unknown attribute by static type checkers. The ``# type: ignore`` pragmas +below document (and, via the pre-commit type checkers, assert) that these lines +are indeed type errors. Importing the names from their respective submodules is +the supported, statically typed alternative. +""" + +from __future__ import annotations + +import warnings + +import pytest + +import qcodes +import qcodes.dataset +import qcodes.instrument +import qcodes.monitor +import qcodes.parameters +import qcodes.station +import qcodes.validators +from qcodes.utils import QCoDeSDeprecationWarning + + +def test_top_level_parameter_shorthand_deprecated() -> None: + with pytest.warns( + QCoDeSDeprecationWarning, match=r"top level.*'qcodes\.parameters'" + ): + obj = qcodes.Parameter # type: ignore[attr-defined] + assert obj is qcodes.parameters.Parameter + + +def test_top_level_combine_shorthand_deprecated() -> None: + with pytest.warns( + QCoDeSDeprecationWarning, match=r"top level.*'qcodes\.parameters'" + ): + obj = qcodes.combine # type: ignore[attr-defined] + assert obj is qcodes.parameters.combine + + +def test_top_level_measurement_shorthand_deprecated() -> None: + with pytest.warns(QCoDeSDeprecationWarning, match=r"top level.*'qcodes\.dataset'"): + obj = qcodes.Measurement # type: ignore[attr-defined] + assert obj is qcodes.dataset.Measurement + + +def test_top_level_instrument_shorthand_deprecated() -> None: + with pytest.warns( + QCoDeSDeprecationWarning, match=r"top level.*'qcodes\.instrument'" + ): + obj = qcodes.Instrument # type: ignore[attr-defined] + assert obj is qcodes.instrument.Instrument + + +def test_top_level_monitor_shorthand_deprecated() -> None: + with pytest.warns(QCoDeSDeprecationWarning, match=r"top level.*'qcodes\.monitor'"): + obj = qcodes.Monitor # type: ignore[attr-defined] + assert obj is qcodes.monitor.Monitor + + +def test_top_level_station_shorthand_deprecated() -> None: + with pytest.warns(QCoDeSDeprecationWarning, match=r"top level.*'qcodes\.station'"): + obj = qcodes.Station # type: ignore[attr-defined] + assert obj is qcodes.station.Station + + +def test_top_level_submodule_access_not_deprecated() -> None: + """Accessing the (imported) submodules from ``qcodes`` is statically typed + and does not emit a deprecation warning.""" + with warnings.catch_warnings(): + warnings.simplefilter("error", QCoDeSDeprecationWarning) + assert qcodes.dataset is not None + assert qcodes.instrument is not None + assert qcodes.parameters is not None + assert qcodes.monitor is not None + assert qcodes.station is not None + assert qcodes.validators is not None + + +def test_top_level_unknown_attribute_raises() -> None: + with pytest.raises(AttributeError, match="definitely_not_a_qcodes_attribute"): + qcodes.definitely_not_a_qcodes_attribute # type: ignore[attr-defined] + + +def test_instrument_parameter_reexport_deprecated() -> None: + """Document that a parameter class accessed from ``qcodes.instrument`` is + both a runtime deprecation warning and a static type error.""" + with pytest.warns( + QCoDeSDeprecationWarning, match=r"'ManualParameter'.*'qcodes\.parameters'" + ): + obj = qcodes.instrument.ManualParameter # type: ignore[attr-defined] + assert obj is qcodes.parameters.ManualParameter + + +@pytest.mark.parametrize( + "name", + [ + "ArrayParameter", + "CombinedParameter", + "DelegateParameter", + "Function", + "ManualParameter", + "MultiParameter", + "Parameter", + "ParameterWithSetpoints", + "ScaledParameter", + "SweepFixedValues", + "SweepValues", + "combine", + ], +) +def test_instrument_parameter_reexport_deprecated_all(name: str) -> None: + """Every parameter class re-exported from ``qcodes.instrument`` still + resolves to the object in ``qcodes.parameters`` and emits a warning.""" + with pytest.warns( + QCoDeSDeprecationWarning, match=rf"{name!r}.*'qcodes\.parameters'" + ): + obj = getattr(qcodes.instrument, name) + assert obj is getattr(qcodes.parameters, name) + + +def test_instrument_unknown_attribute_raises() -> None: + with pytest.raises(AttributeError, match="definitely_not_a_qcodes_attribute"): + qcodes.instrument.definitely_not_a_qcodes_attribute # type: ignore[attr-defined]