From 3621e155f3939ea32fdfa62957aff5c5ff0db406 Mon Sep 17 00:00:00 2001 From: Mikhail Astafev Date: Thu, 30 Jul 2026 14:41:16 +0200 Subject: [PATCH 1/5] Add 'Only_invalid' snapshot update mode Add string update values `All`/`Only_invalid`/`Never` to `snapshot`/`snapshot_base` (from `Metadatable` down), keeping the legacy `True`/`None`/`False` as aliases. `Only_invalid` snapshots only refresh parameters whose cache is invalid via `cache.get(get_if_invalid=True)`. The station snapshot taken by `Measurement` now uses `Only_invalid`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa3f57b5-ddc1-45ce-b920-3f8efed6851c --- src/qcodes/dataset/measurements.py | 2 +- src/qcodes/instrument/channel.py | 4 +- src/qcodes/instrument/instrument_base.py | 14 ++- src/qcodes/instrument/ip.py | 4 +- src/qcodes/instrument/visa.py | 3 +- .../Keithley/_Keithley_2600.py | 4 +- .../Keysight/keysightb1500/KeysightB1517A.py | 3 +- .../instrument_drivers/QDev/QDac_channels.py | 8 +- .../american_magnetics/AMI430_visa.py | 6 +- .../mock_instruments/__init__.py | 4 +- .../stanford_research/SR86x.py | 3 +- src/qcodes/metadatable/__init__.py | 4 +- src/qcodes/metadatable/metadatable_base.py | 57 ++++++++- src/qcodes/parameters/combined_parameter.py | 7 +- src/qcodes/parameters/delegate_parameter.py | 3 +- src/qcodes/parameters/parameter_base.py | 24 ++-- src/qcodes/parameters/sweep_values.py | 3 +- src/qcodes/station.py | 16 +-- tests/dataset/test_snapshot.py | 57 ++++++++- tests/parameter/test_snapshot.py | 110 ++++++++++++++++++ 20 files changed, 293 insertions(+), 43 deletions(-) diff --git a/src/qcodes/dataset/measurements.py b/src/qcodes/dataset/measurements.py index 3437bbb8a7c6..83603b9037ad 100644 --- a/src/qcodes/dataset/measurements.py +++ b/src/qcodes/dataset/measurements.py @@ -663,7 +663,7 @@ def __enter__(self) -> DataSaver: station = self.station if station is not None: - snapshot = {"station": station.snapshot()} + snapshot = {"station": station.snapshot(update="Only_invalid")} else: snapshot = {} if self._registered_parameters is not None: diff --git a/src/qcodes/instrument/channel.py b/src/qcodes/instrument/channel.py index 1e807320e4da..a4e098eea3e2 100644 --- a/src/qcodes/instrument/channel.py +++ b/src/qcodes/instrument/channel.py @@ -23,6 +23,8 @@ if TYPE_CHECKING: from typing import Unpack + from qcodes.metadatable import SnapshotUpdate + from .instrument_base import InstrumentBaseKWArgs @@ -380,7 +382,7 @@ def get_validator(self) -> ChannelTupleValidator: def snapshot_base( self, - update: bool | None = True, + update: bool | SnapshotUpdate | None = True, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ diff --git a/src/qcodes/instrument/instrument_base.py b/src/qcodes/instrument/instrument_base.py index c3030c102200..d316814a84b5 100644 --- a/src/qcodes/instrument/instrument_base.py +++ b/src/qcodes/instrument/instrument_base.py @@ -21,6 +21,7 @@ from qcodes.instrument.channel import ChannelTuple, InstrumentModule from qcodes.logger.instrument_logger import InstrumentLoggerAdapter + from qcodes.metadatable import SnapshotUpdate from qcodes.utils import QCoDeSDeprecationWarning @@ -407,7 +408,7 @@ def _get_component_by_name( def snapshot_base( self, - update: bool | None = False, + update: bool | SnapshotUpdate | None = False, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -417,10 +418,11 @@ def snapshot_base( supports). Args: - update: If ``True``, update the state by querying the - instrument. If None update the state if known to be invalid. - If ``False``, just use the latest values in memory and never - update state. + update: If ``"All"`` (or legacy ``True``), update the state by + querying the instrument. If ``"Only_invalid"`` (or legacy + ``None``) update the state only for values known to be invalid. + If ``"Never"`` (or legacy ``False``), just use the latest + values in memory and never update state. params_to_skip_update: List of parameter names that will be skipped in update even if update is True. This is useful if you have parameters that are slow to update but can be updated in a @@ -453,7 +455,7 @@ def snapshot_base( if param.snapshot_exclude: continue if params_to_skip_update and name in params_to_skip_update: - update_par: bool | None = False + update_par: bool | SnapshotUpdate | None = False else: update_par = update try: diff --git a/src/qcodes/instrument/ip.py b/src/qcodes/instrument/ip.py index 28940d80f81e..6221668fc46f 100644 --- a/src/qcodes/instrument/ip.py +++ b/src/qcodes/instrument/ip.py @@ -13,6 +13,8 @@ from types import TracebackType from typing import Unpack + from qcodes.metadatable import SnapshotUpdate + from .instrument_base import InstrumentBaseKWArgs log = logging.getLogger(__name__) @@ -213,7 +215,7 @@ def ask_raw(self, cmd: str) -> str: def snapshot_base( self, - update: bool | None = False, + update: bool | SnapshotUpdate | None = False, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ diff --git a/src/qcodes/instrument/visa.py b/src/qcodes/instrument/visa.py index 4d91ce6b7e24..13c5f7e238b2 100644 --- a/src/qcodes/instrument/visa.py +++ b/src/qcodes/instrument/visa.py @@ -24,6 +24,7 @@ from collections.abc import Mapping, Sequence from typing import NotRequired, Unpack + from qcodes.metadatable import SnapshotUpdate from qcodes.parameters.parameter import Parameter VISA_LOGGER = ".".join((InstrumentBase.__module__, "com", "visa")) @@ -450,7 +451,7 @@ def ask_raw(self, cmd: str) -> str: def snapshot_base( self, - update: bool | None = True, + update: bool | SnapshotUpdate | None = True, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ diff --git a/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py b/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py index 21ff305c7f78..ff5d32c829ac 100644 --- a/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py +++ b/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py @@ -29,6 +29,8 @@ from collections.abc import Callable, Sequence from typing import Unpack, assert_never + from qcodes.metadatable import SnapshotUpdate + log = logging.getLogger(__name__) @@ -566,7 +568,7 @@ def _parse_response(data: str) -> tuple[float, Keithley2600MeasurementStatus]: def snapshot_base( self, - update: bool | None = True, + update: bool | SnapshotUpdate | None = True, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: snapshot = super().snapshot_base( diff --git a/src/qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1517A.py b/src/qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1517A.py index 94e67aab13a8..0227622dfe8f 100644 --- a/src/qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1517A.py +++ b/src/qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1517A.py @@ -33,6 +33,7 @@ from qcodes.instrument_drivers.Keysight.keysightb1500.KeysightB1500_base import ( KeysightB1500, ) + from qcodes.metadatable import SnapshotUpdate class SweepSteps(TypedDict): @@ -713,7 +714,7 @@ def measurement_status(self) -> MeasurementStatus | None: def snapshot_base( self, - update: bool | None = True, + update: "bool | SnapshotUpdate | None" = True, params_to_skip_update: "Sequence[str] | None" = None, ) -> dict[Any, Any]: snapshot = super().snapshot_base( diff --git a/src/qcodes/instrument_drivers/QDev/QDac_channels.py b/src/qcodes/instrument_drivers/QDev/QDac_channels.py index 67b95e44ae6e..42aeafebb00b 100644 --- a/src/qcodes/instrument_drivers/QDev/QDac_channels.py +++ b/src/qcodes/instrument_drivers/QDev/QDac_channels.py @@ -17,12 +17,14 @@ VisaInstrument, VisaInstrumentKWArgs, ) +from qcodes.metadatable.metadatable_base import _normalize_snapshot_update from qcodes.parameters import MultiChannelInstrumentParameter, ParamRawDataType if TYPE_CHECKING: from collections.abc import Sequence from typing import Unpack + from qcodes.metadatable import SnapshotUpdate from qcodes.parameters import Parameter log = logging.getLogger(__name__) @@ -139,9 +141,10 @@ def __init__( def snapshot_base( self, - update: bool | None = False, + update: "bool | SnapshotUpdate | None" = False, params_to_skip_update: "Sequence[str] | None" = None, ) -> dict[Any, Any]: + update = _normalize_snapshot_update(update) # setting update not None will override parent setting # otherwise we use parent setting # parent._update | update | do update @@ -337,9 +340,10 @@ def __init__( def snapshot_base( self, - update: bool | None = False, + update: "bool | SnapshotUpdate | None" = False, params_to_skip_update: "Sequence[str] | None" = None, ) -> dict[Any, Any]: + update = _normalize_snapshot_update(update) update_currents = self._update_currents and update is True if update: self._update_cache(readcurrents=update_currents) diff --git a/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py b/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py index 651b76822197..834de1a61a0a 100644 --- a/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py +++ b/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py @@ -19,6 +19,7 @@ VisaInstrumentKWArgs, ) from qcodes.math_utils import FieldVector +from qcodes.metadatable.metadatable_base import _normalize_snapshot_update from qcodes.parameters import Parameter from qcodes.utils.types import NumberType from qcodes.validators import Anything, Bool, Enum, Ints, Numbers @@ -26,6 +27,8 @@ if TYPE_CHECKING: from typing import Unpack + from qcodes.metadatable import SnapshotUpdate + log = logging.getLogger(__name__) @@ -149,9 +152,10 @@ def _check_state(self) -> bool: def snapshot_base( self, - update: bool | None = False, + update: bool | SnapshotUpdate | None = False, params_to_skip_update: Sequence[str] | None = None, ) -> dict[str, Any]: + update = _normalize_snapshot_update(update) if params_to_skip_update is None: params_to_skip_update = [] diff --git a/src/qcodes/instrument_drivers/mock_instruments/__init__.py b/src/qcodes/instrument_drivers/mock_instruments/__init__.py index edd248da108d..048c05aa299c 100644 --- a/src/qcodes/instrument_drivers/mock_instruments/__init__.py +++ b/src/qcodes/instrument_drivers/mock_instruments/__init__.py @@ -29,6 +29,8 @@ from collections.abc import Generator, Sequence from typing import Unpack + from qcodes.metadatable import SnapshotUpdate + log = logging.getLogger(__name__) @@ -1067,7 +1069,7 @@ def _getter(self, name: str) -> ParamRawDataType: def snapshot_base( self, - update: bool | None = True, + update: bool | SnapshotUpdate | None = True, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: if params_to_skip_update is None: diff --git a/src/qcodes/instrument_drivers/stanford_research/SR86x.py b/src/qcodes/instrument_drivers/stanford_research/SR86x.py index a7bdc84d3aad..091f4f1e4875 100644 --- a/src/qcodes/instrument_drivers/stanford_research/SR86x.py +++ b/src/qcodes/instrument_drivers/stanford_research/SR86x.py @@ -21,6 +21,7 @@ from collections.abc import Callable, Sequence from typing import Unpack + from qcodes.metadatable import SnapshotUpdate from qcodes.parameters import Parameter log = logging.getLogger(__name__) @@ -210,7 +211,7 @@ def __init__( def snapshot_base( self, - update: bool | None = False, + update: bool | SnapshotUpdate | None = False, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: if params_to_skip_update is None: diff --git a/src/qcodes/metadatable/__init__.py b/src/qcodes/metadatable/__init__.py index a3c097e7fcaf..e6e4e4c42062 100644 --- a/src/qcodes/metadatable/__init__.py +++ b/src/qcodes/metadatable/__init__.py @@ -1,3 +1,3 @@ -from .metadatable_base import Metadatable, MetadatableWithName +from .metadatable_base import Metadatable, MetadatableWithName, SnapshotUpdate -__all__ = ["Metadatable", "MetadatableWithName"] +__all__ = ["Metadatable", "MetadatableWithName", "SnapshotUpdate"] diff --git a/src/qcodes/metadatable/metadatable_base.py b/src/qcodes/metadatable/metadatable_base.py index a37897b4ee21..ddf10607de62 100644 --- a/src/qcodes/metadatable/metadatable_base.py +++ b/src/qcodes/metadatable/metadatable_base.py @@ -1,11 +1,54 @@ from abc import abstractmethod -from typing import TYPE_CHECKING, Any, final +from typing import TYPE_CHECKING, Any, Literal, final from qcodes.utils import deep_update if TYPE_CHECKING: from collections.abc import Mapping, Sequence +SnapshotUpdate = Literal["All", "Only_invalid", "Never"] +""" +Allowed string values for the ``update`` argument of ``snapshot`` and +``snapshot_base``: + +* ``"All"``: force an update of every value (equivalent to legacy ``True``). +* ``"Only_invalid"``: only update values whose cache is invalid, using the + latest cached value otherwise (equivalent to legacy ``None``). +* ``"Never"``: never update, always use the latest values in memory + (equivalent to legacy ``False``). +""" + +_SNAPSHOT_UPDATE_ALIASES: "dict[str, bool | None]" = { + "All": True, + "Only_invalid": None, + "Never": False, +} + + +def _normalize_snapshot_update( + update: "bool | SnapshotUpdate | None", +) -> "bool | None": + """ + Normalize the ``update`` argument of ``snapshot``/``snapshot_base`` into + its legacy ``bool | None`` representation, where ``True`` means update all + values, ``None`` means update only values with an invalid cache, and + ``False`` means never update. + + The string values ``"All"``, ``"Only_invalid"`` and ``"Never"`` are mapped + to ``True``, ``None`` and ``False`` respectively. ``bool`` and ``None`` + values are returned unchanged for backwards compatibility. + """ + if isinstance(update, str): + try: + return _SNAPSHOT_UPDATE_ALIASES[update] + except KeyError: + raise ValueError( + f"Invalid value for snapshot ``update``: {update!r}. Expected " + f"one of {list(_SNAPSHOT_UPDATE_ALIASES)}, or a bool, or None." + ) from None + return update + + # NB: At the moment, the Snapshot type is a bit weak, as the Any # for the value type doesn't tell us anything about the schema # followed by snapshots. @@ -34,21 +77,25 @@ def load_metadata(self, metadata: "Mapping[str, Any]") -> None: deep_update(self.metadata, metadata) @final - def snapshot(self, update: bool | None = False) -> Snapshot: + def snapshot(self, update: "bool | SnapshotUpdate | None" = False) -> Snapshot: """ Decorate a snapshot dictionary with metadata. DO NOT override this method if you want metadata in the snapshot instead, override :meth:`snapshot_base`. Args: - update: Passed to snapshot_base. + update: Passed to snapshot_base. Accepts the string values + ``"All"``, ``"Only_invalid"`` and ``"Never"`` as well as the + legacy values ``True`` (equivalent to ``"All"``), ``None`` + (equivalent to ``"Only_invalid"``) and ``False`` (equivalent + to ``"Never"``). Returns: Base snapshot. """ - snap = self.snapshot_base(update=update) + snap = self.snapshot_base(update=_normalize_snapshot_update(update)) if len(self.metadata): snap["metadata"] = self.metadata @@ -57,7 +104,7 @@ def snapshot(self, update: bool | None = False) -> Snapshot: def snapshot_base( self, - update: bool | None = False, + update: "bool | SnapshotUpdate | None" = False, params_to_skip_update: "Sequence[str] | None" = None, ) -> Snapshot: """ diff --git a/src/qcodes/parameters/combined_parameter.py b/src/qcodes/parameters/combined_parameter.py index 44f70b13912e..f38780fd773e 100644 --- a/src/qcodes/parameters/combined_parameter.py +++ b/src/qcodes/parameters/combined_parameter.py @@ -14,6 +14,8 @@ if TYPE_CHECKING: from collections.abc import Callable, Iterator, Sequence + from qcodes.metadatable import SnapshotUpdate + from .parameter import Parameter _LOG = logging.getLogger(__name__) @@ -194,7 +196,7 @@ def __len__(self) -> int: def snapshot_base( self, - update: bool | None = False, + update: bool | SnapshotUpdate | None = False, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -203,7 +205,8 @@ def snapshot_base( :class:`.NumpyJSONEncoder` supports). Args: - update: ``True`` or ``False``. + update: ``"All"``, ``"Only_invalid"`` or ``"Never"`` (or the legacy + ``True`` / ``None`` / ``False``). params_to_skip_update: Unused in this subclass. Returns: diff --git a/src/qcodes/parameters/delegate_parameter.py b/src/qcodes/parameters/delegate_parameter.py index df8b2f6f2628..203a5916be50 100644 --- a/src/qcodes/parameters/delegate_parameter.py +++ b/src/qcodes/parameters/delegate_parameter.py @@ -13,6 +13,7 @@ from typing import Unpack from qcodes.instrument import InstrumentBase + from qcodes.metadatable import SnapshotUpdate from qcodes.validators.validators import Validator from .parameter_base import ( @@ -324,7 +325,7 @@ def set_raw(self, value: Any) -> None: def snapshot_base( self, - update: bool | None = True, + update: bool | SnapshotUpdate | None = True, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: snapshot = super().snapshot_base( diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index dcbaa6b60bd0..062e74473fb8 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -13,7 +13,8 @@ import numpy as np from typing_extensions import TypedDict, TypeVar -from qcodes.metadatable import Metadatable, MetadatableWithName +from qcodes.metadatable import Metadatable, MetadatableWithName, SnapshotUpdate +from qcodes.metadatable.metadatable_base import _normalize_snapshot_update from qcodes.parameters import ParamSpecBase from qcodes.utils import ( DelegateAttributes, @@ -699,7 +700,7 @@ def __call__(self, *args: Any, **kwargs: Any) -> ParameterDataTypeVar | None: def snapshot_base( self, - update: bool | None = True, + update: bool | SnapshotUpdate | None = True, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -712,17 +713,26 @@ def snapshot_base( parameter. Args: - update: If True, update the state by calling ``parameter.get()`` - unless ``snapshot_get`` of the parameter is ``False``. - If ``update`` is ``None``, use the current value from the - ``cache`` unless the cache is invalid. If ``False``, never call - ``parameter.get()``. + update: What to do about the value stored in the snapshot. + + * ``"All"`` (or legacy ``True``): update the state by calling + ``parameter.get()`` unless ``snapshot_get`` of the parameter + is ``False``. + * ``"Only_invalid"`` (or legacy ``None``): call + ``parameter.get()`` only if the parameter's cache is invalid, + i.e. use ``cache.get(get_if_invalid=True)``, otherwise use the + cached value. This never calls ``get()`` if ``snapshot_get`` + is ``False`` or the parameter is not gettable. + * ``"Never"`` (or legacy ``False``): never call + ``parameter.get()``, always use the latest cached value. params_to_skip_update: No effect but may be passed from superclass Returns: base snapshot """ + update = _normalize_snapshot_update(update) + if self.snapshot_exclude: warnings.warn( f"Parameter ({self.full_name}) is used in the snapshot while it " diff --git a/src/qcodes/parameters/sweep_values.py b/src/qcodes/parameters/sweep_values.py index 0a261a1f3e4a..bdce9273c152 100644 --- a/src/qcodes/parameters/sweep_values.py +++ b/src/qcodes/parameters/sweep_values.py @@ -15,6 +15,7 @@ from collections.abc import Iterator, Sequence from typing import Self + from qcodes.metadatable import SnapshotUpdate from qcodes.parameters import ParameterBase @@ -348,7 +349,7 @@ def reverse(self) -> None: def snapshot_base( self, - update: bool | None = False, + update: bool | SnapshotUpdate | None = False, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ diff --git a/src/qcodes/station.py b/src/qcodes/station.py index 1b604775f46d..8ce7871d1c26 100644 --- a/src/qcodes/station.py +++ b/src/qcodes/station.py @@ -56,6 +56,8 @@ from pathlib import Path from types import ModuleType + from qcodes.metadatable import SnapshotUpdate + log = logging.getLogger(__name__) PARAMETER_ATTRIBUTES = [ @@ -185,7 +187,7 @@ def __init__( def snapshot_base( self, - update: bool | None = True, + update: bool | SnapshotUpdate | None = True, params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -198,12 +200,12 @@ def snapshot_base( from the station during the execution of this function. Args: - update: If ``True``, update the state by querying the - all the children: f.ex. instruments, parameters, - components, etc. If None only update if the state - is known to be invalid. - If ``False``, just use the latest - values in memory and never update the state. + update: What to do about the values stored in the snapshot of the + children (f.ex. instruments, parameters, components, etc.). + ``"All"`` (or legacy ``True``) updates every value, + ``"Only_invalid"`` (or legacy ``None``) only updates values + whose cache is invalid, and ``"Never"`` (or legacy ``False``) + never updates and uses the latest values in memory. params_to_skip_update: Not used. Returns: diff --git a/tests/dataset/test_snapshot.py b/tests/dataset/test_snapshot.py index 0efee53da7a9..b723033574e0 100644 --- a/tests/dataset/test_snapshot.py +++ b/tests/dataset/test_snapshot.py @@ -5,7 +5,7 @@ from qcodes.dataset.measurements import Measurement from qcodes.instrument_drivers.mock_instruments import DummyInstrument -from qcodes.parameters import ManualParameter +from qcodes.parameters import ManualParameter, Parameter from qcodes.station import Station @@ -116,3 +116,58 @@ def test_snapshot_creation_for_types_not_supported_by_builtin_json(experiment) - assert False is snapshot["station"]["parameters"]["p_np_bool"]["value"] assert False is snapshot["station"]["parameters"]["p_np_bool"]["raw_value"] + + +def test_station_snapshot_in_measurement_refreshes_only_invalid_caches( + experiment, +) -> None: + """ + The station snapshot taken by a ``Measurement`` uses ``update="Only_invalid"`` + so that parameters with an invalid cache are refreshed via a single ``get``, + while parameters with a valid cache are not gotten. + """ + invalid_calls = {"n": 0} + valid_calls = {"n": 0} + + def invalid_getter() -> int: + invalid_calls["n"] += 1 + return 42 + + def valid_getter() -> int: + valid_calls["n"] += 1 + return 99 + + p_invalid = Parameter("p_invalid", get_cmd=invalid_getter, set_cmd=None) + p_valid = Parameter("p_valid", get_cmd=valid_getter, set_cmd=None) + + # add components without updating their snapshot (which would call ``get``) + station = Station() + station.add_component(p_invalid, update_snapshot=False) + station.add_component(p_valid, update_snapshot=False) + + # make ``p_valid``'s cache valid without triggering a ``get`` + p_valid.set(7) + + assert not p_invalid.cache.valid + assert p_valid.cache.valid + assert invalid_calls["n"] == 0 + assert valid_calls["n"] == 0 + + measurement = Measurement(experiment, station) + # we need at least 1 parameter to be able to run the measurement + measurement.register_custom_parameter("dummy") + + with measurement.run() as data_saver: + pass + + snapshot = data_saver.dataset.snapshot + assert snapshot is not None + params = snapshot["station"]["parameters"] + + # invalid cache -> refreshed via a single get + assert invalid_calls["n"] == 1 + assert params["p_invalid"]["value"] == 42 + + # valid cache -> not gotten, cached value used + assert valid_calls["n"] == 0 + assert params["p_valid"]["value"] == 7 diff --git a/tests/parameter/test_snapshot.py b/tests/parameter/test_snapshot.py index 8f48d9ea8f77..ecf3c7b2f96b 100644 --- a/tests/parameter/test_snapshot.py +++ b/tests/parameter/test_snapshot.py @@ -3,8 +3,10 @@ from datetime import datetime, timedelta from typing import TYPE_CHECKING, Any, Literal, TypeVar +import pytest from typing_extensions import ParamSpec +from qcodes.metadatable.metadatable_base import _normalize_snapshot_update from qcodes.parameters import Parameter from .conftest import NOT_PASSED @@ -363,3 +365,111 @@ def test_snapshot_value() -> None: assert "value" not in snap assert "raw_value" not in snap assert "ts" in snap + + +def test_normalize_snapshot_update_maps_strings_to_legacy_values() -> None: + assert _normalize_snapshot_update("All") is True + assert _normalize_snapshot_update("Only_invalid") is None + assert _normalize_snapshot_update("Never") is False + # legacy values are passed through unchanged + assert _normalize_snapshot_update(True) is True + assert _normalize_snapshot_update(False) is False + assert _normalize_snapshot_update(None) is None + + +def test_normalize_snapshot_update_rejects_unknown_string() -> None: + with pytest.raises(ValueError, match="Invalid value for snapshot"): + _normalize_snapshot_update("bogus") # type: ignore[arg-type] + + +def test_snapshot_update_all_always_calls_get() -> None: + p = create_parameter( + snapshot_get=True, + snapshot_value=True, + get_cmd=lambda: 69, + cache_is_valid=True, + ) + s = p.snapshot(update="All") + assert s["value"] == 69 + assert p.get.call_count() == 1 # type: ignore[attr-defined] + + +def test_snapshot_update_never_never_calls_get() -> None: + p = create_parameter( + snapshot_get=True, + snapshot_value=True, + get_cmd=lambda: 69, + cache_is_valid=False, + ) + s = p.snapshot(update="Never") + assert s["value"] is None + assert p.get.call_count() == 0 # type: ignore[attr-defined] + + +def test_snapshot_update_only_invalid_calls_get_when_cache_invalid() -> None: + p = create_parameter( + snapshot_get=True, + snapshot_value=True, + get_cmd=lambda: 69, + cache_is_valid=False, + ) + s = p.snapshot(update="Only_invalid") + assert s["value"] == 69 + assert p.get.call_count() == 1 # type: ignore[attr-defined] + + +def test_snapshot_update_only_invalid_skips_get_when_cache_valid() -> None: + p = create_parameter( + snapshot_get=True, + snapshot_value=True, + get_cmd=lambda: 69, + cache_is_valid=True, + ) + s = p.snapshot(update="Only_invalid") + # the cached (set) value is used, ``get`` is not called + assert s["value"] == 42 + assert p.get.call_count() == 0 # type: ignore[attr-defined] + + +@pytest.mark.parametrize( + ("legacy", "string"), + ((True, "All"), (None, "Only_invalid"), (False, "Never")), +) +def test_snapshot_update_string_matches_legacy_value( + legacy: bool | None, + string: Literal["All", "Only_invalid", "Never"], + cache_is_valid: bool, +) -> None: + p_legacy = create_parameter( + snapshot_get=True, + snapshot_value=True, + get_cmd=lambda: 69, + cache_is_valid=cache_is_valid, + ) + p_string = create_parameter( + snapshot_get=True, + snapshot_value=True, + get_cmd=lambda: 69, + cache_is_valid=cache_is_valid, + ) + + s_legacy = p_legacy.snapshot(update=legacy) + s_string = p_string.snapshot(update=string) + + assert s_legacy["value"] == s_string["value"] + assert s_legacy["raw_value"] == s_string["raw_value"] + assert ( + p_legacy.get.call_count() # type: ignore[attr-defined] + == p_string.get.call_count() # type: ignore[attr-defined] + ) + + +def test_snapshot_rejects_unknown_update_value() -> None: + p = create_parameter( + snapshot_get=True, + snapshot_value=True, + get_cmd=lambda: 69, + cache_is_valid=True, + ) + with pytest.raises(ValueError, match="Invalid value for snapshot"): + p.snapshot(update="bogus") # type: ignore[arg-type] From 062f68a251955e50697ec6e4050f19302667b5a6 Mon Sep 17 00:00:00 2001 From: Mikhail Astafev Date: Thu, 30 Jul 2026 14:42:54 +0200 Subject: [PATCH 2/5] Add newsfragment for Only_invalid snapshot update mode Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa3f57b5-ddc1-45ce-b920-3f8efed6851c --- docs/changes/newsfragments/8354.new | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 docs/changes/newsfragments/8354.new diff --git a/docs/changes/newsfragments/8354.new b/docs/changes/newsfragments/8354.new new file mode 100644 index 000000000000..0dbde08b6d7e --- /dev/null +++ b/docs/changes/newsfragments/8354.new @@ -0,0 +1,12 @@ +Added a new set of values for the ``update`` argument of ``snapshot`` and +``snapshot_base`` (available on every :class:`.Metadatable`, including +instruments, channels and parameters): ``"All"``, ``"Only_invalid"`` and +``"Never"``. The legacy ``True`` / ``None`` / ``False`` values remain supported +as aliases for ``"All"`` / ``"Only_invalid"`` / ``"Never"`` respectively, so the +change is backwards compatible. With ``"Only_invalid"`` only parameters whose +cache is invalid are refreshed via a ``get`` (i.e. +``cache.get(get_if_invalid=True)``), which makes snapshotting more performant +when only some parameter caches are stale. The station snapshot taken by +:class:`.Measurement` now uses ``"Only_invalid"`` (previously the equivalent of +``"Never"``), so parameters with an invalid cache get refreshed while the others +are not queried. From 8fb304b1b6a6ae2fdc56053fc9e9b555caaac1ae Mon Sep 17 00:00:00 2001 From: Mikhail Astafev Date: Thu, 30 Jul 2026 16:04:38 +0200 Subject: [PATCH 3/5] Normalize snapshot update to SnapshotUpdate; default Only_invalid; deprecate legacy Address review feedback: - normalize_snapshot_update now returns the canonical SnapshotUpdate literal (All/Only_invalid/Never); all internal code branches on those values. - Export normalize_snapshot_update and SnapshotUpdate from qcodes.metadatable. - Default of update for snapshot/snapshot_base/print_readable_snapshot is now 'Only_invalid' across the Metadatable hierarchy. - Deprecate the legacy bool/None update values via typing_extensions.deprecated overloads on Metadatable.snapshot (static only, no runtime warning); internal code and docs use only the string values (except the Metadatable note). - Measurement registered-parameter snapshots pinned to 'Never' to preserve prior behavior after the default change. - Update docstrings, the 'Working with snapshots' notebook and driver examples; add newsfragments (.new and .breaking). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa3f57b5-ddc1-45ce-b920-3f8efed6851c --- docs/changes/newsfragments/8354.breaking | 19 ++++ docs/changes/newsfragments/8354.new | 25 +++-- .../DataSet/Working with snapshots.ipynb | 30 ++++- ...eS example with CopperMountain_M5065.ipynb | 2 +- .../Qcodes example with DynaCool PPMS.ipynb | 2 +- .../Qcodes example with HP8753D.ipynb | 2 +- .../Qcodes example with Keysight 344xxA.ipynb | 2 +- ...odes example with Oxford Mercury iPS.ipynb | 2 +- ...Schwarz RTO 1000 series Oscilloscope.ipynb | 2 +- ...s example with Rohde Schwarz SGS100A.ipynb | 2 +- ...codes example with Rohde Schwarz ZNB.ipynb | 2 +- ...des example with Tektronix AWG70002A.ipynb | 2 +- src/qcodes/dataset/measurements.py | 4 +- src/qcodes/instrument/channel.py | 21 ++-- src/qcodes/instrument/instrument_base.py | 40 ++++--- src/qcodes/instrument/ip.py | 12 +- src/qcodes/instrument/visa.py | 14 +-- .../instrument_drivers/AimTTi/_AimTTi_PL_P.py | 2 +- .../Keithley/_Keithley_2600.py | 6 +- .../Keysight/keysightb1500/KeysightB1517A.py | 2 +- .../private/Keysight_344xxA_submodules.py | 2 +- .../instrument_drivers/QDev/QDac_channels.py | 38 +++---- .../QuantumDesign/DynaCoolPPMS/DynaCool.py | 2 +- .../american_magnetics/AMI430_visa.py | 8 +- .../mock_instruments/__init__.py | 2 +- .../stanford_research/SR86x.py | 2 +- src/qcodes/metadatable/__init__.py | 14 ++- src/qcodes/metadatable/metadatable_base.py | 104 ++++++++++++------ src/qcodes/parameters/combined_parameter.py | 5 +- src/qcodes/parameters/delegate_parameter.py | 5 +- src/qcodes/parameters/parameter_base.py | 38 ++++--- src/qcodes/parameters/sweep_values.py | 8 +- src/qcodes/station.py | 16 ++- .../b1500_driver_tests/test_b1500.py | 2 +- tests/drivers/test_ami430_visa.py | 4 +- tests/parameter/conftest.py | 7 +- tests/parameter/test_array_parameter.py | 10 +- tests/parameter/test_delegate_parameter.py | 2 +- tests/parameter/test_multi_parameter.py | 10 +- tests/parameter/test_parameter_basics.py | 4 +- tests/parameter/test_parameter_cache.py | 6 +- tests/parameter/test_snapshot.py | 67 ++++++----- tests/test_instrument.py | 2 +- tests/test_snapshot.py | 4 +- 44 files changed, 345 insertions(+), 210 deletions(-) create mode 100644 docs/changes/newsfragments/8354.breaking diff --git a/docs/changes/newsfragments/8354.breaking b/docs/changes/newsfragments/8354.breaking new file mode 100644 index 000000000000..e59ce9af5730 --- /dev/null +++ b/docs/changes/newsfragments/8354.breaking @@ -0,0 +1,19 @@ +The default value of the ``update`` argument of ``snapshot``, ``snapshot_base`` +and ``print_readable_snapshot`` (on every :class:`.Metadatable`, including +instruments, channels and parameters) is now ``"Only_invalid"``. Previously +calling e.g. ``instrument.snapshot()`` or ``parameter.snapshot()`` without an +argument effectively defaulted to the equivalent of ``"Never"`` and never +refreshed anything. As a result, ``snapshot()`` (and ``print_readable_snapshot()``) +will now call ``get()`` on parameters whose cache is invalid, which may query the +underlying instrument. Pass ``update="Never"`` explicitly to restore the previous +"do not update" behavior, or ``update="All"`` to force a full update. + +Relatedly, the station snapshot that :class:`.Measurement` stores before a +measurement starts is now taken with ``update="Only_invalid"`` (previously the +equivalent of ``"Never"``), so parameters with an invalid cache are refreshed via +a single ``get`` while parameters with a valid cache keep their cached value. + +The legacy ``True`` / ``None`` / ``False`` values of the ``update`` argument are +now deprecated aliases for ``"All"`` / ``"Only_invalid"`` / ``"Never"``. They keep +working (no runtime warning is raised), but type checkers will flag their use; +prefer the string values instead. diff --git a/docs/changes/newsfragments/8354.new b/docs/changes/newsfragments/8354.new index 0dbde08b6d7e..768f35e81b00 100644 --- a/docs/changes/newsfragments/8354.new +++ b/docs/changes/newsfragments/8354.new @@ -1,12 +1,13 @@ -Added a new set of values for the ``update`` argument of ``snapshot`` and -``snapshot_base`` (available on every :class:`.Metadatable`, including -instruments, channels and parameters): ``"All"``, ``"Only_invalid"`` and -``"Never"``. The legacy ``True`` / ``None`` / ``False`` values remain supported -as aliases for ``"All"`` / ``"Only_invalid"`` / ``"Never"`` respectively, so the -change is backwards compatible. With ``"Only_invalid"`` only parameters whose -cache is invalid are refreshed via a ``get`` (i.e. -``cache.get(get_if_invalid=True)``), which makes snapshotting more performant -when only some parameter caches are stale. The station snapshot taken by -:class:`.Measurement` now uses ``"Only_invalid"`` (previously the equivalent of -``"Never"``), so parameters with an invalid cache get refreshed while the others -are not queried. +The ``update`` argument of ``snapshot`` and ``snapshot_base`` (available on every +:class:`.Metadatable`, including instruments, channels and parameters) now accepts +the explicit string values ``"All"``, ``"Only_invalid"`` and ``"Never"``: + +* ``"All"`` forces an update of every value (calls ``get()`` on each parameter). +* ``"Only_invalid"`` only refreshes parameters whose cache is invalid, via a + single ``get`` (``cache.get(get_if_invalid=True)``), and uses the cached value + for everything else. +* ``"Never"`` never updates and uses the latest values already in memory. + +The new public helper :func:`qcodes.metadatable.normalize_snapshot_update` and the +:data:`qcodes.metadatable.SnapshotUpdate` type are exported for drivers that +override ``snapshot_base`` and need to interpret the ``update`` argument. diff --git a/docs/examples/DataSet/Working with snapshots.ipynb b/docs/examples/DataSet/Working with snapshots.ipynb index d90eb8024926..01ee99d42e31 100644 --- a/docs/examples/DataSet/Working with snapshots.ipynb +++ b/docs/examples/DataSet/Working with snapshots.ipynb @@ -460,6 +460,34 @@ "pprint(snapshot_of_station)" ] }, + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "## Controlling what gets updated: the `update` argument\n", + "\n", + "`snapshot()` (and `snapshot_base()`) accept an `update` argument that controls whether parameter values are refreshed from the instruments while the snapshot is taken. It takes one of three string values:\n", + "\n", + "- `\"All\"`: force an update of every value by calling `get()` on each parameter (unless the parameter has `snapshot_get=False`).\n", + "- `\"Only_invalid\"` (the default): only call `get()` for parameters whose cache is invalid, and use the latest cached value for everything else. This keeps snapshotting fast while making sure stale values are refreshed.\n", + "- `\"Never\"`: never call `get()`, always use the latest values already in memory.\n", + "\n", + "The legacy boolean/`None` values (`True`/`None`/`False`) are deprecated aliases for `\"All\"`/`\"Only_invalid\"`/`\"Never\"` respectively and should no longer be used.\n", + "\n", + "For example, to force a full update:" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "snapshot_of_p = p.snapshot(update=\"All\")\n", + "\n", + "pprint(snapshot_of_p)" + ] + }, { "cell_type": "markdown", "metadata": {}, @@ -468,7 +496,7 @@ "\n", "With the power of the station object, it is now possible to conveniently associate the snapshot information with the measured data.\n", "\n", - "In order to do so, a station needs to be created, and then that station needs to be provided to the `Measurement` object. If no station is explicitly provided, the `Measurement` object will use the default station, `Station.default` (refer to `Measurement` and `Station` objects docstrings for more information). At the moment the new measurement run is started, a snapshot of the whole station will be taken, and added next to the measured data.\n", + "In order to do so, a station needs to be created, and then that station needs to be provided to the `Measurement` object. If no station is explicitly provided, the `Measurement` object will use the default station, `Station.default` (refer to `Measurement` and `Station` objects docstrings for more information). At the moment the new measurement run is started, a snapshot of the whole station will be taken (with `update=\"Only_invalid\"`, so that parameters with an invalid cache are refreshed while the rest use their cached values), and added next to the measured data.\n", "\n", "The measured dataset also automatically snapshots the parameters involved in the measurement and stores it alongside the station snapshot.\n", "\n", diff --git a/docs/examples/driver_examples/QCoDeS example with CopperMountain_M5065.ipynb b/docs/examples/driver_examples/QCoDeS example with CopperMountain_M5065.ipynb index b07f2cd803cd..d17339df81f3 100644 --- a/docs/examples/driver_examples/QCoDeS example with CopperMountain_M5065.ipynb +++ b/docs/examples/driver_examples/QCoDeS example with CopperMountain_M5065.ipynb @@ -116,7 +116,7 @@ ], "source": [ "# Let's look at all parameters\n", - "vna.print_readable_snapshot(update=True)" + "vna.print_readable_snapshot(update=\"All\")" ] }, { diff --git a/docs/examples/driver_examples/Qcodes example with DynaCool PPMS.ipynb b/docs/examples/driver_examples/Qcodes example with DynaCool PPMS.ipynb index 00080a6f38df..ac7d27b86564 100644 --- a/docs/examples/driver_examples/Qcodes example with DynaCool PPMS.ipynb +++ b/docs/examples/driver_examples/Qcodes example with DynaCool PPMS.ipynb @@ -94,7 +94,7 @@ } ], "source": [ - "dynacool.print_readable_snapshot(update=True)" + "dynacool.print_readable_snapshot(update=\"All\")" ] }, { diff --git a/docs/examples/driver_examples/Qcodes example with HP8753D.ipynb b/docs/examples/driver_examples/Qcodes example with HP8753D.ipynb index cfc729c12d82..62875c10d883 100644 --- a/docs/examples/driver_examples/Qcodes example with HP8753D.ipynb +++ b/docs/examples/driver_examples/Qcodes example with HP8753D.ipynb @@ -119,7 +119,7 @@ } ], "source": [ - "vna.print_readable_snapshot(update=True)" + "vna.print_readable_snapshot(update=\"All\")" ] }, { diff --git a/docs/examples/driver_examples/Qcodes example with Keysight 344xxA.ipynb b/docs/examples/driver_examples/Qcodes example with Keysight 344xxA.ipynb index dff35b838192..422a38b43b2e 100644 --- a/docs/examples/driver_examples/Qcodes example with Keysight 344xxA.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Keysight 344xxA.ipynb @@ -168,7 +168,7 @@ } ], "source": [ - "dmm.print_readable_snapshot(update=True)" + "dmm.print_readable_snapshot(update=\"All\")" ] }, { diff --git a/docs/examples/driver_examples/Qcodes example with Oxford Mercury iPS.ipynb b/docs/examples/driver_examples/Qcodes example with Oxford Mercury iPS.ipynb index c6a39f07c52d..177941b5fd86 100644 --- a/docs/examples/driver_examples/Qcodes example with Oxford Mercury iPS.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Oxford Mercury iPS.ipynb @@ -185,7 +185,7 @@ } ], "source": [ - "mips.print_readable_snapshot(update=True)" + "mips.print_readable_snapshot(update=\"All\")" ] }, { diff --git a/docs/examples/driver_examples/Qcodes example with Rohde Schwarz RTO 1000 series Oscilloscope.ipynb b/docs/examples/driver_examples/Qcodes example with Rohde Schwarz RTO 1000 series Oscilloscope.ipynb index 407b12e53ea5..80be469bc66f 100644 --- a/docs/examples/driver_examples/Qcodes example with Rohde Schwarz RTO 1000 series Oscilloscope.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Rohde Schwarz RTO 1000 series Oscilloscope.ipynb @@ -97,7 +97,7 @@ "outputs": [], "source": [ "# To get an overview of all instrument settings, print_readable_sanpshot is great\n", - "rto.print_readable_snapshot(update=True)" + "rto.print_readable_snapshot(update=\"All\")" ] }, { diff --git a/docs/examples/driver_examples/Qcodes example with Rohde Schwarz SGS100A.ipynb b/docs/examples/driver_examples/Qcodes example with Rohde Schwarz SGS100A.ipynb index afa69e031efd..e5a36bc820ef 100644 --- a/docs/examples/driver_examples/Qcodes example with Rohde Schwarz SGS100A.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Rohde Schwarz SGS100A.ipynb @@ -68,7 +68,7 @@ } ], "source": [ - "sgsa.print_readable_snapshot(update=True)" + "sgsa.print_readable_snapshot(update=\"All\")" ] }, { diff --git a/docs/examples/driver_examples/Qcodes example with Rohde Schwarz ZNB.ipynb b/docs/examples/driver_examples/Qcodes example with Rohde Schwarz ZNB.ipynb index e09dec239ae3..0d22e8eb049c 100644 --- a/docs/examples/driver_examples/Qcodes example with Rohde Schwarz ZNB.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Rohde Schwarz ZNB.ipynb @@ -1622,7 +1622,7 @@ } ], "source": [ - "vna.channels.S11.print_readable_snapshot(update=True)" + "vna.channels.S11.print_readable_snapshot(update=\"All\")" ] }, { diff --git a/docs/examples/driver_examples/Qcodes example with Tektronix AWG70002A.ipynb b/docs/examples/driver_examples/Qcodes example with Tektronix AWG70002A.ipynb index a8f5857a1486..152ba82ad7aa 100644 --- a/docs/examples/driver_examples/Qcodes example with Tektronix AWG70002A.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Tektronix AWG70002A.ipynb @@ -105,7 +105,7 @@ "source": [ "# Let's have a look at the available parameters\n", "\n", - "awg.print_readable_snapshot(update=True)" + "awg.print_readable_snapshot(update=\"All\")" ] }, { diff --git a/src/qcodes/dataset/measurements.py b/src/qcodes/dataset/measurements.py index 83603b9037ad..35f4418e7be4 100644 --- a/src/qcodes/dataset/measurements.py +++ b/src/qcodes/dataset/measurements.py @@ -668,12 +668,12 @@ def __enter__(self) -> DataSaver: snapshot = {} if self._registered_parameters is not None: parameter_snapshot = { - param.short_name: param.snapshot() + param.short_name: param.snapshot(update="Never") for param in self._registered_parameters } parameter_snapshot.update( { - param.register_name: param.snapshot() + param.register_name: param.snapshot(update="Never") for param in self._registered_parameters } ) diff --git a/src/qcodes/instrument/channel.py b/src/qcodes/instrument/channel.py index a4e098eea3e2..7a8829a3050c 100644 --- a/src/qcodes/instrument/channel.py +++ b/src/qcodes/instrument/channel.py @@ -8,7 +8,7 @@ from typing_extensions import TypeVar -from qcodes.metadatable import MetadatableWithName +from qcodes.metadatable import MetadatableWithName, normalize_snapshot_update from qcodes.parameters import ( ArrayParameter, MultiChannelInstrumentParameter, @@ -382,7 +382,7 @@ def get_validator(self) -> ChannelTupleValidator: def snapshot_base( self, - update: bool | SnapshotUpdate | None = True, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -391,13 +391,13 @@ def snapshot_base( :class:`.NumpyJSONEncoder` supports). Args: - update: If True, update the state by querying the - instrument. If None only update if the state is known to be - invalid. If False, just use the latest values in memory - and never update. + update: If ``"All"``, update the state by querying the instrument. + If ``"Only_invalid"`` (the default) only update values whose + cache is invalid. If ``"Never"``, just use the latest values in + memory and never update. params_to_skip_update: List of parameter names that will be skipped - in update even if update is True. This is useful if you have - parameters that are slow to update but can be updated in a + in update even if update is ``"All"``. This is useful if you + have parameters that are slow to update but can be updated in a different way (as in the qdac). If you want to skip the update of certain parameters in all snapshots, use the ``snapshot_get`` attribute of those parameters instead. @@ -406,6 +406,7 @@ def snapshot_base( dict: base snapshot """ + update = normalize_snapshot_update(update) if self._snapshotable: snap = { "channels": { @@ -613,7 +614,9 @@ def __dir__(self) -> list[Any]: return sorted(set(names)) def print_readable_snapshot( - self, update: bool = False, max_chars: int = 80 + self, + update: bool | SnapshotUpdate | None = "Only_invalid", + max_chars: int = 80, ) -> None: if self._snapshotable: for channel in self._channels: diff --git a/src/qcodes/instrument/instrument_base.py b/src/qcodes/instrument/instrument_base.py index d316814a84b5..e2316d9c2bc0 100644 --- a/src/qcodes/instrument/instrument_base.py +++ b/src/qcodes/instrument/instrument_base.py @@ -11,7 +11,11 @@ from typing_extensions import TypedDict, TypeVar, deprecated from qcodes.logger import get_instrument_logger -from qcodes.metadatable import Metadatable, MetadatableWithName +from qcodes.metadatable import ( + Metadatable, + MetadatableWithName, + normalize_snapshot_update, +) from qcodes.parameters import Function, Parameter, ParameterBase from qcodes.utils import DelegateAttributes, full_class @@ -408,7 +412,7 @@ def _get_component_by_name( def snapshot_base( self, - update: bool | SnapshotUpdate | None = False, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -418,14 +422,13 @@ def snapshot_base( supports). Args: - update: If ``"All"`` (or legacy ``True``), update the state by - querying the instrument. If ``"Only_invalid"`` (or legacy - ``None``) update the state only for values known to be invalid. - If ``"Never"`` (or legacy ``False``), just use the latest - values in memory and never update state. + update: If ``"All"``, update the state by querying the instrument. + If ``"Only_invalid"`` (the default) update the state only for + values whose cache is invalid. If ``"Never"``, just use the + latest values in memory and never update state. params_to_skip_update: List of parameter names that will be skipped - in update even if update is True. This is useful if you have - parameters that are slow to update but can be updated in a + in update even if update is ``"All"``. This is useful if you + have parameters that are slow to update but can be updated in a different way (as in the qdac). If you want to skip the update of certain parameters in all snapshots, use the ``snapshot_get`` attribute of those parameters instead. @@ -434,6 +437,7 @@ def snapshot_base( dict: base snapshot """ + update = normalize_snapshot_update(update) if params_to_skip_update is None: params_to_skip_update = [] @@ -455,7 +459,7 @@ def snapshot_base( if param.snapshot_exclude: continue if params_to_skip_update and name in params_to_skip_update: - update_par: bool | SnapshotUpdate | None = False + update_par: SnapshotUpdate = "Never" else: update_par = update try: @@ -465,7 +469,7 @@ def snapshot_base( # at lower level with more info for file based loggers self.log.warning("Snapshot: Could not update parameter: %s", name) self.log.info("Details for Snapshot:", exc_info=True) - snap["parameters"][name] = param.snapshot(update=False) + snap["parameters"][name] = param.snapshot(update="Never") for attr in set(self._meta_attrs): val = getattr(self, attr, None) @@ -478,7 +482,9 @@ def snapshot_base( return snap def print_readable_snapshot( - self, update: bool = False, max_chars: int = 80 + self, + update: bool | SnapshotUpdate | None = "Only_invalid", + max_chars: int = 80, ) -> None: """ Prints a readable version of the snapshot. @@ -488,16 +494,18 @@ def print_readable_snapshot( status of an instrument. Args: - update: If ``True``, update the state by querying the - instrument. If ``False``, just use the latest values in memory. - This argument gets passed to the snapshot function. + update: What to do about the values in the snapshot. ``"All"`` + updates every value by querying the instrument, + ``"Only_invalid"`` (the default) only updates values whose + cache is invalid, and ``"Never"`` just uses the latest values + in memory. This argument gets passed to the snapshot function. max_chars: the maximum number of characters per line. The readable snapshot will be cropped if this value is exceeded. Defaults to 80 to be consistent with default terminal width. """ floating_types = (float, np.integer, np.floating) - snapshot = self.snapshot(update=update) + snapshot = self.snapshot(update=normalize_snapshot_update(update)) par_lengths = [len(p) for p in snapshot["parameters"]] # handle the case of no parameters diff --git a/src/qcodes/instrument/ip.py b/src/qcodes/instrument/ip.py index 6221668fc46f..286b0a6ca266 100644 --- a/src/qcodes/instrument/ip.py +++ b/src/qcodes/instrument/ip.py @@ -215,7 +215,7 @@ def ask_raw(self, cmd: str) -> str: def snapshot_base( self, - update: bool | SnapshotUpdate | None = False, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -225,12 +225,12 @@ def snapshot_base( supports). Args: - update: If True, update the state by querying the - instrument. If None only update if the state is known to be - invalid. If False, just use the latest values in memory and - never update. + update: If ``"All"``, update the state by querying the instrument. + If ``"Only_invalid"`` (the default) only update values whose + cache is invalid. If ``"Never"``, just use the latest values in + memory and never update. params_to_skip_update: List of parameter names that will be - skipped in update even if update is True. This is useful + skipped in update even if update is ``"All"``. This is useful if you have parameters that are slow to update but can be updated in a different way (as in the qdac). If you want to skip the update of certain parameters in all diff --git a/src/qcodes/instrument/visa.py b/src/qcodes/instrument/visa.py index 13c5f7e238b2..f952d1c6babe 100644 --- a/src/qcodes/instrument/visa.py +++ b/src/qcodes/instrument/visa.py @@ -451,7 +451,7 @@ def ask_raw(self, cmd: str) -> str: def snapshot_base( self, - update: bool | SnapshotUpdate | None = True, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -460,13 +460,13 @@ def snapshot_base( supports). Args: - update: If True, update the state by querying the - instrument. If None only update if the state is known to be - invalid. If False, just use the latest values in memory and - never update. + update: If ``"All"``, update the state by querying the instrument. + If ``"Only_invalid"`` (the default) only update values whose + cache is invalid. If ``"Never"``, just use the latest values in + memory and never update. params_to_skip_update: List of parameter names that will be skipped - in update even if update is True. This is useful if you have - parameters that are slow to update but can be updated in a + in update even if update is ``"All"``. This is useful if you + have parameters that are slow to update but can be updated in a different way (as in the qdac). If you want to skip the update of certain parameters in all snapshots, use the ``snapshot_get`` attribute of those parameters instead. diff --git a/src/qcodes/instrument_drivers/AimTTi/_AimTTi_PL_P.py b/src/qcodes/instrument_drivers/AimTTi/_AimTTi_PL_P.py index 748053bdb155..5c5d66be29c8 100644 --- a/src/qcodes/instrument_drivers/AimTTi/_AimTTi_PL_P.py +++ b/src/qcodes/instrument_drivers/AimTTi/_AimTTi_PL_P.py @@ -215,7 +215,7 @@ def load_setup(self, slot: int) -> None: channel_id = self.channel self.write(f"RCL{channel_id} {slot}") # Update snapshot after load. - _ = self.snapshot(update=True) + _ = self.snapshot(update="All") def set_damping(self, val: int) -> None: """ diff --git a/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py b/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py index ff5d32c829ac..ff0411dc55aa 100644 --- a/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py +++ b/src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py @@ -568,7 +568,7 @@ def _parse_response(data: str) -> tuple[float, Keithley2600MeasurementStatus]: def snapshot_base( self, - update: bool | SnapshotUpdate | None = True, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: snapshot = super().snapshot_base( @@ -1021,7 +1021,7 @@ def reset(self) -> None: self.write(f"{self.channel}.reset()") # remember to update all the metadata log.debug(f"Reset channel {self.channel}. Updating settings...") - self.snapshot(update=True) + self.snapshot(update="All") def setup_fastsweep( self, @@ -1403,7 +1403,7 @@ def reset(self) -> None: self.write("reset()") # remember to update all the metadata log.debug("Reset instrument. Re-querying settings...") - self.snapshot(update=True) + self.snapshot(update="All") def ask(self, cmd: str) -> str: """ diff --git a/src/qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1517A.py b/src/qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1517A.py index 0227622dfe8f..ffd26be092aa 100644 --- a/src/qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1517A.py +++ b/src/qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1517A.py @@ -714,7 +714,7 @@ def measurement_status(self) -> MeasurementStatus | None: def snapshot_base( self, - update: "bool | SnapshotUpdate | None" = True, + update: "bool | SnapshotUpdate | None" = "Only_invalid", params_to_skip_update: "Sequence[str] | None" = None, ) -> dict[Any, Any]: snapshot = super().snapshot_base( diff --git a/src/qcodes/instrument_drivers/Keysight/private/Keysight_344xxA_submodules.py b/src/qcodes/instrument_drivers/Keysight/private/Keysight_344xxA_submodules.py index e35c0930b652..ac681756cd48 100644 --- a/src/qcodes/instrument_drivers/Keysight/private/Keysight_344xxA_submodules.py +++ b/src/qcodes/instrument_drivers/Keysight/private/Keysight_344xxA_submodules.py @@ -1122,7 +1122,7 @@ def reset(self) -> None: self.write("*RST") # before we can update the snapshot, the reset must complete self.ask("*OPC?") - self.snapshot(update=True) + self.snapshot(update="All") def abort_measurement(self) -> None: """ diff --git a/src/qcodes/instrument_drivers/QDev/QDac_channels.py b/src/qcodes/instrument_drivers/QDev/QDac_channels.py index 42aeafebb00b..b3720e9a89ba 100644 --- a/src/qcodes/instrument_drivers/QDev/QDac_channels.py +++ b/src/qcodes/instrument_drivers/QDev/QDac_channels.py @@ -17,7 +17,7 @@ VisaInstrument, VisaInstrumentKWArgs, ) -from qcodes.metadatable.metadatable_base import _normalize_snapshot_update +from qcodes.metadatable import normalize_snapshot_update from qcodes.parameters import MultiChannelInstrumentParameter, ParamRawDataType if TYPE_CHECKING: @@ -141,23 +141,23 @@ def __init__( def snapshot_base( self, - update: "bool | SnapshotUpdate | None" = False, + update: "bool | SnapshotUpdate | None" = "Only_invalid", params_to_skip_update: "Sequence[str] | None" = None, ) -> dict[Any, Any]: - update = _normalize_snapshot_update(update) - # setting update not None will override parent setting - # otherwise we use parent setting - # parent._update | update | do update - # True | True | True - # True | None | True - # True | False | False - # False | True | True - # False | None | False - # False | False | False + update = normalize_snapshot_update(update) + # setting update to "All"/"Only_invalid" (i.e. not "Never") will + # override parent setting; otherwise we use the parent setting + # parent._update | update | do update + # True | "All" | True + # True | "Only_invalid" | True + # True | "Never" | False + # False | "All" | True + # False | "Only_invalid" | False + # False | "Never" | False update_currents = ( - self.parent._update_currents and update is not False - ) or update is True - if update and not self.parent._get_status_performed: + self.parent._update_currents and update != "Never" + ) or update == "All" + if update != "Never" and not self.parent._get_status_performed: self.parent._update_cache(readcurrents=update_currents) # call get_status rather than getting the status individually for # each parameter. This is only done if _get_status_performed is False @@ -340,12 +340,12 @@ def __init__( def snapshot_base( self, - update: "bool | SnapshotUpdate | None" = False, + update: "bool | SnapshotUpdate | None" = "Only_invalid", params_to_skip_update: "Sequence[str] | None" = None, ) -> dict[Any, Any]: - update = _normalize_snapshot_update(update) - update_currents = self._update_currents and update is True - if update: + update = normalize_snapshot_update(update) + update_currents = self._update_currents and update == "All" + if update != "Never": self._update_cache(readcurrents=update_currents) self._get_status_performed = True # call get_status rather than getting the status individually for diff --git a/src/qcodes/instrument_drivers/QuantumDesign/DynaCoolPPMS/DynaCool.py b/src/qcodes/instrument_drivers/QuantumDesign/DynaCoolPPMS/DynaCool.py index ad0f97e4f641..d73b0906583c 100644 --- a/src/qcodes/instrument_drivers/QuantumDesign/DynaCoolPPMS/DynaCool.py +++ b/src/qcodes/instrument_drivers/QuantumDesign/DynaCoolPPMS/DynaCool.py @@ -261,7 +261,7 @@ def __init__( self._error_code = 0 # we must know all parameter values because of interlinked parameters - self.snapshot(update=True) + self.snapshot(update="All") # it is a safe default to set the target to the current value self.field_target(self.field_measured()) diff --git a/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py b/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py index 834de1a61a0a..c82dc0a2273e 100644 --- a/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py +++ b/src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py @@ -19,7 +19,7 @@ VisaInstrumentKWArgs, ) from qcodes.math_utils import FieldVector -from qcodes.metadatable.metadatable_base import _normalize_snapshot_update +from qcodes.metadatable import normalize_snapshot_update from qcodes.parameters import Parameter from qcodes.utils.types import NumberType from qcodes.validators import Anything, Bool, Enum, Ints, Numbers @@ -152,14 +152,14 @@ def _check_state(self) -> bool: def snapshot_base( self, - update: bool | SnapshotUpdate | None = False, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[str, Any]: - update = _normalize_snapshot_update(update) + update = normalize_snapshot_update(update) if params_to_skip_update is None: params_to_skip_update = [] - if update is True: + if update == "All": enabled = self.enabled.get() else: enabled = self.enabled.cache.get() diff --git a/src/qcodes/instrument_drivers/mock_instruments/__init__.py b/src/qcodes/instrument_drivers/mock_instruments/__init__.py index 048c05aa299c..9f0da5711d31 100644 --- a/src/qcodes/instrument_drivers/mock_instruments/__init__.py +++ b/src/qcodes/instrument_drivers/mock_instruments/__init__.py @@ -1069,7 +1069,7 @@ def _getter(self, name: str) -> ParamRawDataType: def snapshot_base( self, - update: bool | SnapshotUpdate | None = True, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: if params_to_skip_update is None: diff --git a/src/qcodes/instrument_drivers/stanford_research/SR86x.py b/src/qcodes/instrument_drivers/stanford_research/SR86x.py index 091f4f1e4875..cfea0dea5cdb 100644 --- a/src/qcodes/instrument_drivers/stanford_research/SR86x.py +++ b/src/qcodes/instrument_drivers/stanford_research/SR86x.py @@ -211,7 +211,7 @@ def __init__( def snapshot_base( self, - update: bool | SnapshotUpdate | None = False, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: if params_to_skip_update is None: diff --git a/src/qcodes/metadatable/__init__.py b/src/qcodes/metadatable/__init__.py index e6e4e4c42062..ce212a8d84a8 100644 --- a/src/qcodes/metadatable/__init__.py +++ b/src/qcodes/metadatable/__init__.py @@ -1,3 +1,13 @@ -from .metadatable_base import Metadatable, MetadatableWithName, SnapshotUpdate +from .metadatable_base import ( + Metadatable, + MetadatableWithName, + SnapshotUpdate, + normalize_snapshot_update, +) -__all__ = ["Metadatable", "MetadatableWithName", "SnapshotUpdate"] +__all__ = [ + "Metadatable", + "MetadatableWithName", + "SnapshotUpdate", + "normalize_snapshot_update", +] diff --git a/src/qcodes/metadatable/metadatable_base.py b/src/qcodes/metadatable/metadatable_base.py index ddf10607de62..a0ab34f1b9c9 100644 --- a/src/qcodes/metadatable/metadatable_base.py +++ b/src/qcodes/metadatable/metadatable_base.py @@ -1,5 +1,7 @@ from abc import abstractmethod -from typing import TYPE_CHECKING, Any, Literal, final +from typing import TYPE_CHECKING, Any, Literal, final, overload + +from typing_extensions import deprecated from qcodes.utils import deep_update @@ -8,7 +10,7 @@ SnapshotUpdate = Literal["All", "Only_invalid", "Never"] """ -Allowed string values for the ``update`` argument of ``snapshot`` and +Canonical string values for the ``update`` argument of ``snapshot`` and ``snapshot_base``: * ``"All"``: force an update of every value (equivalent to legacy ``True``). @@ -16,37 +18,51 @@ latest cached value otherwise (equivalent to legacy ``None``). * ``"Never"``: never update, always use the latest values in memory (equivalent to legacy ``False``). -""" -_SNAPSHOT_UPDATE_ALIASES: "dict[str, bool | None]" = { - "All": True, - "Only_invalid": None, - "Never": False, -} +Internally, the ``update`` argument is always normalized to one of these +values via :func:`normalize_snapshot_update`. The legacy ``bool``/``None`` +values are still accepted at the public interface for backwards compatibility. +""" -def _normalize_snapshot_update( +def normalize_snapshot_update( update: "bool | SnapshotUpdate | None", -) -> "bool | None": +) -> SnapshotUpdate: """ Normalize the ``update`` argument of ``snapshot``/``snapshot_base`` into - its legacy ``bool | None`` representation, where ``True`` means update all - values, ``None`` means update only values with an invalid cache, and - ``False`` means never update. + one of the canonical :data:`SnapshotUpdate` string values. + + The legacy values ``True``, ``None`` and ``False`` are mapped to + ``"All"``, ``"Only_invalid"`` and ``"Never"`` respectively, and the + canonical string values are returned unchanged. This is the single place + where the ``update`` argument is interpreted; all internal code should + work with the returned :data:`SnapshotUpdate` value rather than with the + legacy ``bool``/``None`` representation. + + Args: + update: The ``update`` argument as passed to ``snapshot``/ + ``snapshot_base``. + + Returns: + The equivalent canonical :data:`SnapshotUpdate` value. + + Raises: + ValueError: If ``update`` is a string that is not a valid + :data:`SnapshotUpdate` value. - The string values ``"All"``, ``"Only_invalid"`` and ``"Never"`` are mapped - to ``True``, ``None`` and ``False`` respectively. ``bool`` and ``None`` - values are returned unchanged for backwards compatibility. """ - if isinstance(update, str): - try: - return _SNAPSHOT_UPDATE_ALIASES[update] - except KeyError: - raise ValueError( - f"Invalid value for snapshot ``update``: {update!r}. Expected " - f"one of {list(_SNAPSHOT_UPDATE_ALIASES)}, or a bool, or None." - ) from None - return update + if update is True: + return "All" + if update is False: + return "Never" + if update is None: + return "Only_invalid" + if update in ("All", "Only_invalid", "Never"): + return update + raise ValueError( + f"Invalid value for snapshot ``update``: {update!r}. Expected one of " + f"'All', 'Only_invalid', 'Never', or a bool, or None." + ) # NB: At the moment, the Snapshot type is a bit weak, as the Any @@ -76,26 +92,48 @@ def load_metadata(self, metadata: "Mapping[str, Any]") -> None: """ deep_update(self.metadata, metadata) + @overload + def snapshot(self, update: "SnapshotUpdate" = ...) -> Snapshot: ... + + @overload + @deprecated( + "Passing a bool or None as the snapshot ``update`` argument is " + "deprecated; use one of the string values 'All', 'Only_invalid' or " + "'Never' instead." + ) + def snapshot(self, update: "bool | None" = ...) -> Snapshot: ... + @final - def snapshot(self, update: "bool | SnapshotUpdate | None" = False) -> Snapshot: + def snapshot( + self, update: "bool | SnapshotUpdate | None" = "Only_invalid" + ) -> Snapshot: """ Decorate a snapshot dictionary with metadata. DO NOT override this method if you want metadata in the snapshot instead, override :meth:`snapshot_base`. Args: - update: Passed to snapshot_base. Accepts the string values - ``"All"``, ``"Only_invalid"`` and ``"Never"`` as well as the - legacy values ``True`` (equivalent to ``"All"``), ``None`` - (equivalent to ``"Only_invalid"``) and ``False`` (equivalent - to ``"Never"``). + update: What to do about the values stored in the snapshot; passed + to :meth:`snapshot_base` after being normalized to a + :data:`SnapshotUpdate` value. + + * ``"All"``: force an update of every value. + * ``"Only_invalid"`` (the default): only update values whose + cache is invalid, using the latest cached value otherwise. + * ``"Never"``: never update, always use the latest values in + memory. + + The legacy ``True`` / ``None`` / ``False`` values are deprecated + aliases for ``"All"`` / ``"Only_invalid"`` / ``"Never"`` and are + still accepted for backwards compatibility (no warning is + raised). Returns: Base snapshot. """ - snap = self.snapshot_base(update=_normalize_snapshot_update(update)) + snap = self.snapshot_base(update=normalize_snapshot_update(update)) if len(self.metadata): snap["metadata"] = self.metadata @@ -104,7 +142,7 @@ def snapshot(self, update: "bool | SnapshotUpdate | None" = False) -> Snapshot: def snapshot_base( self, - update: "bool | SnapshotUpdate | None" = False, + update: "bool | SnapshotUpdate | None" = "Only_invalid", params_to_skip_update: "Sequence[str] | None" = None, ) -> Snapshot: """ diff --git a/src/qcodes/parameters/combined_parameter.py b/src/qcodes/parameters/combined_parameter.py index f38780fd773e..96d434736165 100644 --- a/src/qcodes/parameters/combined_parameter.py +++ b/src/qcodes/parameters/combined_parameter.py @@ -196,7 +196,7 @@ def __len__(self) -> int: def snapshot_base( self, - update: bool | SnapshotUpdate | None = False, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -205,8 +205,7 @@ def snapshot_base( :class:`.NumpyJSONEncoder` supports). Args: - update: ``"All"``, ``"Only_invalid"`` or ``"Never"`` (or the legacy - ``True`` / ``None`` / ``False``). + update: One of ``"All"``, ``"Only_invalid"`` or ``"Never"``. params_to_skip_update: Unused in this subclass. Returns: diff --git a/src/qcodes/parameters/delegate_parameter.py b/src/qcodes/parameters/delegate_parameter.py index 203a5916be50..cbca76e2a2f7 100644 --- a/src/qcodes/parameters/delegate_parameter.py +++ b/src/qcodes/parameters/delegate_parameter.py @@ -4,6 +4,8 @@ from typing_extensions import TypeVar +from qcodes.metadatable import normalize_snapshot_update + from .parameter import Parameter, ParameterKWArgs from .parameter_base import InstrumentTypeVar_co, ParameterDataTypeVar @@ -325,9 +327,10 @@ def set_raw(self, value: Any) -> None: def snapshot_base( self, - update: bool | SnapshotUpdate | None = True, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: + update = normalize_snapshot_update(update) snapshot = super().snapshot_base( update=update, params_to_skip_update=params_to_skip_update ) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index 062e74473fb8..da5abd902524 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -13,8 +13,12 @@ import numpy as np from typing_extensions import TypedDict, TypeVar -from qcodes.metadatable import Metadatable, MetadatableWithName, SnapshotUpdate -from qcodes.metadatable.metadatable_base import _normalize_snapshot_update +from qcodes.metadatable import ( + Metadatable, + MetadatableWithName, + SnapshotUpdate, + normalize_snapshot_update, +) from qcodes.parameters import ParamSpecBase from qcodes.utils import ( DelegateAttributes, @@ -700,7 +704,7 @@ def __call__(self, *args: Any, **kwargs: Any) -> ParameterDataTypeVar | None: def snapshot_base( self, - update: bool | SnapshotUpdate | None = True, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -715,23 +719,22 @@ def snapshot_base( Args: update: What to do about the value stored in the snapshot. - * ``"All"`` (or legacy ``True``): update the state by calling - ``parameter.get()`` unless ``snapshot_get`` of the parameter - is ``False``. - * ``"Only_invalid"`` (or legacy ``None``): call - ``parameter.get()`` only if the parameter's cache is invalid, - i.e. use ``cache.get(get_if_invalid=True)``, otherwise use the - cached value. This never calls ``get()`` if ``snapshot_get`` - is ``False`` or the parameter is not gettable. - * ``"Never"`` (or legacy ``False``): never call - ``parameter.get()``, always use the latest cached value. + * ``"All"``: update the state by calling ``parameter.get()`` + unless ``snapshot_get`` of the parameter is ``False``. + * ``"Only_invalid"`` (the default): call ``parameter.get()`` + only if the parameter's cache is invalid, i.e. use + ``cache.get(get_if_invalid=True)``, otherwise use the cached + value. This never calls ``get()`` if ``snapshot_get`` is + ``False`` or the parameter is not gettable. + * ``"Never"``: never call ``parameter.get()``, always use the + latest cached value. params_to_skip_update: No effect but may be passed from superclass Returns: base snapshot """ - update = _normalize_snapshot_update(update) + update = normalize_snapshot_update(update) if self.snapshot_exclude: warnings.warn( @@ -745,15 +748,18 @@ def snapshot_base( if self.snapshot_value: has_get = self.gettable allowed_to_call_get_when_snapshotting = ( - self._snapshot_get and update is not False + self._snapshot_get and update != "Never" ) can_call_get_when_snapshotting = ( allowed_to_call_get_when_snapshotting and has_get ) - if can_call_get_when_snapshotting and update: + if can_call_get_when_snapshotting and update == "All": state["value"] = self.get() else: + # ``get_if_invalid`` is True only for ``"Only_invalid"`` (when + # the parameter is gettable and ``snapshot_get`` is True), so + # that only parameters with an invalid cache are refreshed. state["value"] = self.cache.get( get_if_invalid=can_call_get_when_snapshotting ) diff --git a/src/qcodes/parameters/sweep_values.py b/src/qcodes/parameters/sweep_values.py index bdce9273c152..abea5f25e7ce 100644 --- a/src/qcodes/parameters/sweep_values.py +++ b/src/qcodes/parameters/sweep_values.py @@ -5,7 +5,7 @@ import numpy as np -from qcodes.metadatable import Metadatable +from qcodes.metadatable import Metadatable, normalize_snapshot_update from .named_repr import named_repr from .permissive_range import permissive_range @@ -349,7 +349,7 @@ def reverse(self) -> None: def snapshot_base( self, - update: bool | SnapshotUpdate | None = False, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -363,7 +363,9 @@ def snapshot_base( dict: base snapshot """ - self._snapshot["parameter"] = self.parameter.snapshot(update=update) + self._snapshot["parameter"] = self.parameter.snapshot( + update=normalize_snapshot_update(update) + ) self._snapshot["values"] = self._value_snapshot return self._snapshot diff --git a/src/qcodes/station.py b/src/qcodes/station.py index 8ce7871d1c26..9bbb50de4446 100644 --- a/src/qcodes/station.py +++ b/src/qcodes/station.py @@ -36,7 +36,11 @@ from qcodes import validators from qcodes.instrument import Instrument, InstrumentBase from qcodes.instrument.channel import ChannelTuple -from qcodes.metadatable import Metadatable, MetadatableWithName +from qcodes.metadatable import ( + Metadatable, + MetadatableWithName, + normalize_snapshot_update, +) from qcodes.monitor.monitor import Monitor from qcodes.parameters import ( DelegateParameter, @@ -187,7 +191,7 @@ def __init__( def snapshot_base( self, - update: bool | SnapshotUpdate | None = True, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -202,9 +206,8 @@ def snapshot_base( Args: update: What to do about the values stored in the snapshot of the children (f.ex. instruments, parameters, components, etc.). - ``"All"`` (or legacy ``True``) updates every value, - ``"Only_invalid"`` (or legacy ``None``) only updates values - whose cache is invalid, and ``"Never"`` (or legacy ``False``) + ``"All"`` updates every value, ``"Only_invalid"`` (the default) + only updates values whose cache is invalid, and ``"Never"`` never updates and uses the latest values in memory. params_to_skip_update: Not used. @@ -212,6 +215,7 @@ def snapshot_base( dict: Base snapshot. """ + update = normalize_snapshot_update(update) snap: dict[str, Any] = { "instruments": {}, "parameters": {}, @@ -263,7 +267,7 @@ def add_component( """ try: if not (isinstance(component, Parameter) and component.snapshot_exclude): - component.snapshot(update=update_snapshot) + component.snapshot(update="All" if update_snapshot else "Never") except Exception: pass if name is None: diff --git a/tests/drivers/keysight_b1500/b1500_driver_tests/test_b1500.py b/tests/drivers/keysight_b1500/b1500_driver_tests/test_b1500.py index e9fd04f2ed31..1fe0b5973e2a 100644 --- a/tests/drivers/keysight_b1500/b1500_driver_tests/test_b1500.py +++ b/tests/drivers/keysight_b1500/b1500_driver_tests/test_b1500.py @@ -63,7 +63,7 @@ def test_init(b1500: KeysightB1500) -> None: def test_snapshot_does_not_raise_warnings(b1500: KeysightB1500) -> None: with warnings.catch_warnings(): warnings.simplefilter("error") - b1500.snapshot(update=True) + b1500.snapshot(update="All") def test_submodule_access_by_class(b1500: KeysightB1500) -> None: diff --git a/tests/drivers/test_ami430_visa.py b/tests/drivers/test_ami430_visa.py index ba98879e2471..cb9a8d621b5c 100644 --- a/tests/drivers/test_ami430_visa.py +++ b/tests/drivers/test_ami430_visa.py @@ -1445,7 +1445,7 @@ def test_switch_heater_enabled(ami430: AMIModel430, caplog: LogCaptureFixture) - # make sure that getting snapshot with heater disabled works without warning caplog.clear() with caplog.at_level(logging.WARNING, logger=ami430.log.name): - snap = ami430.snapshot(update=True) + snap = ami430.snapshot(update="All") assert len(caplog.records) == 0 # When heater is disabled, heater-specific parameters should not be updated @@ -1463,7 +1463,7 @@ def test_switch_heater_enabled(ami430: AMIModel430, caplog: LogCaptureFixture) - # When heater is enabled, snapshot should update all parameters ami430.switch_heater.enabled(True) - snap_enabled = ami430.snapshot(update=True) + snap_enabled = ami430.snapshot(update="All") heater_snap_enabled = snap_enabled["submodules"]["switch_heater"]["parameters"] for param_name in ( "state", diff --git a/tests/parameter/conftest.py b/tests/parameter/conftest.py index 6e61f4265f8d..1a4450581750 100644 --- a/tests/parameter/conftest.py +++ b/tests/parameter/conftest.py @@ -12,6 +12,7 @@ from collections.abc import Callable, Generator from qcodes.instrument import InstrumentBase + from qcodes.metadatable import SnapshotUpdate T = TypeVar("T") @@ -40,8 +41,10 @@ def get_if_invalid(request: pytest.FixtureRequest) -> bool | Literal["NOT_PASSED return request.param -@pytest.fixture(params=(True, False, None, NOT_PASSED)) -def update(request: pytest.FixtureRequest) -> bool | Literal["NOT_PASSED"] | None: +@pytest.fixture(params=("All", "Never", "Only_invalid", NOT_PASSED)) +def update( + request: pytest.FixtureRequest, +) -> SnapshotUpdate | Literal["NOT_PASSED"]: return request.param diff --git a/tests/parameter/test_array_parameter.py b/tests/parameter/test_array_parameter.py index e169ac3289b6..8d9ff7cc75e5 100644 --- a/tests/parameter/test_array_parameter.py +++ b/tests/parameter/test_array_parameter.py @@ -41,7 +41,7 @@ def test_default_attributes() -> None: assert str(p) == name assert p._get_count == 0 - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert p._get_count == 0 snap_expected = {"name": name, "label": name, "unit": ""} for k, v in snap_expected.items(): @@ -58,7 +58,7 @@ def test_snapshot_value_default_false() -> None: """snapshot_value defaults to False for ArrayParameter.""" p = SimpleArrayParam([1, 2, 3], "arr", shape=(3,)) assert p._snapshot_value is False - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert "value" not in snap assert "raw_value" not in snap @@ -67,7 +67,7 @@ def test_snapshot_value_explicit_true() -> None: """snapshot_value=True includes value in snapshot for ArrayParameter.""" p = SimpleArrayParam([1, 2, 3], "arr", shape=(3,), snapshot_value=True) assert p._snapshot_value is True - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert snap["value"] == [1, 2, 3] @@ -75,7 +75,7 @@ def test_snapshot_value_explicit_false() -> None: """snapshot_value=False excludes value from snapshot for ArrayParameter.""" p = SimpleArrayParam([1, 2, 3], "arr", shape=(3,), snapshot_value=False) assert p._snapshot_value is False - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert "value" not in snap @@ -113,7 +113,7 @@ def test_explicit_attributes() -> None: assert p.setpoint_labels == setpoint_labels assert p._get_count == 0 - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert p._get_count == 1 snap_expected = { "name": name, diff --git a/tests/parameter/test_delegate_parameter.py b/tests/parameter/test_delegate_parameter.py index 90d15aef3b60..b2b47985de70 100644 --- a/tests/parameter/test_delegate_parameter.py +++ b/tests/parameter/test_delegate_parameter.py @@ -444,7 +444,7 @@ def _assert_none_source_is_correct(delegate_param: DelegateParameter) -> None: assert snapshot["source_parameter"] is None assert "value" not in snapshot.keys() snapshot.pop("ts") - updated_snapshot = delegate_param.snapshot(update=True) + updated_snapshot = delegate_param.snapshot(update="All") updated_snapshot.pop("ts") assert snapshot == updated_snapshot diff --git a/tests/parameter/test_multi_parameter.py b/tests/parameter/test_multi_parameter.py index 585d0813c117..5e60778a9657 100644 --- a/tests/parameter/test_multi_parameter.py +++ b/tests/parameter/test_multi_parameter.py @@ -45,7 +45,7 @@ def test_default_attributes() -> None: assert str(p) == name assert p._get_count == 0 - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert p._get_count == 0 snap_expected = { "name": name, @@ -71,7 +71,7 @@ def test_snapshot_value_default_false() -> None: """snapshot_value defaults to False for MultiParameter.""" p = SimpleMultiParam([0], "mp", names=("x",), shapes=((),)) assert p._snapshot_value is False - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert "value" not in snap assert "raw_value" not in snap @@ -80,7 +80,7 @@ def test_snapshot_value_explicit_true() -> None: """snapshot_value=True includes value in snapshot for MultiParameter.""" p = SimpleMultiParam([0], "mp", names=("x",), shapes=((),), snapshot_value=True) assert p._snapshot_value is True - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert snap["value"] == [0] @@ -88,7 +88,7 @@ def test_snapshot_value_explicit_false() -> None: """snapshot_value=False excludes value from snapshot for MultiParameter.""" p = SimpleMultiParam([0], "mp", names=("x",), shapes=((),), snapshot_value=False) assert p._snapshot_value is False - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert "value" not in snap @@ -132,7 +132,7 @@ def test_explicit_attributes() -> None: assert p.setpoint_labels == setpoint_labels assert p._get_count == 0 - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert p._get_count == 1 snap_expected = { "name": name, diff --git a/tests/parameter/test_parameter_basics.py b/tests/parameter/test_parameter_basics.py index 6dd9628789d4..18a5446e1370 100644 --- a/tests/parameter/test_parameter_basics.py +++ b/tests/parameter/test_parameter_basics.py @@ -39,7 +39,7 @@ def test_default_attributes() -> None: # test snapshot_get by looking at _get_count # by default, snapshot_get is True, hence we expect ``get`` to be called assert p._get_count == 0 - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert p._get_count == 1 snap_expected = { "name": name, @@ -89,7 +89,7 @@ def test_explicit_attributes() -> None: # test snapshot_get by looking at _get_count assert p._get_count == 0 # Snapshot should not perform get since snapshot_get is False - snap = p.snapshot(update=True) + snap = p.snapshot(update="All") assert p._get_count == 0 snap_expected = { "name": name, diff --git a/tests/parameter/test_parameter_cache.py b/tests/parameter/test_parameter_cache.py index 1768b7de72f5..f39245304365 100644 --- a/tests/parameter/test_parameter_cache.py +++ b/tests/parameter/test_parameter_cache.py @@ -385,7 +385,7 @@ def _assert_cache_status(valid: bool) -> None: for instrument_module in dummy_instrument.instrument_modules.values(): for param in instrument_module.parameters.values(): # parameters not snapshotted will not have a cache - # updated when calling snapshot(update=None) os + # updated when calling snapshot(update="Only_invalid") os # exclude them if ( param._snapshot_get is True @@ -394,7 +394,7 @@ def _assert_cache_status(valid: bool) -> None: ): assert param.cache.valid is valid, param.full_name - dummy_instrument.snapshot(update=None) + dummy_instrument.snapshot(update="Only_invalid") _assert_cache_status(True) @@ -402,7 +402,7 @@ def _assert_cache_status(valid: bool) -> None: _assert_cache_status(False) - dummy_instrument.snapshot(update=None) + dummy_instrument.snapshot(update="Only_invalid") _assert_cache_status(True) diff --git a/tests/parameter/test_snapshot.py b/tests/parameter/test_snapshot.py index ecf3c7b2f96b..0b35a29f2924 100644 --- a/tests/parameter/test_snapshot.py +++ b/tests/parameter/test_snapshot.py @@ -6,7 +6,7 @@ import pytest from typing_extensions import ParamSpec -from qcodes.metadatable.metadatable_base import _normalize_snapshot_update +from qcodes.metadatable import normalize_snapshot_update from qcodes.parameters import Parameter from .conftest import NOT_PASSED @@ -14,6 +14,8 @@ if TYPE_CHECKING: from collections.abc import Callable + from qcodes.metadatable import SnapshotUpdate + T = TypeVar("T") P = ParamSpec("P") @@ -76,7 +78,7 @@ def test_snapshot_contains_parameter_attributes( snapshot_value: bool | Literal["NOT_PASSED"], get_cmd: Literal[False, "NOT_PASSED"] | None, cache_is_valid: bool, - update: bool | Literal["NOT_PASSED"] | None, + update: SnapshotUpdate | Literal["NOT_PASSED"], ) -> None: p = create_parameter(snapshot_get, snapshot_value, cache_is_valid, get_cmd) @@ -122,7 +124,7 @@ def test_snapshot_contains_parameter_attributes( def test_snapshot_timestamp_of_non_gettable_depends_only_on_cache_validity( snapshot_get: bool | Literal["NOT_PASSED"], snapshot_value: bool | Literal["NOT_PASSED"], - update: bool | Literal["NOT_PASSED"] | None, + update: SnapshotUpdate | Literal["NOT_PASSED"], cache_is_valid: bool, ) -> None: p = create_parameter(snapshot_get, snapshot_value, cache_is_valid, get_cmd=False) @@ -147,7 +149,7 @@ def test_snapshot_timestamp_of_non_gettable_depends_only_on_cache_validity( def test_snapshot_timestamp_for_valid_cache_depends_on_cache_update( snapshot_get: bool | Literal["NOT_PASSED"], snapshot_value: bool | Literal["NOT_PASSED"], - update: bool | Literal["NOT_PASSED"] | None, + update: SnapshotUpdate | Literal["NOT_PASSED"], ) -> None: p = create_parameter( snapshot_get, snapshot_value, get_cmd=lambda: 69, cache_is_valid=True @@ -169,8 +171,9 @@ def test_snapshot_timestamp_for_valid_cache_depends_on_cache_update( ts = datetime.strptime(s["ts"], "%Y-%m-%d %H:%M:%S") tu_up_to_seconds = tu.replace(microsecond=0) + effective = "Only_invalid" if update == NOT_PASSED else update cache_gets_updated_on_snapshot_call = ( - snapshot_value is not False and snapshot_get is not False and update is True + snapshot_value is not False and snapshot_get is not False and effective == "All" ) if cache_gets_updated_on_snapshot_call: @@ -182,17 +185,17 @@ def test_snapshot_timestamp_for_valid_cache_depends_on_cache_update( def test_snapshot_timestamp_for_invalid_cache_depends_only_on_snapshot_flags( snapshot_get: bool | Literal["NOT_PASSED"], snapshot_value: bool | Literal["NOT_PASSED"], - update: bool | Literal["NOT_PASSED"] | None, + update: SnapshotUpdate | Literal["NOT_PASSED"], ) -> None: p = create_parameter( snapshot_get, snapshot_value, get_cmd=lambda: 69, cache_is_valid=False ) + effective = "Only_invalid" if update == NOT_PASSED else update cache_gets_updated_on_snapshot_call = ( snapshot_value is not False and snapshot_get is not False - and update is not False - and update != NOT_PASSED + and effective != "Never" ) if cache_gets_updated_on_snapshot_call: @@ -218,7 +221,7 @@ def test_snapshot_when_snapshot_value_is_false( snapshot_get: bool | Literal["NOT_PASSED"], get_cmd: Literal[False, "NOT_PASSED"] | None, cache_is_valid: bool, - update: bool | Literal["NOT_PASSED"] | None, + update: SnapshotUpdate | Literal["NOT_PASSED"], ) -> None: p = create_parameter( snapshot_get=snapshot_get, @@ -267,7 +270,7 @@ def test_snapshot_get_is_true_by_default( def test_snapshot_when_snapshot_get_is_false( get_cmd: Literal[False, "NOT_PASSED"] | None, - update: bool | Literal["NOT_PASSED"] | None, + update: SnapshotUpdate | Literal["NOT_PASSED"], cache_is_valid: bool, ) -> None: p = create_parameter( @@ -295,7 +298,7 @@ def test_snapshot_when_snapshot_get_is_false( def test_snapshot_of_non_gettable_parameter_mirrors_cache( - update: bool | Literal["NOT_PASSED"] | None, cache_is_valid: bool + update: SnapshotUpdate | Literal["NOT_PASSED"], cache_is_valid: bool ) -> None: p = create_parameter( snapshot_get=True, @@ -319,7 +322,7 @@ def test_snapshot_of_non_gettable_parameter_mirrors_cache( def test_snapshot_of_gettable_parameter_depends_on_update( - update: bool | Literal["NOT_PASSED"] | None, cache_is_valid: bool + update: SnapshotUpdate | Literal["NOT_PASSED"], cache_is_valid: bool ) -> None: p = create_parameter( snapshot_get=True, @@ -334,18 +337,23 @@ def test_snapshot_of_gettable_parameter_depends_on_update( else: s = p.snapshot() - if update is not True and cache_is_valid: + effective = "Only_invalid" if update == NOT_PASSED else update + should_get = effective == "All" or ( + effective == "Only_invalid" and not cache_is_valid + ) + + if should_get: + assert s["value"] == 65 + assert s["raw_value"] == 69 + assert p.get.call_count() == 1 # type: ignore[attr-defined] + elif cache_is_valid: assert s["value"] == 42 assert s["raw_value"] == 46 assert p.get.call_count() == 0 # type: ignore[attr-defined] - elif update is False or update == NOT_PASSED: + else: assert s["value"] is None assert s["raw_value"] is None assert p.get.call_count() == 0 # type: ignore[attr-defined] - else: - assert s["value"] == 65 - assert s["raw_value"] == 69 - assert p.get.call_count() == 1 # type: ignore[attr-defined] def test_snapshot_value() -> None: @@ -367,19 +375,20 @@ def test_snapshot_value() -> None: assert "ts" in snap -def test_normalize_snapshot_update_maps_strings_to_legacy_values() -> None: - assert _normalize_snapshot_update("All") is True - assert _normalize_snapshot_update("Only_invalid") is None - assert _normalize_snapshot_update("Never") is False - # legacy values are passed through unchanged - assert _normalize_snapshot_update(True) is True - assert _normalize_snapshot_update(False) is False - assert _normalize_snapshot_update(None) is None +def test_normalize_snapshot_update_maps_to_canonical_values() -> None: + # canonical string values are returned unchanged + assert normalize_snapshot_update("All") == "All" + assert normalize_snapshot_update("Only_invalid") == "Only_invalid" + assert normalize_snapshot_update("Never") == "Never" + # legacy values are mapped to the canonical string values + assert normalize_snapshot_update(True) == "All" + assert normalize_snapshot_update(None) == "Only_invalid" + assert normalize_snapshot_update(False) == "Never" def test_normalize_snapshot_update_rejects_unknown_string() -> None: with pytest.raises(ValueError, match="Invalid value for snapshot"): - _normalize_snapshot_update("bogus") # type: ignore[arg-type] + normalize_snapshot_update("bogus") # type: ignore[arg-type] def test_snapshot_update_all_always_calls_get() -> None: @@ -453,7 +462,9 @@ def test_snapshot_update_string_matches_legacy_value( cache_is_valid=cache_is_valid, ) - s_legacy = p_legacy.snapshot(update=legacy) + # ``update=legacy`` intentionally uses the deprecated bool/None values to + # confirm they still map to the new canonical behavior. + s_legacy = p_legacy.snapshot(update=legacy) # pyright: ignore[reportDeprecated] s_string = p_string.snapshot(update=string) assert s_legacy["value"] == s_string["value"] diff --git a/tests/test_instrument.py b/tests/test_instrument.py index cbd4ce126319..603d31816e8d 100644 --- a/tests/test_instrument.py +++ b/tests/test_instrument.py @@ -357,7 +357,7 @@ def test_meta_instrument(parabola) -> None: assert mock_instrument.parabola() == parabola.parabola() * 2 # Check snapshots - snap = mock_instrument.snapshot(update=True) + snap = mock_instrument.snapshot(update="All") assert "parameters" in snap assert "gain" in snap["parameters"] assert snap["parameters"]["gain"]["value"] == 2 diff --git a/tests/test_snapshot.py b/tests/test_snapshot.py index 86a4daffa039..407b2011cf4c 100644 --- a/tests/test_snapshot.py +++ b/tests/test_snapshot.py @@ -38,11 +38,11 @@ def test_snapshot_skip_params_update( assert list(inst._get_calls.values()) == [0, 0, 0, 0] - inst.snapshot(update=False) + inst.snapshot(update="Never") assert list(inst._get_calls.values()) == [0, 0, 0, 0] - inst.snapshot(update=True) + inst.snapshot(update="All") expected_list = [1, 1, 1, 1] if params_to_skip: From 3161889359fa62cc26b72c043a1a4d56b64e0fe9 Mon Sep 17 00:00:00 2001 From: Mikhail Astafev Date: Fri, 31 Jul 2026 12:46:45 +0200 Subject: [PATCH 4/5] Address review: only-invalid-aware QDac bulk refresh and CombinedParameter forwarding - QDac snapshot: only perform the bulk status read (_update_cache) for update='All', or for 'Only_invalid' when a channel cache that would be reported is actually invalid, instead of on every non-'Never' snapshot. Clarify the confusing comment (parent._update -> self.parent._update_currents). - CombinedParameter.snapshot_base now forwards the normalized update value to the underlying parameters' snapshots (previously it ignored update). Add a test. - Fix 'os' -> 'so' typo in a test comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa3f57b5-ddc1-45ce-b920-3f8efed6851c --- .../instrument_drivers/QDev/QDac_channels.py | 55 ++++++++++++------- src/qcodes/parameters/combined_parameter.py | 5 +- tests/parameter/test_combined_par.py | 25 ++++++++- tests/parameter/test_parameter_cache.py | 2 +- 4 files changed, 63 insertions(+), 24 deletions(-) diff --git a/src/qcodes/instrument_drivers/QDev/QDac_channels.py b/src/qcodes/instrument_drivers/QDev/QDac_channels.py index b3720e9a89ba..bce4fe753af6 100644 --- a/src/qcodes/instrument_drivers/QDev/QDac_channels.py +++ b/src/qcodes/instrument_drivers/QDev/QDac_channels.py @@ -145,26 +145,26 @@ def snapshot_base( params_to_skip_update: "Sequence[str] | None" = None, ) -> dict[Any, Any]: update = normalize_snapshot_update(update) - # setting update to "All"/"Only_invalid" (i.e. not "Never") will - # override parent setting; otherwise we use the parent setting - # parent._update | update | do update - # True | "All" | True - # True | "Only_invalid" | True - # True | "Never" | False - # False | "All" | True - # False | "Only_invalid" | False - # False | "Never" | False - update_currents = ( - self.parent._update_currents and update != "Never" - ) or update == "All" - if update != "Never" and not self.parent._get_status_performed: - self.parent._update_cache(readcurrents=update_currents) - # call get_status rather than getting the status individually for - # each parameter. This is only done if _get_status_performed is False - # this is used to signal that the parent has already called it and - # no need to repeat. if params_to_skip_update is None: params_to_skip_update = ("v", "i", "irange", "vrange") + # The channel parameters in ``params_to_skip_update`` are not queried + # individually during the snapshot; instead a single bulk status read + # on the parent (``self.parent._update_cache``) refreshes their caches. + # Only trigger that (potentially expensive) read when forced ("All") + # or, for "Only_invalid", when one of those caches is actually invalid. + # ``self.parent._get_status_performed`` signals that the parent has + # already performed the bulk read for all channels. + needs_bulk_update = update == "All" or ( + update == "Only_invalid" + and any( + not self.parameters[name].cache.valid + for name in params_to_skip_update + if name in self.parameters + ) + ) + if needs_bulk_update and not self.parent._get_status_performed: + readcurrents = self.parent._update_currents or update == "All" + self.parent._update_cache(readcurrents=readcurrents) snap = super().snapshot_base( update=update, params_to_skip_update=params_to_skip_update ) @@ -344,8 +344,23 @@ def snapshot_base( params_to_skip_update: "Sequence[str] | None" = None, ) -> dict[Any, Any]: update = normalize_snapshot_update(update) - update_currents = self._update_currents and update == "All" - if update != "Never": + # As in the per-channel snapshot, only perform the bulk status read + # (``_update_cache``, which refreshes the ``v``/``i``/``irange``/ + # ``vrange`` caches of every channel) when forced ("All") or, for + # "Only_invalid", when one of those channel caches is actually invalid. + if update == "All": + needs_bulk_update = True + elif update == "Only_invalid": + needs_bulk_update = any( + not chan.parameters[name].cache.valid + for chan in self.channels + for name in ("v", "i", "irange", "vrange") + if name in chan.parameters + ) + else: # "Never" + needs_bulk_update = False + if needs_bulk_update: + update_currents = self._update_currents and update == "All" self._update_cache(readcurrents=update_currents) self._get_status_performed = True # call get_status rather than getting the status individually for diff --git a/src/qcodes/parameters/combined_parameter.py b/src/qcodes/parameters/combined_parameter.py index 96d434736165..84d4d48f39e3 100644 --- a/src/qcodes/parameters/combined_parameter.py +++ b/src/qcodes/parameters/combined_parameter.py @@ -8,7 +8,7 @@ import numpy as np import numpy.typing as npt -from qcodes.metadatable import Metadatable +from qcodes.metadatable import Metadatable, normalize_snapshot_update from qcodes.utils import full_class if TYPE_CHECKING: @@ -219,7 +219,8 @@ def snapshot_base( meta_data["label"] = param.label # type: ignore[attr-defined] meta_data["full_name"] = param.full_name # type: ignore[attr-defined] meta_data["aggregator"] = repr(getattr(self, "f", None)) + update = normalize_snapshot_update(update) for parameter in self.parameters: - meta_data[str(parameter)] = parameter.snapshot() + meta_data[str(parameter)] = parameter.snapshot(update=update) return meta_data diff --git a/tests/parameter/test_combined_par.py b/tests/parameter/test_combined_par.py index 913a5f87c622..e81fcf82c3c3 100644 --- a/tests/parameter/test_combined_par.py +++ b/tests/parameter/test_combined_par.py @@ -8,7 +8,7 @@ import pytest from hypothesis import HealthCheck, given, settings -from qcodes.parameters import ManualParameter, combine +from qcodes.parameters import ManualParameter, Parameter, combine from qcodes.utils import full_class if TYPE_CHECKING: @@ -127,6 +127,29 @@ def test_meta(parameters: list[ManualParameter]) -> None: assert out == snap +def test_snapshot_forwards_update_to_underlying_parameters() -> None: + calls = {"n": 0} + + def getter() -> int: + calls["n"] += 1 + return 5 + + gettable = Parameter("gettable", get_cmd=getter, set_cmd=None) + other = ManualParameter("other", initial_value=1) + combined = combine(gettable, other, name="combined") + + # cache of ``gettable`` is invalid; ``"Never"`` must not call ``get`` + assert not gettable.cache.valid + calls["n"] = 0 + combined.snapshot(update="Never") + assert calls["n"] == 0 + + # ``"Only_invalid"`` refreshes the invalid cache via a single ``get`` + calls["n"] = 0 + combined.snapshot(update="Only_invalid") + assert calls["n"] == 1 + + def test_mutable(parameters: list[ManualParameter]) -> None: setpoints = np.array([[1, 1, 1], [1, 1, 1]]) diff --git a/tests/parameter/test_parameter_cache.py b/tests/parameter/test_parameter_cache.py index f39245304365..a9ab2653c412 100644 --- a/tests/parameter/test_parameter_cache.py +++ b/tests/parameter/test_parameter_cache.py @@ -385,7 +385,7 @@ def _assert_cache_status(valid: bool) -> None: for instrument_module in dummy_instrument.instrument_modules.values(): for param in instrument_module.parameters.values(): # parameters not snapshotted will not have a cache - # updated when calling snapshot(update="Only_invalid") os + # updated when calling snapshot(update="Only_invalid") so # exclude them if ( param._snapshot_get is True From 64135f4fbd2bf00a0d2471d16a3339e930321310 Mon Sep 17 00:00:00 2001 From: Mikhail Astafev Date: Fri, 31 Jul 2026 14:00:52 +0200 Subject: [PATCH 5/5] Add newsfragment for QDac only-invalid snapshot improvement Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa3f57b5-ddc1-45ce-b920-3f8efed6851c --- docs/changes/newsfragments/8354.improved_driver | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 docs/changes/newsfragments/8354.improved_driver diff --git a/docs/changes/newsfragments/8354.improved_driver b/docs/changes/newsfragments/8354.improved_driver new file mode 100644 index 000000000000..c9ced65a5da6 --- /dev/null +++ b/docs/changes/newsfragments/8354.improved_driver @@ -0,0 +1,7 @@ +The QDev QDac driver now honors the ``"Only_invalid"`` snapshot ``update`` mode: +when snapshotting, the bulk status read that refreshes the channel +``v``/``i``/``irange``/``vrange`` caches is only performed for ``update="All"``, +or for ``update="Only_invalid"`` when one of those channel caches is actually +invalid. Previously the bulk read was performed on every snapshot that was not +``"Never"``, which made repeated snapshots unnecessarily expensive even when the +channel caches were already valid.