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.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. diff --git a/docs/changes/newsfragments/8354.new b/docs/changes/newsfragments/8354.new new file mode 100644 index 000000000000..768f35e81b00 --- /dev/null +++ b/docs/changes/newsfragments/8354.new @@ -0,0 +1,13 @@ +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 3437bbb8a7c6..35f4418e7be4 100644 --- a/src/qcodes/dataset/measurements.py +++ b/src/qcodes/dataset/measurements.py @@ -663,17 +663,17 @@ 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: 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 1e807320e4da..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, @@ -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 = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -389,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. @@ -404,6 +406,7 @@ def snapshot_base( dict: base snapshot """ + update = normalize_snapshot_update(update) if self._snapshotable: snap = { "channels": { @@ -611,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 c3030c102200..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 @@ -21,6 +25,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 +412,7 @@ def _get_component_by_name( def snapshot_base( self, - update: bool | None = False, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -417,13 +422,13 @@ 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"``, 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. @@ -432,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 = [] @@ -453,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 | None = False + update_par: SnapshotUpdate = "Never" else: update_par = update try: @@ -463,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) @@ -476,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. @@ -486,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 28940d80f81e..286b0a6ca266 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 = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -223,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 4d91ce6b7e24..f952d1c6babe 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 = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -459,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 21ff305c7f78..ff0411dc55aa 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 = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: snapshot = super().snapshot_base( @@ -1019,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, @@ -1401,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 94e67aab13a8..ffd26be092aa 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" = "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 67b95e44ae6e..bce4fe753af6 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 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,29 +141,30 @@ def __init__( def snapshot_base( self, - update: bool | None = False, + update: "bool | SnapshotUpdate | None" = "Only_invalid", params_to_skip_update: "Sequence[str] | None" = None, ) -> dict[Any, Any]: - # 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_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_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. + update = normalize_snapshot_update(update) 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 ) @@ -337,11 +340,27 @@ def __init__( def snapshot_base( self, - update: bool | None = False, + update: "bool | SnapshotUpdate | None" = "Only_invalid", params_to_skip_update: "Sequence[str] | None" = None, ) -> dict[Any, Any]: - update_currents = self._update_currents and update is True - if update: + update = normalize_snapshot_update(update) + # 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/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 651b76822197..c82dc0a2273e 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 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,13 +152,14 @@ def _check_state(self) -> bool: def snapshot_base( self, - update: bool | None = False, + update: bool | SnapshotUpdate | None = "Only_invalid", 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 = [] - 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 edd248da108d..9f0da5711d31 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 = "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 a7bdc84d3aad..cfea0dea5cdb 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 = "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 a3c097e7fcaf..ce212a8d84a8 100644 --- a/src/qcodes/metadatable/__init__.py +++ b/src/qcodes/metadatable/__init__.py @@ -1,3 +1,13 @@ -from .metadatable_base import Metadatable, MetadatableWithName +from .metadatable_base import ( + Metadatable, + MetadatableWithName, + SnapshotUpdate, + normalize_snapshot_update, +) -__all__ = ["Metadatable", "MetadatableWithName"] +__all__ = [ + "Metadatable", + "MetadatableWithName", + "SnapshotUpdate", + "normalize_snapshot_update", +] diff --git a/src/qcodes/metadatable/metadatable_base.py b/src/qcodes/metadatable/metadatable_base.py index a37897b4ee21..a0ab34f1b9c9 100644 --- a/src/qcodes/metadatable/metadatable_base.py +++ b/src/qcodes/metadatable/metadatable_base.py @@ -1,11 +1,70 @@ from abc import abstractmethod -from typing import TYPE_CHECKING, Any, final +from typing import TYPE_CHECKING, Any, Literal, final, overload + +from typing_extensions import deprecated from qcodes.utils import deep_update if TYPE_CHECKING: from collections.abc import Mapping, Sequence +SnapshotUpdate = Literal["All", "Only_invalid", "Never"] +""" +Canonical 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``). + +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( + update: "bool | SnapshotUpdate | None", +) -> SnapshotUpdate: + """ + Normalize the ``update`` argument of ``snapshot``/``snapshot_base`` into + 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. + + """ + 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 # for the value type doesn't tell us anything about the schema # followed by snapshots. @@ -33,22 +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 | 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. + 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=update) + snap = self.snapshot_base(update=normalize_snapshot_update(update)) if len(self.metadata): snap["metadata"] = self.metadata @@ -57,7 +142,7 @@ def snapshot(self, update: bool | None = False) -> Snapshot: def snapshot_base( self, - update: bool | 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 44f70b13912e..84d4d48f39e3 100644 --- a/src/qcodes/parameters/combined_parameter.py +++ b/src/qcodes/parameters/combined_parameter.py @@ -8,12 +8,14 @@ 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: 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 = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -203,7 +205,7 @@ def snapshot_base( :class:`.NumpyJSONEncoder` supports). Args: - update: ``True`` or ``False``. + update: One of ``"All"``, ``"Only_invalid"`` or ``"Never"``. params_to_skip_update: Unused in this subclass. Returns: @@ -217,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/src/qcodes/parameters/delegate_parameter.py b/src/qcodes/parameters/delegate_parameter.py index df8b2f6f2628..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 @@ -13,6 +15,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,9 +327,10 @@ def set_raw(self, value: Any) -> None: def snapshot_base( self, - update: bool | 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 dcbaa6b60bd0..da5abd902524 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -13,7 +13,12 @@ import numpy as np from typing_extensions import TypedDict, TypeVar -from qcodes.metadatable import Metadatable, MetadatableWithName +from qcodes.metadatable import ( + Metadatable, + MetadatableWithName, + SnapshotUpdate, + normalize_snapshot_update, +) from qcodes.parameters import ParamSpecBase from qcodes.utils import ( DelegateAttributes, @@ -699,7 +704,7 @@ def __call__(self, *args: Any, **kwargs: Any) -> ParameterDataTypeVar | None: def snapshot_base( self, - update: bool | None = True, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -712,17 +717,25 @@ 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"``: 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) + if self.snapshot_exclude: warnings.warn( f"Parameter ({self.full_name}) is used in the snapshot while it " @@ -735,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 0a261a1f3e4a..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 @@ -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 = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -362,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 1b604775f46d..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, @@ -56,6 +60,8 @@ from pathlib import Path from types import ModuleType + from qcodes.metadatable import SnapshotUpdate + log = logging.getLogger(__name__) PARAMETER_ATTRIBUTES = [ @@ -185,7 +191,7 @@ def __init__( def snapshot_base( self, - update: bool | None = True, + update: bool | SnapshotUpdate | None = "Only_invalid", params_to_skip_update: Sequence[str] | None = None, ) -> dict[Any, Any]: """ @@ -198,18 +204,18 @@ 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"`` 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. Returns: dict: Base snapshot. """ + update = normalize_snapshot_update(update) snap: dict[str, Any] = { "instruments": {}, "parameters": {}, @@ -261,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/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/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_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_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..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=None) os + # updated when calling snapshot(update="Only_invalid") so # 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 8f48d9ea8f77..0b35a29f2924 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 import normalize_snapshot_update from qcodes.parameters import Parameter from .conftest import NOT_PASSED @@ -12,6 +14,8 @@ if TYPE_CHECKING: from collections.abc import Callable + from qcodes.metadatable import SnapshotUpdate + T = TypeVar("T") P = ParamSpec("P") @@ -74,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) @@ -120,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) @@ -145,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 @@ -167,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: @@ -180,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: @@ -216,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, @@ -265,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( @@ -293,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, @@ -317,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, @@ -332,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: @@ -363,3 +373,114 @@ 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_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] + + +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, + ) + + # ``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"] + 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] 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: