From 396de8bea9bc76b1b7f5eacf13899ab590f06650 Mon Sep 17 00:00:00 2001 From: Esben Bork Hansen Date: Thu, 23 Jan 2025 13:40:48 +0100 Subject: [PATCH 1/7] extend test_value_validation to cover more cases --- tests/parameter/test_delegate_parameter.py | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/parameter/test_delegate_parameter.py b/tests/parameter/test_delegate_parameter.py index 07b7d093c0f..4e8bdb2580c 100644 --- a/tests/parameter/test_delegate_parameter.py +++ b/tests/parameter/test_delegate_parameter.py @@ -574,18 +574,23 @@ def test_value_validation() -> None: source_param = Parameter("source", set_cmd=None, get_cmd=None) delegate_param = DelegateParameter("delegate", source=source_param) + # Test case where source parameter validator is None and delegate parameter validator is + # specified. delegate_param.vals = vals.Numbers(-10, 10) source_param.vals = None delegate_param.validate(1) with pytest.raises(ValueError): delegate_param.validate(11) + # Test where delegate parameter validator is None and source parameter validator is + # specified. delegate_param.vals = None source_param.vals = vals.Numbers(-5, 5) delegate_param.validate(1) with pytest.raises(ValueError): delegate_param.validate(6) + # Test case where source parameter validator is more restricted than delegate parameter. delegate_param.vals = vals.Numbers(-10, 10) source_param.vals = vals.Numbers(-5, 5) delegate_param.validate(1) @@ -594,6 +599,33 @@ def test_value_validation() -> None: with pytest.raises(ValueError): delegate_param.validate(11) + # Test case that the order of setting validator on source and delegate parameters does not matter. + source_param.vals = vals.Numbers(-5, 5) + delegate_param.vals = vals.Numbers(-10, 10) + delegate_param.validate(1) + with pytest.raises(ValueError): + delegate_param.validate(6) + with pytest.raises(ValueError): + delegate_param.validate(11) + + # Test case where delegate parameter validator is more restricted than source parameter. + delegate_param.vals = vals.Numbers(-5, 5) + source_param.vals = vals.Numbers(-10, 10) + delegate_param.validate(1) + with pytest.raises(ValueError): + delegate_param.validate(6) + with pytest.raises(ValueError): + delegate_param.validate(11) + + # Test case that the order of setting validator on source and delegate parameters does not matter. + source_param.vals = vals.Numbers(-10, 10) + delegate_param.vals = vals.Numbers(-5, 5) + delegate_param.validate(1) + with pytest.raises(ValueError): + delegate_param.validate(6) + with pytest.raises(ValueError): + delegate_param.validate(11) + def test_value_validation_with_offset_and_scale() -> None: source_param = Parameter( From 0000926a2b414712fa1be73081846213e0cc5370 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 27 Jan 2025 14:15:21 +0100 Subject: [PATCH 2/7] Add source validators to delegate parameter validator This will ensure that when the validator is used to find the datatype of a parameter it can be detected correctly --- src/qcodes/parameters/delegate_parameter.py | 17 +++++++++++++++++ src/qcodes/parameters/parameter_base.py | 5 +++-- tests/parameter/test_delegate_parameter.py | 13 +++++++++++++ 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/qcodes/parameters/delegate_parameter.py b/src/qcodes/parameters/delegate_parameter.py index b4c6ecfedf6..f9a4d69f384 100644 --- a/src/qcodes/parameters/delegate_parameter.py +++ b/src/qcodes/parameters/delegate_parameter.py @@ -8,6 +8,8 @@ from collections.abc import Sequence from datetime import datetime + from qcodes.validators.validators import Validator + from .parameter_base import ParamDataType, ParamRawDataType @@ -314,3 +316,18 @@ def validate(self, value: ParamDataType) -> None: super().validate(value) if self.source is not None: self.source.validate(self._from_value_to_raw_value(value)) + + @property + def validators(self) -> tuple[Validator, ...]: + """ + Tuple of all validators associated with the parameter. Note that this + includes validators of the source parameter if source parameter is set + and has any validators. + + :getter: All validators associated with the parameter. + """ + source_validators: tuple[Validator, ...] = ( + self.source.validators if self.source is not None else () + ) + + return source_validators + tuple(self._vals) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index 452896f6fb5..6ce74a53321 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -383,9 +383,10 @@ def vals(self) -> Validator | None: RuntimeError: If removing the first validator when more than one validator is set. """ + validators = self.validators - if len(self._vals): - return self._vals[0] + if len(validators): + return validators[0] else: return None diff --git a/tests/parameter/test_delegate_parameter.py b/tests/parameter/test_delegate_parameter.py index 4e8bdb2580c..4f65eb231be 100644 --- a/tests/parameter/test_delegate_parameter.py +++ b/tests/parameter/test_delegate_parameter.py @@ -627,6 +627,19 @@ def test_value_validation() -> None: delegate_param.validate(11) +def test_validator_delegates_as_expected() -> None: + source_param = Parameter("source", set_cmd=None, get_cmd=None) + delegate_param = DelegateParameter("delegate", source=source_param) + some_validator = vals.Numbers(-10, 10) + source_param.vals = some_validator + delegate_param.vals = None + delegate_param.validate(1) + with pytest.raises(ValueError): + delegate_param.validate(11) + assert delegate_param.validators == (some_validator,) + assert delegate_param.vals == some_validator + + def test_value_validation_with_offset_and_scale() -> None: source_param = Parameter( "source", set_cmd=None, get_cmd=None, vals=vals.Numbers(-5, 5) From d0fbb6cfaddc5e4e0ce94966e67d8ae3a2db52f2 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 10:17:43 +0100 Subject: [PATCH 3/7] Add basic test that delegate parameters are registered with the correct type --- .../test_measurement_context_manager.py | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/dataset/measurement/test_measurement_context_manager.py b/tests/dataset/measurement/test_measurement_context_manager.py index 0a0e0dee211..714ac9c6b25 100644 --- a/tests/dataset/measurement/test_measurement_context_manager.py +++ b/tests/dataset/measurement/test_measurement_context_manager.py @@ -24,8 +24,14 @@ from qcodes.dataset.export_config import DataExportType from qcodes.dataset.measurements import Measurement from qcodes.dataset.sqlite.connection import atomic_transaction -from qcodes.parameters import ManualParameter, Parameter, expand_setpoints_helper +from qcodes.parameters import ( + DelegateParameter, + ManualParameter, + Parameter, + expand_setpoints_helper, +) from qcodes.station import Station +from qcodes.validators import ComplexNumbers from tests.common import retry_until_does_not_throw @@ -201,6 +207,23 @@ def test_register_custom_parameter(DAC) -> None: ) +def test_register_delegate_parameters(): + x_param = Parameter("x", set_cmd=None, get_cmd=None) + + complex_param = Parameter( + "complex_param", get_cmd=None, set_cmd=None, vals=ComplexNumbers() + ) + delegate_param = DelegateParameter("delegate", source=complex_param) + + meas = Measurement() + + meas.register_parameter(x_param) + meas.register_parameter(delegate_param, setpoints=(x_param,)) + assert len(meas.parameters) == 2 + assert meas.parameters["delegate"].type == "complex" + assert meas.parameters["x"].type == "numeric" + + def test_unregister_parameter(DAC, DMM) -> None: """ Test the unregistering of parameters. From a79300cdb694834e6633af5f4b74c71f7798e7be Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 10:29:24 +0100 Subject: [PATCH 4/7] Add test that DelegateParameter correctly can see source param's validator --- src/qcodes/parameters/delegate_parameter.py | 2 +- tests/parameter/test_delegate_parameter.py | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/qcodes/parameters/delegate_parameter.py b/src/qcodes/parameters/delegate_parameter.py index f9a4d69f384..f49d695f10e 100644 --- a/src/qcodes/parameters/delegate_parameter.py +++ b/src/qcodes/parameters/delegate_parameter.py @@ -330,4 +330,4 @@ def validators(self) -> tuple[Validator, ...]: self.source.validators if self.source is not None else () ) - return source_validators + tuple(self._vals) + return tuple(self._vals) + source_validators diff --git a/tests/parameter/test_delegate_parameter.py b/tests/parameter/test_delegate_parameter.py index 4f65eb231be..34fe24f2377 100644 --- a/tests/parameter/test_delegate_parameter.py +++ b/tests/parameter/test_delegate_parameter.py @@ -640,6 +640,26 @@ def test_validator_delegates_as_expected() -> None: assert delegate_param.vals == some_validator +def test_validator_delegates_and_source() -> None: + source_param = Parameter("source", set_cmd=None, get_cmd=None) + delegate_param = DelegateParameter("delegate", source=source_param) + some_validator = vals.Numbers(-10, 10) + some_other_validator = vals.Numbers(-5, 5) + source_param.vals = some_validator + delegate_param.vals = some_other_validator + delegate_param.validate(1) + with pytest.raises(ValueError): + delegate_param.validate(6) + assert delegate_param.validators == (some_other_validator, some_validator) + assert delegate_param.vals == some_other_validator + + assert delegate_param.source is not None + delegate_param.source.vals = None + + assert delegate_param.validators == (some_other_validator,) + assert delegate_param.vals == some_other_validator + + def test_value_validation_with_offset_and_scale() -> None: source_param = Parameter( "source", set_cmd=None, get_cmd=None, vals=vals.Numbers(-5, 5) From b69b34d908a5a54406fd275d8dc996bc7d2a597f Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 16:30:53 +0100 Subject: [PATCH 5/7] Add test that delegate paramere registration works if source is set late --- .../test_measurement_context_manager.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/dataset/measurement/test_measurement_context_manager.py b/tests/dataset/measurement/test_measurement_context_manager.py index 714ac9c6b25..a7ef3ec29fd 100644 --- a/tests/dataset/measurement/test_measurement_context_manager.py +++ b/tests/dataset/measurement/test_measurement_context_manager.py @@ -224,6 +224,26 @@ def test_register_delegate_parameters(): assert meas.parameters["x"].type == "numeric" +def test_register_delegate_parameters_with_late_source(): + x_param = Parameter("x", set_cmd=None, get_cmd=None) + + complex_param = Parameter( + "complex_param", get_cmd=None, set_cmd=None, vals=ComplexNumbers() + ) + delegate_param = DelegateParameter("delegate", source=None) + + meas = Measurement() + + meas.register_parameter(x_param) + + delegate_param.source = complex_param + + meas.register_parameter(delegate_param, setpoints=(x_param,)) + assert len(meas.parameters) == 2 + assert meas.parameters["delegate"].type == "complex" + assert meas.parameters["x"].type == "numeric" + + def test_unregister_parameter(DAC, DMM) -> None: """ Test the unregistering of parameters. From 7ed6be5e46f0374307ccaaf47525dafd9146c2c2 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Wed, 5 Feb 2025 15:01:35 +0100 Subject: [PATCH 6/7] Add changelog for 6585 --- docs/changes/newsfragments/6585.improved | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 docs/changes/newsfragments/6585.improved diff --git a/docs/changes/newsfragments/6585.improved b/docs/changes/newsfragments/6585.improved new file mode 100644 index 00000000000..1b9c5ad5561 --- /dev/null +++ b/docs/changes/newsfragments/6585.improved @@ -0,0 +1,3 @@ +``DelegateParameter`` now includes validators of its source Parameter into its validators. This ensures that a ``DelegateParameter`` +with a non numeric source parameter is registered correctly in a measurement when the ``DelegateParameter`` it self does not +set a validator. From 8154a2dc34bf6bf70886e5b0a93d94420dbedb7c Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Wed, 5 Feb 2025 15:12:51 +0100 Subject: [PATCH 7/7] Add tests with chained delegates --- .../test_measurement_context_manager.py | 26 +++++++++- tests/parameter/test_delegate_parameter.py | 49 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/tests/dataset/measurement/test_measurement_context_manager.py b/tests/dataset/measurement/test_measurement_context_manager.py index a7ef3ec29fd..edd2f80d997 100644 --- a/tests/dataset/measurement/test_measurement_context_manager.py +++ b/tests/dataset/measurement/test_measurement_context_manager.py @@ -207,7 +207,7 @@ def test_register_custom_parameter(DAC) -> None: ) -def test_register_delegate_parameters(): +def test_register_delegate_parameters() -> None: x_param = Parameter("x", set_cmd=None, get_cmd=None) complex_param = Parameter( @@ -224,7 +224,7 @@ def test_register_delegate_parameters(): assert meas.parameters["x"].type == "numeric" -def test_register_delegate_parameters_with_late_source(): +def test_register_delegate_parameters_with_late_source() -> None: x_param = Parameter("x", set_cmd=None, get_cmd=None) complex_param = Parameter( @@ -244,6 +244,28 @@ def test_register_delegate_parameters_with_late_source(): assert meas.parameters["x"].type == "numeric" +def test_register_delegate_parameters_with_late_source_chain(): + x_param = Parameter("x", set_cmd=None, get_cmd=None) + + complex_param = Parameter( + "complex_param", get_cmd=None, set_cmd=None, vals=ComplexNumbers() + ) + delegate_inner = DelegateParameter("delegate_inner", source=None) + delegate_outer = DelegateParameter("delegate_outer", source=None) + + meas = Measurement() + + meas.register_parameter(x_param) + + delegate_outer.source = delegate_inner + delegate_inner.source = complex_param + + meas.register_parameter(delegate_outer, setpoints=(x_param,)) + assert len(meas.parameters) == 2 + assert meas.parameters["delegate_outer"].type == "complex" + assert meas.parameters["x"].type == "numeric" + + def test_unregister_parameter(DAC, DMM) -> None: """ Test the unregistering of parameters. diff --git a/tests/parameter/test_delegate_parameter.py b/tests/parameter/test_delegate_parameter.py index 34fe24f2377..a232154fdeb 100644 --- a/tests/parameter/test_delegate_parameter.py +++ b/tests/parameter/test_delegate_parameter.py @@ -660,6 +660,55 @@ def test_validator_delegates_and_source() -> None: assert delegate_param.vals == some_other_validator +def test_validator_delegates_and_source_chain() -> None: + source_param = Parameter("source", set_cmd=None, get_cmd=None) + delegate_inner = DelegateParameter("delegate_inner", source=source_param) + delegate_outer = DelegateParameter("delegate_outer", source=delegate_inner) + source_validator = vals.Numbers(-10, 10) + delegate_inner_validator = vals.Numbers(-7, 7) + delegate_outer_validator = vals.Numbers(-5, 5) + + source_param.vals = source_validator + delegate_inner.vals = delegate_inner_validator + delegate_outer.vals = delegate_outer_validator + + delegate_outer.validate(1) + with pytest.raises(ValueError): + delegate_outer.validate(6) + + delegate_inner.validate(6) + source_param.validate(6) + + assert delegate_outer.validators == ( + delegate_outer_validator, + delegate_inner_validator, + source_validator, + ) + assert delegate_outer.vals == delegate_outer_validator + + assert delegate_inner.validators == ( + delegate_inner_validator, + source_validator, + ) + assert delegate_inner.vals == delegate_inner_validator + + assert delegate_outer.source is not None + delegate_outer.source.vals = None + + assert delegate_outer.validators == ( + delegate_outer_validator, + source_validator, + ) + assert delegate_outer.vals == delegate_outer_validator + + assert isinstance(delegate_outer.source, DelegateParameter) + assert delegate_outer.source.source is not None + delegate_outer.source.source.vals = None + + assert delegate_outer.validators == (delegate_outer_validator,) + assert delegate_outer.vals == delegate_outer_validator + + def test_value_validation_with_offset_and_scale() -> None: source_param = Parameter( "source", set_cmd=None, get_cmd=None, vals=vals.Numbers(-5, 5)