From b28a7c3a5a940eef2cce7e5a785d3d68302ed5a6 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Wed, 26 Aug 2026 16:58:04 +0200 Subject: [PATCH 1/4] Scale and offset sequences element wise when setting a parameter Multiplying a sequence by a number repeats it rather than scaling its elements, so setting a parameter to a list with a scalar scale silently set a repeated raw value, and a scalar offset raised a TypeError. The get path already falls back on element wise arithmetic; do the same on the set path so that sequences round trip. See #8450 for the remaining silent truncation on length mismatch. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e8c5964a-6418-4d35-b69c-bb44dd727a3c --- docs/changes/newsfragments/8451.improved | 7 ++ src/qcodes/parameters/parameter_base.py | 8 +++ .../parameter/test_parameter_scale_offset.py | 65 +++++++++++++++++++ 3 files changed, 80 insertions(+) create mode 100644 docs/changes/newsfragments/8451.improved diff --git a/docs/changes/newsfragments/8451.improved b/docs/changes/newsfragments/8451.improved new file mode 100644 index 00000000000..c67ed8db1b9 --- /dev/null +++ b/docs/changes/newsfragments/8451.improved @@ -0,0 +1,7 @@ +Fixed applying a scalar ``scale`` or ``offset`` when setting a parameter to a +sequence such as a ``list`` or a ``tuple``. Multiplying a sequence by a number +repeats it rather than scaling its elements, so ``param([10, 20])`` with +``param.scale = 2`` used to set the raw value to ``[10, 20, 10, 20]``, and a +scalar ``offset`` raised a ``TypeError``. Sequences are now converted element +wise, matching how the values are converted back when the parameter is read. +Numpy arrays are unaffected since they scale and offset element wise already. diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index d20f1e27d15..6ec52341ade 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -270,6 +270,10 @@ def _scale_raw_value(raw_value: Any, scale: float | Iterable[float]) -> Any: if isinstance(scale, collections.abc.Iterable): # Scale contains multiple elements, one for each value return tuple(val * sub_scale for val, sub_scale in zip(raw_value, scale)) + if isinstance(raw_value, collections.abc.Sequence): + # Multiplying a sequence by a number repeats it rather than + # scaling its elements, so these must be handled element wise. + return tuple(val * scale for val in raw_value) # Use single scale for all values return raw_value * scale @@ -279,6 +283,10 @@ def _offset_raw_value(raw_value: Any, offset: float | Iterable[float]) -> Any: if isinstance(offset, collections.abc.Iterable): # offset contains multiple elements, one for each value return tuple(val + sub_offset for val, sub_offset in zip(raw_value, offset)) + if isinstance(raw_value, collections.abc.Sequence): + # Adding a number to a sequence is an error, so these must be + # handled element wise. + return tuple(val + offset for val in raw_value) # Use single offset for all values return raw_value + offset diff --git a/tests/parameter/test_parameter_scale_offset.py b/tests/parameter/test_parameter_scale_offset.py index 4350d1c5a92..6706b8583b0 100644 --- a/tests/parameter/test_parameter_scale_offset.py +++ b/tests/parameter/test_parameter_scale_offset.py @@ -261,6 +261,63 @@ def test_set_numpy_array_with_scalar_scale_and_offset() -> None: np.testing.assert_allclose(param.get(), [10, 20]) +@pytest.mark.parametrize("container", [list, tuple]) +def test_set_sequence_with_scalar_scale_and_offset(container: type) -> None: + """A list or tuple must be scaled element wise, not repeated.""" + param = Parameter(name="test_param", set_cmd=None, get_cmd=None) + param.scale = 2 + param.offset = 1 + + param(container([10, 20])) + + assert param.raw_value == (21, 41) + assert param.get() == (10, 20) + + +@pytest.mark.parametrize("container", [list, tuple]) +def test_set_sequence_with_scalar_scale_only(container: type) -> None: + param = Parameter(name="test_param", set_cmd=None, get_cmd=None) + param.scale = 2 + + param(container([10, 20])) + + assert param.raw_value == (20, 40) + assert param.get() == (10, 20) + + +@pytest.mark.parametrize("container", [list, tuple]) +def test_set_sequence_with_scalar_offset_only(container: type) -> None: + param = Parameter(name="test_param", set_cmd=None, get_cmd=None) + param.offset = 1 + + param(container([10, 20])) + + assert param.raw_value == (11, 21) + assert param.get() == (10, 20) + + +def test_set_sequence_with_scalar_scale_and_iterable_offset() -> None: + param = Parameter(name="test_param", set_cmd=None, get_cmd=None) + param.scale = 2 + param.offset = [1, 2] + + param([10, 20]) + + assert param.raw_value == (21, 42) + assert param.get() == (10, 20) + + +def test_set_sequence_with_iterable_scale_and_scalar_offset() -> None: + param = Parameter(name="test_param", set_cmd=None, get_cmd=None) + param.scale = [2, 4] + param.offset = 1 + + param([10, 20]) + + assert param.raw_value == (21, 81) + assert param.get() == (10, 20) + + def test_get_sequence_with_scalar_scale_and_offset() -> None: """A list valued raw value falls back on element wise arithmetic.""" param = Parameter(name="test_param", set_cmd=None, get_cmd=lambda: [10, 20]) @@ -294,12 +351,20 @@ def test_get_raises_for_non_numeric_value(attribute: str) -> None: def test_scale_raw_value_helper() -> None: assert _scale_raw_value(10, 2) == 20 assert _scale_raw_value([10, 20], [2, 4]) == (20, 80) + assert _scale_raw_value([10, 20], 2) == (20, 40) + assert _scale_raw_value((10, 20), 2) == (20, 40) + # any sequence, not just list and tuple + assert _scale_raw_value(range(10, 30, 10), 2) == (20, 40) np.testing.assert_allclose(_scale_raw_value(np.array([10, 20]), 2), [20, 40]) def test_offset_raw_value_helper() -> None: assert _offset_raw_value(10, 2) == 12 assert _offset_raw_value([10, 20], [2, 4]) == (12, 24) + assert _offset_raw_value([10, 20], 2) == (12, 22) + assert _offset_raw_value((10, 20), 2) == (12, 22) + # any sequence, not just list and tuple + assert _offset_raw_value(range(10, 30, 10), 2) == (12, 22) np.testing.assert_allclose(_offset_raw_value(np.array([10, 20]), 2), [12, 22]) From 50b073a12415ab3b2603d6877d6c394d067316a9 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Thu, 27 Aug 2026 08:04:57 +0200 Subject: [PATCH 2/4] Raise if scale or offset does not match the length of the value Zipping the value with an iterable scale or offset silently dropped the extra elements when their lengths differed. Apply the conversions via a shared helper that zips strictly and reports the two lengths, so that a mismatch is an error rather than a shorter value. Closes #8450 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e8c5964a-6418-4d35-b69c-bb44dd727a3c --- docs/changes/newsfragments/8452.breaking | 5 + src/qcodes/parameters/parameter_base.py | 49 +++++++++- .../parameter/test_parameter_scale_offset.py | 92 +++++++++++++++++++ 3 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 docs/changes/newsfragments/8452.breaking diff --git a/docs/changes/newsfragments/8452.breaking b/docs/changes/newsfragments/8452.breaking new file mode 100644 index 00000000000..93a0778d194 --- /dev/null +++ b/docs/changes/newsfragments/8452.breaking @@ -0,0 +1,5 @@ +Applying a ``scale`` or ``offset`` that does not match the length of the value +now raises a ``ValueError``. Previously the value and the scale/offset were +zipped together without checking their lengths, so a mismatch silently dropped +the extra elements, e.g. setting a parameter with ``scale = [2, 4]`` to +``[10, 20, 30]`` used to set the raw value to ``(20, 80)``. See :pr:`8450`. diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index 6ec52341ade..bda25bcf5a7 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -2,6 +2,7 @@ import collections.abc import logging +import operator import time import warnings from collections.abc import Iterator, MutableSet @@ -265,11 +266,51 @@ class ParameterBaseKWArgs( # generic ``ParameterDataTypeVar`` out of the arithmetic. +def _apply_elementwise( + value: Any, + conversion: Iterable[float], + operation: Callable[[Any, Any], Any], + kind: str, +) -> tuple[Any, ...]: + """Combine ``value`` and ``conversion`` element by element. + + Args: + value: The (iterable) value to convert. + conversion: The scale or offset to apply, one element per value. + operation: The arithmetic operation to apply to each pair of elements. + kind: The name of the conversion, used in the error message. + + Returns: + The converted values. + + Raises: + ValueError: If ``value`` and ``conversion`` are of different length. + + """ + try: + return tuple( + operation(val, sub_value) + for val, sub_value in zip(value, conversion, strict=True) + ) + except ValueError as err: + raise ValueError( + f"Cannot apply {kind} of length {_length_for_error(conversion)} " + f"to a value of length {_length_for_error(value)}." + ) from err + + +def _length_for_error(obj: Any) -> str: + """The length of ``obj`` for use in an error message.""" + if isinstance(obj, collections.abc.Sized): + return str(len(obj)) + return "unknown" + + def _scale_raw_value(raw_value: Any, scale: float | Iterable[float]) -> Any: """Multiply a value by ``scale`` on the way to the instrument.""" if isinstance(scale, collections.abc.Iterable): # Scale contains multiple elements, one for each value - return tuple(val * sub_scale for val, sub_scale in zip(raw_value, scale)) + return _apply_elementwise(raw_value, scale, operator.mul, "scale") if isinstance(raw_value, collections.abc.Sequence): # Multiplying a sequence by a number repeats it rather than # scaling its elements, so these must be handled element wise. @@ -282,7 +323,7 @@ def _offset_raw_value(raw_value: Any, offset: float | Iterable[float]) -> Any: """Add ``offset`` to a value on the way to the instrument.""" if isinstance(offset, collections.abc.Iterable): # offset contains multiple elements, one for each value - return tuple(val + sub_offset for val, sub_offset in zip(raw_value, offset)) + return _apply_elementwise(raw_value, offset, operator.add, "offset") if isinstance(raw_value, collections.abc.Sequence): # Adding a number to a sequence is an error, so these must be # handled element wise. @@ -298,7 +339,7 @@ def _unoffset_value(value: Any, offset: float | Iterable[float]) -> Any: except TypeError: if isinstance(offset, collections.abc.Iterable): # offset contains multiple elements, one for each value - return tuple(val - sub_offset for val, sub_offset in zip(value, offset)) + return _apply_elementwise(value, offset, operator.sub, "offset") elif isinstance(value, collections.abc.Iterable): # Use single offset for all values return tuple(val - offset for val in value) @@ -313,7 +354,7 @@ def _unscale_value(value: Any, scale: float | Iterable[float]) -> Any: except TypeError: if isinstance(scale, collections.abc.Iterable): # Scale contains multiple elements, one for each value - return tuple(val / sub_scale for val, sub_scale in zip(value, scale)) + return _apply_elementwise(value, scale, operator.truediv, "scale") elif isinstance(value, collections.abc.Iterable): # Use single scale for all values return tuple(val / scale for val in value) diff --git a/tests/parameter/test_parameter_scale_offset.py b/tests/parameter/test_parameter_scale_offset.py index 6706b8583b0..7ba9533c668 100644 --- a/tests/parameter/test_parameter_scale_offset.py +++ b/tests/parameter/test_parameter_scale_offset.py @@ -398,3 +398,95 @@ def test_helpers_do_not_mutate_their_input( value = [10.0, 20.0] helper(value, [2.0, 4.0]) assert value == [10.0, 20.0] + + +@pytest.mark.parametrize("attribute", ["scale", "offset"]) +def test_set_raises_on_length_mismatch(attribute: str) -> None: + """A scale/offset that does not match the length of the value is an error.""" + param = Parameter(name="test_param", set_cmd=None, get_cmd=None) + setattr(param, attribute, [2, 4]) + + with pytest.raises( + ValueError, + match=f"Cannot apply {attribute} of length 2 to a value of length 3", + ): + param([10, 20, 30]) + + +@pytest.mark.parametrize("attribute", ["scale", "offset"]) +def test_set_raises_on_length_mismatch_for_numpy_value(attribute: str) -> None: + param = Parameter(name="test_param", set_cmd=None, get_cmd=None) + setattr(param, attribute, [2, 4]) + + with pytest.raises( + ValueError, + match=f"Cannot apply {attribute} of length 2 to a value of length 3", + ): + param(np.array([10, 20, 30])) + + +@pytest.mark.parametrize("attribute", ["scale", "offset"]) +def test_get_raises_on_length_mismatch(attribute: str) -> None: + param = Parameter(name="test_param", set_cmd=None, get_cmd=lambda: [10, 20, 30]) + setattr(param, attribute, [2, 4]) + + with pytest.raises( + ValueError, + match=f"Cannot apply {attribute} of length 2 to a value of length 3", + ): + param.get() + + +@pytest.mark.parametrize("attribute", ["scale", "offset"]) +def test_get_raises_on_length_mismatch_for_numpy_value(attribute: str) -> None: + """Numpy raises its own error before the element wise fallback is reached.""" + param = Parameter( + name="test_param", set_cmd=None, get_cmd=lambda: np.array([10.0, 20.0, 30.0]) + ) + setattr(param, attribute, [2, 4]) + + with pytest.raises(ValueError, match="could not be broadcast together"): + param.get() + + +@pytest.mark.parametrize( + ("helper", "kind"), + [ + (_scale_raw_value, "scale"), + (_offset_raw_value, "offset"), + (_unoffset_value, "offset"), + (_unscale_value, "scale"), + ], +) +def test_helpers_raise_on_length_mismatch( + helper: Callable[[Any, Any], Any], kind: str +) -> None: + with pytest.raises( + ValueError, match=f"Cannot apply {kind} of length 2 to a value of length 3" + ): + helper([10, 20, 30], [2, 4]) + + with pytest.raises( + ValueError, match=f"Cannot apply {kind} of length 3 to a value of length 2" + ): + helper([10, 20], [2, 4, 6]) + + +@pytest.mark.parametrize( + ("helper", "kind"), + [ + (_scale_raw_value, "scale"), + (_offset_raw_value, "offset"), + (_unoffset_value, "offset"), + (_unscale_value, "scale"), + ], +) +def test_helpers_report_unknown_length_for_iterators( + helper: Callable[[Any, Any], Any], kind: str +) -> None: + """An iterable that has no length is reported as unknown.""" + with pytest.raises( + ValueError, + match=f"Cannot apply {kind} of length unknown to a value of length 3", + ): + helper([10, 20, 30], iter([2, 4])) From dc5583fd851985563cc200781aa8ad4726df451b Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Thu, 27 Aug 2026 10:38:51 +0200 Subject: [PATCH 3/4] Merge newsfragments into a single 8451.breaking Both the scalar scale/offset sequence fix and the scale/offset length mismatch ValueError are potentially breaking, so combine them into one breaking newsfragment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4dac2652-68f4-41a7-8e96-7c8130b845f0 --- .../changes/newsfragments/{8451.improved => 8451.breaking} | 7 +++++++ docs/changes/newsfragments/8452.breaking | 5 ----- 2 files changed, 7 insertions(+), 5 deletions(-) rename docs/changes/newsfragments/{8451.improved => 8451.breaking} (57%) delete mode 100644 docs/changes/newsfragments/8452.breaking diff --git a/docs/changes/newsfragments/8451.improved b/docs/changes/newsfragments/8451.breaking similarity index 57% rename from docs/changes/newsfragments/8451.improved rename to docs/changes/newsfragments/8451.breaking index c67ed8db1b9..02be0b926ae 100644 --- a/docs/changes/newsfragments/8451.improved +++ b/docs/changes/newsfragments/8451.breaking @@ -5,3 +5,10 @@ repeats it rather than scaling its elements, so ``param([10, 20])`` with scalar ``offset`` raised a ``TypeError``. Sequences are now converted element wise, matching how the values are converted back when the parameter is read. Numpy arrays are unaffected since they scale and offset element wise already. + +Additionally, applying a ``scale`` or ``offset`` that does not match the length +of the value now raises a ``ValueError``. Previously the value and the +scale/offset were zipped together without checking their lengths, so a mismatch +silently dropped the extra elements, e.g. setting a parameter with +``scale = [2, 4]`` to ``[10, 20, 30]`` used to set the raw value to ``(20, 80)``. +See :pr:`8450`. diff --git a/docs/changes/newsfragments/8452.breaking b/docs/changes/newsfragments/8452.breaking deleted file mode 100644 index 93a0778d194..00000000000 --- a/docs/changes/newsfragments/8452.breaking +++ /dev/null @@ -1,5 +0,0 @@ -Applying a ``scale`` or ``offset`` that does not match the length of the value -now raises a ``ValueError``. Previously the value and the scale/offset were -zipped together without checking their lengths, so a mismatch silently dropped -the extra elements, e.g. setting a parameter with ``scale = [2, 4]`` to -``[10, 20, 30]`` used to set the raw value to ``(20, 80)``. See :pr:`8450`. From 96a44ef64f13ec46459e2a8df7cc9632999c15de Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Thu, 27 Aug 2026 11:38:08 +0200 Subject: [PATCH 4/4] Derive scale/offset error message from the operator Rather than passing an explicit kind argument to _apply_elementwise, map the arithmetic operator to the name used in the error message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08b47f6f-0b50-41ff-842b-ff42bbef4a0f --- src/qcodes/parameters/parameter_base.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index bda25bcf5a7..16d96c8b08e 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -266,11 +266,18 @@ class ParameterBaseKWArgs( # generic ``ParameterDataTypeVar`` out of the arithmetic. +_CONVERSION_KIND: dict[Callable[[Any, Any], Any], str] = { + operator.mul: "scale", + operator.truediv: "scale", + operator.add: "offset", + operator.sub: "offset", +} + + def _apply_elementwise( value: Any, conversion: Iterable[float], operation: Callable[[Any, Any], Any], - kind: str, ) -> tuple[Any, ...]: """Combine ``value`` and ``conversion`` element by element. @@ -278,7 +285,8 @@ def _apply_elementwise( value: The (iterable) value to convert. conversion: The scale or offset to apply, one element per value. operation: The arithmetic operation to apply to each pair of elements. - kind: The name of the conversion, used in the error message. + The operation also determines the name of the conversion used in + the error message. Returns: The converted values. @@ -293,6 +301,7 @@ def _apply_elementwise( for val, sub_value in zip(value, conversion, strict=True) ) except ValueError as err: + kind = _CONVERSION_KIND[operation] raise ValueError( f"Cannot apply {kind} of length {_length_for_error(conversion)} " f"to a value of length {_length_for_error(value)}." @@ -310,7 +319,7 @@ def _scale_raw_value(raw_value: Any, scale: float | Iterable[float]) -> Any: """Multiply a value by ``scale`` on the way to the instrument.""" if isinstance(scale, collections.abc.Iterable): # Scale contains multiple elements, one for each value - return _apply_elementwise(raw_value, scale, operator.mul, "scale") + return _apply_elementwise(raw_value, scale, operator.mul) if isinstance(raw_value, collections.abc.Sequence): # Multiplying a sequence by a number repeats it rather than # scaling its elements, so these must be handled element wise. @@ -323,7 +332,7 @@ def _offset_raw_value(raw_value: Any, offset: float | Iterable[float]) -> Any: """Add ``offset`` to a value on the way to the instrument.""" if isinstance(offset, collections.abc.Iterable): # offset contains multiple elements, one for each value - return _apply_elementwise(raw_value, offset, operator.add, "offset") + return _apply_elementwise(raw_value, offset, operator.add) if isinstance(raw_value, collections.abc.Sequence): # Adding a number to a sequence is an error, so these must be # handled element wise. @@ -339,7 +348,7 @@ def _unoffset_value(value: Any, offset: float | Iterable[float]) -> Any: except TypeError: if isinstance(offset, collections.abc.Iterable): # offset contains multiple elements, one for each value - return _apply_elementwise(value, offset, operator.sub, "offset") + return _apply_elementwise(value, offset, operator.sub) elif isinstance(value, collections.abc.Iterable): # Use single offset for all values return tuple(val - offset for val in value) @@ -354,7 +363,7 @@ def _unscale_value(value: Any, scale: float | Iterable[float]) -> Any: except TypeError: if isinstance(scale, collections.abc.Iterable): # Scale contains multiple elements, one for each value - return _apply_elementwise(value, scale, operator.truediv, "scale") + return _apply_elementwise(value, scale, operator.truediv) elif isinstance(value, collections.abc.Iterable): # Use single scale for all values return tuple(val / scale for val in value)