From 568b0be1ab910d514b2961fa3c67abdf87b713aa Mon Sep 17 00:00:00 2001 From: Esben Bork Hansen Date: Thu, 23 Jan 2025 13:40:48 +0100 Subject: [PATCH 01/12] 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 07b7d093c0f2..4e8bdb2580c5 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 87a0e79205b562f8d563ff732679fa6befe535e5 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 27 Jan 2025 14:15:21 +0100 Subject: [PATCH 02/12] 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 b4c6ecfedf68..f9a4d69f3847 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 452896f6fb50..6ce74a533218 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 4e8bdb2580c5..4f65eb231bec 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 4ed8c91a5446277b56bc6ec71575b7adede2699b Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 09:57:00 +0100 Subject: [PATCH 03/12] Add root_source to DelegateParameter --- src/qcodes/parameters/delegate_parameter.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/qcodes/parameters/delegate_parameter.py b/src/qcodes/parameters/delegate_parameter.py index f9a4d69f3847..d39de4289b7d 100644 --- a/src/qcodes/parameters/delegate_parameter.py +++ b/src/qcodes/parameters/delegate_parameter.py @@ -202,6 +202,23 @@ def source(self) -> Parameter | None: def source(self, source: Parameter | None) -> None: self._source: Parameter | None = source + @property + def root_source(self) -> Parameter | None: + """ + The root source parameter that this :class:`DelegateParameter` is bound to + or ``None`` if this :class:`DelegateParameter` is unbound. If + the source is it self a DelegateParameter it will recursively return that Parameter's + source until a non DelegateParameter is found. For a non DelegateParameter source + this behaves the same as ``self.source`` + + :getter: Returns the current source. + :setter: Sets the source. + """ + if isinstance(self.source, DelegateParameter): + return self.source.root_source + else: + return self.source + @property def snapshot_value(self) -> bool: if self.source is None: From 972d1c3035818997a24aed4e2c9b2c7785c9d940 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 10:17:43 +0100 Subject: [PATCH 04/12] 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 0a0e0dee2118..714ac9c6b25e 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 17794cd89d07bc4b6495c267b524366252ee4f2d Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 10:29:24 +0100 Subject: [PATCH 05/12] 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 d39de4289b7d..9b647e1e571d 100644 --- a/src/qcodes/parameters/delegate_parameter.py +++ b/src/qcodes/parameters/delegate_parameter.py @@ -347,4 +347,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 4f65eb231bec..34fe24f23773 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 40c5dd6b75f9606a0c4c0c7fb8611eb5d204f780 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 10:35:43 +0100 Subject: [PATCH 06/12] Add test for DelegateParamerer.root_source --- tests/parameter/test_delegate_parameter.py | 24 +++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/parameter/test_delegate_parameter.py b/tests/parameter/test_delegate_parameter.py index 34fe24f23773..d167070d05aa 100644 --- a/tests/parameter/test_delegate_parameter.py +++ b/tests/parameter/test_delegate_parameter.py @@ -704,7 +704,7 @@ def test_value_validation_with_offset_and_scale() -> None: delegate_param.set(1) -def test_delegate_of_delegate_updates_settable_gettable(): +def test_delegate_of_delegate_updates_settable_gettable() -> None: gettable_settable_source_param = Parameter( "source", set_cmd=None, get_cmd=None, vals=vals.Numbers(-5, 5) ) @@ -737,6 +737,28 @@ def test_delegate_of_delegate_updates_settable_gettable(): assert not delegate_param_outer.settable +def test_delegate_of_delegate_root_source() -> None: + gettable_settable_source_param = Parameter( + "source", set_cmd=None, get_cmd=None, vals=vals.Numbers(-5, 5) + ) + + delegate_param_inner = DelegateParameter( + "delegate_inner", source=None, vals=vals.Numbers(-10, 10) + ) + delegate_param_outer = DelegateParameter( + "delegate_outer", source=None, vals=vals.Numbers(-10, 10) + ) + delegate_param_outer.source = delegate_param_inner + delegate_param_inner.source = gettable_settable_source_param + + assert delegate_param_outer.root_source == gettable_settable_source_param + assert delegate_param_outer.source is not None + assert delegate_param_outer.source.source == gettable_settable_source_param + + assert delegate_param_inner.root_source == gettable_settable_source_param + assert delegate_param_inner.source == gettable_settable_source_param + + def test_delegate_parameter_context() -> None: gettable_settable_source_param = Parameter( "source", set_cmd=None, get_cmd=None, vals=vals.Numbers(-5, 5) From 0c3d2e2e0aa5a98561eb7a31bea96932450a81f8 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 10:58:35 +0100 Subject: [PATCH 07/12] Add changelog for 6832 --- docs/changes/newsfragments/6832.improved | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 docs/changes/newsfragments/6832.improved diff --git a/docs/changes/newsfragments/6832.improved b/docs/changes/newsfragments/6832.improved new file mode 100644 index 000000000000..33c25b73eebe --- /dev/null +++ b/docs/changes/newsfragments/6832.improved @@ -0,0 +1,4 @@ +``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. Furthermore `DelegateParameter`` has gained a ``root_source`` attribute that makes it easier to get the +root source of a ``DelegateParameter`` that delegates to another ``DelegateParameter``. From af263147567085e779a7903c06770d474f1ad471 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 13:32:36 +0100 Subject: [PATCH 08/12] Add option to set root_source to DelegateParameter --- src/qcodes/parameters/delegate_parameter.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/qcodes/parameters/delegate_parameter.py b/src/qcodes/parameters/delegate_parameter.py index 9b647e1e571d..9fa92a4d1902 100644 --- a/src/qcodes/parameters/delegate_parameter.py +++ b/src/qcodes/parameters/delegate_parameter.py @@ -212,13 +212,29 @@ def root_source(self) -> Parameter | None: this behaves the same as ``self.source`` :getter: Returns the current source. - :setter: Sets the source. + :setter: Sets the source of the first parameter in the tree that has a None source or non-DelegateParameter source """ if isinstance(self.source, DelegateParameter): return self.source.root_source else: return self.source + @root_source.setter + def root_source(self, source: Parameter | None) -> None: + self.root_delegate.source = source + + @property + def root_delegate(self) -> DelegateParameter: + """ + If this parameter is part of a chain of DelegateParameters return + the first Parameter in the chain that has a non DelegateParameter source + """ + + if not isinstance(self.source, DelegateParameter): + return self + else: + return self.source.root_delegate + @property def snapshot_value(self) -> bool: if self.source is None: From d044bcbd79dc57ae8fb8cd8721344d51ca489c43 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 14:08:03 +0100 Subject: [PATCH 09/12] Add tests for root_delegate and root_source setter --- tests/parameter/test_delegate_parameter.py | 65 ++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/parameter/test_delegate_parameter.py b/tests/parameter/test_delegate_parameter.py index d167070d05aa..24ef7dc41a70 100644 --- a/tests/parameter/test_delegate_parameter.py +++ b/tests/parameter/test_delegate_parameter.py @@ -754,9 +754,74 @@ def test_delegate_of_delegate_root_source() -> None: assert delegate_param_outer.root_source == gettable_settable_source_param assert delegate_param_outer.source is not None assert delegate_param_outer.source.source == gettable_settable_source_param + assert delegate_param_outer.root_delegate == delegate_param_inner assert delegate_param_inner.root_source == gettable_settable_source_param assert delegate_param_inner.source == gettable_settable_source_param + assert delegate_param_inner.root_delegate == delegate_param_inner + + delegate_param_outer.root_source = None + + assert delegate_param_outer.root_source is None + assert delegate_param_outer.source is not None + assert delegate_param_outer.source.source is None + assert delegate_param_outer.root_delegate == delegate_param_inner + + assert delegate_param_inner.root_source is None + assert delegate_param_inner.source is None + assert delegate_param_inner.root_delegate == delegate_param_inner + + +def test_delegate_chain_root_source() -> None: + gettable_settable_source_param = Parameter( + "source", set_cmd=None, get_cmd=None, vals=vals.Numbers(-5, 5) + ) + + delegate_param_inner = DelegateParameter( + "delegate_inner", source=None, vals=vals.Numbers(-10, 10) + ) + delegate_param_middle = DelegateParameter( + "delegate_inner", source=None, vals=vals.Numbers(-10, 10) + ) + delegate_param_outer = DelegateParameter( + "delegate_outer", source=None, vals=vals.Numbers(-10, 10) + ) + delegate_param_outer.source = delegate_param_middle + delegate_param_middle.source = delegate_param_inner + delegate_param_inner.source = gettable_settable_source_param + + assert delegate_param_outer.root_source == gettable_settable_source_param + assert delegate_param_outer.source is not None + assert delegate_param_outer.source.source == delegate_param_inner + assert isinstance(delegate_param_outer.source.source, DelegateParameter) + assert delegate_param_outer.source.source.source == gettable_settable_source_param + assert delegate_param_outer.root_delegate == delegate_param_inner + + assert delegate_param_middle.root_source == gettable_settable_source_param + assert delegate_param_middle.source == delegate_param_inner + assert delegate_param_middle.source.source == gettable_settable_source_param + assert delegate_param_middle.root_delegate == delegate_param_inner + + assert delegate_param_inner.root_source == gettable_settable_source_param + assert delegate_param_inner.source == gettable_settable_source_param + assert delegate_param_inner.root_delegate == delegate_param_inner + + delegate_param_outer.root_source = None + + assert delegate_param_outer.root_source is None + assert delegate_param_outer.source is not None + assert delegate_param_outer.source.source is not None + assert delegate_param_outer.source.source.source is None + assert delegate_param_outer.root_delegate == delegate_param_inner + + assert delegate_param_middle.root_source is None + assert delegate_param_middle.source is not None + assert delegate_param_middle.source.source is None + assert delegate_param_outer.root_delegate == delegate_param_inner + + assert delegate_param_inner.root_source is None + assert delegate_param_inner.source is None + assert delegate_param_inner.root_delegate == delegate_param_inner def test_delegate_parameter_context() -> None: From fb665d5c6c0a219b2d9c8c7d8e4231dd3e9d4080 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 14:10:33 +0100 Subject: [PATCH 10/12] Extend changelog --- docs/changes/newsfragments/6832.improved | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changes/newsfragments/6832.improved b/docs/changes/newsfragments/6832.improved index 33c25b73eebe..afe6f5d15678 100644 --- a/docs/changes/newsfragments/6832.improved +++ b/docs/changes/newsfragments/6832.improved @@ -1,4 +1,4 @@ ``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. Furthermore `DelegateParameter`` has gained a ``root_source`` attribute that makes it easier to get the -root source of a ``DelegateParameter`` that delegates to another ``DelegateParameter``. +set a validator. Furthermore `DelegateParameter`` has gained ``root_source`` and ``root_delegate`` attributes that makes it easier to get the +root source or ``DelegateParameter`` of a ``DelegateParameter`` that delegates to another ``DelegateParameter`` in a chain. From 605c7dbd31aee6e0c59014dc2219f4c82785e2f5 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 14:12:03 +0100 Subject: [PATCH 11/12] Improve docstring --- src/qcodes/parameters/delegate_parameter.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qcodes/parameters/delegate_parameter.py b/src/qcodes/parameters/delegate_parameter.py index 9fa92a4d1902..f1647ca7ea87 100644 --- a/src/qcodes/parameters/delegate_parameter.py +++ b/src/qcodes/parameters/delegate_parameter.py @@ -212,7 +212,7 @@ def root_source(self) -> Parameter | None: this behaves the same as ``self.source`` :getter: Returns the current source. - :setter: Sets the source of the first parameter in the tree that has a None source or non-DelegateParameter source + :setter: Sets the source of the first parameter in the tree that has a non-DelegateParameter source. """ if isinstance(self.source, DelegateParameter): return self.source.root_source @@ -228,6 +228,7 @@ def root_delegate(self) -> DelegateParameter: """ If this parameter is part of a chain of DelegateParameters return the first Parameter in the chain that has a non DelegateParameter source + else return self. """ if not isinstance(self.source, DelegateParameter): From d3d58bfa22e0a3dac7c20d7ab00965c1d4560204 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Mon, 3 Feb 2025 16:30:53 +0100 Subject: [PATCH 12/12] 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 714ac9c6b25e..a7ef3ec29fdc 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.