From 7cbfa39c2f5542abfe05d3ae6496a28729bc9758 Mon Sep 17 00:00:00 2001 From: mahmut-oqs Date: Fri, 14 Feb 2025 17:16:13 +0100 Subject: [PATCH 01/15] Update parameter_base.py --- src/qcodes/parameters/parameter_base.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index 6ce74a533218..fd1d14578f0c 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -191,6 +191,7 @@ class ParameterBase(MetadatableWithName): using a different name than the parameter's full_name """ + _database_callback: Callable[[ParameterBase, Any], None] | None = None def __init__( self, @@ -778,6 +779,14 @@ def set_wrapper(value: ParamDataType, **kwargs: Any) -> None: self.cache._update_with(value=val_step, raw_value=raw_val_step) + if ParameterBase._database_callback is not None: + try: + ParameterBase._database_callback(self, value) + except Exception as e: + LOG.exception( + f"Exception while running parameter callback: {e}" + ) + except Exception as e: e.args = (*e.args, f"setting {self} to {value}") raise e From a8265a93b3c3630d362c6fe267130f1beb50ec8a Mon Sep 17 00:00:00 2001 From: mahmut-oqs Date: Fri, 14 Feb 2025 17:24:23 +0100 Subject: [PATCH 02/15] Create parameter_callback test file --- tests/parameter/parameter_callback | 344 +++++++++++++++++++++++++++++ 1 file changed, 344 insertions(+) create mode 100644 tests/parameter/parameter_callback diff --git a/tests/parameter/parameter_callback b/tests/parameter/parameter_callback new file mode 100644 index 000000000000..9ad6b5d30261 --- /dev/null +++ b/tests/parameter/parameter_callback @@ -0,0 +1,344 @@ +import logging +import time +from collections.abc import Callable +from contextlib import contextmanager +from dataclasses import dataclass +from typing import Optional + +import pytest + +from qcodes.instrument_drivers.mock_instruments import DummyInstrument +from qcodes.parameters import Parameter, ParameterBase +from qcodes.utils.validators import Ints, Numbers + +CallbackFunction = Callable[[ParameterBase, float], None] + + +@dataclass(frozen=True) +class _Constants: + """Test constants centralized in a dataclass for better maintainability.""" + + VALUE_DEFAULT: int = 42 + VALUE_INSTRUMENT: int = 23 + PERFORMANCE_ITERATIONS: int = 1000 + PERFORMANCE_MAX_OVERHEAD: float = 2.0 + + +class _TestDataManager: + """Class to manage test data and state.""" + + def __init__(self) -> None: + self._latest_value: float = 0 + self.callback_called: bool = False + self.callback_count: int = 0 + + @property + def latest_value(self) -> float: + return self._latest_value + + @latest_value.setter + def latest_value(self, value: float) -> None: + self._latest_value = value + + def reset(self) -> None: + """Reset test data state.""" + self._latest_value = 0 + self.callback_called = False + self.callback_count = 0 + + +class ParameterTestHelper: + """Helper class for parameter-related test operations.""" + + def __init__(self) -> None: + self.test_data = _TestDataManager() + + def create_parameter(self) -> Parameter: + """Create a parameter with dummy get/set commands.""" + return Parameter( + name="test_param", + instrument=None, + set_cmd=self._dummy_set, + get_cmd=self._dummy_get, + ) + + def _dummy_set(self, value: float) -> None: + self.test_data.latest_value = value + + def _dummy_get(self) -> float: + return self.test_data.latest_value + + @contextmanager + def parameter_context(self) -> Parameter: + """Context manager for parameter cleanup.""" + param = self.create_parameter() + try: + yield param + finally: + ParameterBase._database_callback = None + self.test_data.reset() + + +class TestParameterCallbacks: + """Test suite for parameter database callbacks.""" + + CONSTANTS = _Constants() + + @pytest.fixture + def helper(self) -> ParameterTestHelper: + """Fixture providing a test helper instance.""" + return ParameterTestHelper() + + @pytest.fixture + def clean_parameter(self, helper: ParameterTestHelper) -> Parameter: + """Fixture providing a clean parameter instance.""" + with helper.parameter_context() as param: + yield param + + @pytest.fixture + def dummy_instrument(self, helper: ParameterTestHelper) -> DummyInstrument: + """Fixture providing a dummy instrument instance.""" + instrument = DummyInstrument("dummy") + instrument.add_parameter( + "test_param", + parameter_class=Parameter, + set_cmd=helper._dummy_set, + get_cmd=helper._dummy_get, + ) + try: + yield instrument + finally: + instrument.close() + + def create_test_callback( + self, test_data: _TestDataManager, expected_value: Optional[float] = None + ) -> CallbackFunction: + """Create a test callback function with validation.""" + + def callback(param: ParameterBase, value: float) -> None: + test_data.callback_called = True + test_data.callback_count += 1 + assert param.name == "test_param", ( + f"Expected parameter name 'test_param', got {param.name}" + ) + if expected_value is not None: + assert value == expected_value, ( + f"Expected value {expected_value}, got {value}" + ) + + return callback + + @pytest.mark.parametrize("test_value", [0, CONSTANTS.VALUE_DEFAULT, -1, 999]) + def test_callback_executes_with_correct_parameters( + self, clean_parameter: Parameter, helper: ParameterTestHelper, test_value: float + ) -> None: + """Test callback execution with various parameter values.""" + ParameterBase._database_callback = self.create_test_callback( + helper.test_data, test_value + ) + clean_parameter(test_value) + assert helper.test_data.callback_called, "Callback was not executed" + + def test_callback_with_instrument( + self, dummy_instrument: DummyInstrument, helper: ParameterTestHelper + ) -> None: + """Test callback functionality with instrument parameters.""" + ParameterBase._database_callback = self.create_test_callback( + helper.test_data, + self.CONSTANTS.VALUE_INSTRUMENT, + ) + param = dummy_instrument.parameters["test_param"] + param(self.CONSTANTS.VALUE_INSTRUMENT) + assert helper.test_data.callback_called, ( + "Instrument parameter callback was not executed" + ) + + def test_callback_error_handling( + self, clean_parameter: Parameter, caplog: pytest.LogCaptureFixture + ) -> None: + """Test error handling in callback execution.""" + error_message = "Test error" + + def failing_callback(param: ParameterBase, value: float) -> None: + raise RuntimeError(error_message) + + ParameterBase._database_callback = failing_callback + + with caplog.at_level(logging.ERROR): + clean_parameter(self.CONSTANTS.VALUE_DEFAULT) + assert "Exception while running parameter callback" in caplog.text + assert error_message in caplog.text + + def test_callback_performance( + self, clean_parameter: Parameter, helper: ParameterTestHelper + ) -> None: + """Test callback performance overhead.""" + + def measure_execution_time( + callback: Optional[CallbackFunction] = None, + ) -> float: + if callback: + ParameterBase._database_callback = callback + start_time = time.perf_counter() + for _ in range(self.CONSTANTS.PERFORMANCE_ITERATIONS): + clean_parameter(self.CONSTANTS.VALUE_DEFAULT) + return time.perf_counter() - start_time + + base_time = measure_execution_time() + assert base_time > 0, "Invalid negative base time measurement" + + callback_time = measure_execution_time( + self.create_test_callback(helper.test_data) + ) + assert callback_time > 0, "Invalid negative callback time measurement" + assert callback_time < base_time * self.CONSTANTS.PERFORMANCE_MAX_OVERHEAD, ( + f"Callback overhead too high: {callback_time:.2f}s vs {base_time:.2f}s base time" + ) + + def test_callback_thread_safety( + self, clean_parameter: Parameter, helper: ParameterTestHelper + ) -> None: + """Test thread safety of callback execution.""" + ParameterBase._database_callback = self.create_test_callback(helper.test_data) + clean_parameter(self.CONSTANTS.VALUE_DEFAULT) + assert helper.test_data.callback_count == 1, ( + f"Expected exactly one callback execution, got {helper.test_data.callback_count}" + ) + + def test_callback_none(self, clean_parameter: Parameter) -> None: + """Test parameter behavior without callback.""" + ParameterBase._database_callback = None + clean_parameter(self.CONSTANTS.VALUE_DEFAULT) + assert clean_parameter() == self.CONSTANTS.VALUE_DEFAULT + + def test_callback_independence(self, clean_parameter: Parameter) -> None: + """Test callback isolation between tests.""" + assert ParameterBase._database_callback is None, ( + "Database callback not properly reset from previous test" + ) + + def test_callback_multiple_parameters(self, helper: ParameterTestHelper) -> None: + """Test callback behavior with multiple parameters.""" + param1 = helper.create_parameter() + param2 = helper.create_parameter() + + ParameterBase._database_callback = self.create_test_callback(helper.test_data) + + param1(self.CONSTANTS.VALUE_DEFAULT) + param2(self.CONSTANTS.VALUE_DEFAULT) + + assert helper.test_data.callback_count == 2, ( + "Callback should be called for each parameter" + ) + + def test_callback_value_persistence(self, clean_parameter: Parameter) -> None: + """Test that callback doesn't interfere with parameter value persistence.""" + test_value = self.CONSTANTS.VALUE_DEFAULT + + def callback(param: ParameterBase, value: float) -> None: + value += 1 + + ParameterBase._database_callback = callback + clean_parameter(test_value) + + assert clean_parameter() == test_value, ( + "Parameter value should remain unchanged" + ) + + def test_callback_with_different_parameter_types( + self, helper: ParameterTestHelper + ) -> None: + """Test callback behavior with different parameter types.""" + int_param = Parameter( + name="test_param_int", + set_cmd=None, + get_cmd=None, + vals=Ints(), + ) + float_param = Parameter( + name="test_param_float", + set_cmd=None, + get_cmd=None, + vals=Numbers(), + ) + + callback_values = [] + + def type_checking_callback(param: ParameterBase, value: float) -> None: + callback_values.append((param.name, type(value))) + + ParameterBase._database_callback = type_checking_callback + + int_param(42) + float_param(42.5) + + assert len(callback_values) == 2, "Both callbacks should execute" + assert callback_values[0][1] is int, "Integer parameter should pass int" + assert callback_values[1][1] is float, "Float parameter should pass float" + + def test_callback_concurrent_updates(self, helper: ParameterTestHelper) -> None: + """Test callback behavior with rapid parameter updates.""" + from concurrent.futures import ThreadPoolExecutor + + param = helper.create_parameter() + update_count = 100 + + def update_parameter(value: float) -> None: + param(value) + + ParameterBase._database_callback = self.create_test_callback(helper.test_data) + + with ThreadPoolExecutor(max_workers=4) as executor: + futures = [ + executor.submit(update_parameter, i) for i in range(update_count) + ] + for future in futures: + future.result() + + assert helper.test_data.callback_count == update_count, ( + f"Expected {update_count} callbacks, got {helper.test_data.callback_count}" + ) + + def test_callback_chaining(self, helper: ParameterTestHelper) -> None: + """Test multiple callbacks can be chained together.""" + param = helper.create_parameter() + callback_order = [] + + def callback1(param: ParameterBase, value: float) -> None: + callback_order.append(1) + callback2(param, value) + + def callback2(param: ParameterBase, value: float) -> None: + callback_order.append(2) + + ParameterBase._database_callback = callback1 + param(self.CONSTANTS.VALUE_DEFAULT) + + assert callback_order == [1, 2], "Callbacks should execute in order" + + def test_callback_memory_cleanup(self, helper: ParameterTestHelper) -> None: + """Test that callbacks don't cause memory leaks.""" + import gc + import weakref + + param = helper.create_parameter() + + def create_callback() -> CallbackFunction: + local_data = [0] + + def callback(param: ParameterBase, value: float) -> None: + local_data[0] = value + + return callback + + callback = create_callback() + ref = weakref.ref(callback) + ParameterBase._database_callback = callback + + param(self.CONSTANTS.VALUE_DEFAULT) + + ParameterBase._database_callback = None + del callback + gc.collect() + + assert ref() is None, "Callback should be garbage collected" From 6841673025fc9d95509f54a5a6fc9ff7b409510c Mon Sep 17 00:00:00 2001 From: mahmut-oqs Date: Tue, 25 Feb 2025 08:31:05 +0100 Subject: [PATCH 03/15] Update parameter_base.py --- src/qcodes/parameters/parameter_base.py | 66 +++++++++++++++++-------- 1 file changed, 45 insertions(+), 21 deletions(-) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index fd1d14578f0c..c7a3533877d4 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -13,7 +13,6 @@ from qcodes.utils import DelegateAttributes, full_class, qcodes_abstractmethod from qcodes.validators import Enum, Ints, Validator -from ..utils.types import NumberType from .cache import _Cache, _CacheProtocol from .named_repr import named_repr from .permissive_range import permissive_range @@ -191,7 +190,6 @@ class ParameterBase(MetadatableWithName): using a different name than the parameter's full_name """ - _database_callback: Callable[[ParameterBase, Any], None] | None = None def __init__( self, @@ -214,6 +212,7 @@ def __init__( abstract: bool | None = False, bind_to_instrument: bool = True, register_name: str | None = None, + value_changed_callback: Callable[[ParameterBase, Any], None] | None = None, ) -> None: super().__init__(metadata) if not str(name).isidentifier(): @@ -344,6 +343,8 @@ def __init__( instrument.parameters[name] = self + self._value_changed_callback = value_changed_callback + @property def _implements_get_raw(self) -> bool: implements_get_raw = hasattr(self, "get_raw") and not getattr( @@ -384,10 +385,9 @@ def vals(self) -> Validator | None: RuntimeError: If removing the first validator when more than one validator is set. """ - validators = self.validators - if len(validators): - return validators[0] + if len(self._vals): + return self._vals[0] else: return None @@ -779,13 +779,7 @@ def set_wrapper(value: ParamDataType, **kwargs: Any) -> None: self.cache._update_with(value=val_step, raw_value=raw_val_step) - if ParameterBase._database_callback is not None: - try: - ParameterBase._database_callback(self, value) - except Exception as e: - LOG.exception( - f"Exception while running parameter callback: {e}" - ) + self._invoke_callback(value) except Exception as e: e.args = (*e.args, f"setting {self} to {value}") @@ -794,8 +788,8 @@ def set_wrapper(value: ParamDataType, **kwargs: Any) -> None: return set_wrapper def get_ramp_values( - self, value: NumberType | Sized, step: NumberType | None = None - ) -> Sequence[NumberType | Sized]: + self, value: float | Sized, step: float | None = None + ) -> Sequence[float | Sized]: """ Return values to sweep from current value to target value. This method can be overridden to have a custom sweep behaviour. @@ -820,7 +814,8 @@ def get_ramp_values( self.get() start_value = self.get_latest() if not ( - isinstance(start_value, NumberType) and isinstance(value, NumberType) + isinstance(start_value, (int, float)) + and isinstance(value, (int, float)) ): # parameter is numeric but either one of the endpoints # is not or the starting point is unknown. The later @@ -869,7 +864,7 @@ def validate(self, value: ParamDataType) -> None: validator.validate(value, self._validate_context) @property - def step(self) -> NumberType | None: + def step(self) -> float | None: """ Stepsize that this Parameter uses during set operation. Stepsize must be a positive number or None. @@ -893,12 +888,12 @@ def step(self) -> NumberType | None: return self._step @step.setter - def step(self, step: NumberType | None) -> None: + def step(self, step: float | None) -> None: if step is None: - self._step: NumberType | None = step + self._step: float | None = step elif not all(getattr(vals, "is_numeric", True) for vals in self._vals): raise TypeError("you can only step numeric parameters") - elif not isinstance(step, NumberType): + elif not isinstance(step, (int, float)): raise TypeError("step must be a number") elif step == 0: self._step = None @@ -938,7 +933,7 @@ def post_delay(self) -> float: @post_delay.setter def post_delay(self, post_delay: float) -> None: - if not isinstance(post_delay, NumberType): + if not isinstance(post_delay, (int, float)): raise TypeError(f"post_delay ({post_delay}) must be a number") if post_delay < 0: raise ValueError(f"post_delay ({post_delay}) must not be negative") @@ -967,7 +962,7 @@ def inter_delay(self) -> float: @inter_delay.setter def inter_delay(self, inter_delay: float) -> None: - if not isinstance(inter_delay, NumberType): + if not isinstance(inter_delay, (int, float)): raise TypeError(f"inter_delay ({inter_delay}) must be a number") if inter_delay < 0: raise ValueError(f"inter_delay ({inter_delay}) must not be negative") @@ -1132,6 +1127,35 @@ def underlying_instrument(self) -> InstrumentBase | None: def abstract(self) -> bool | None: return self._abstract + def _invoke_callback(self, value: Any) -> None: + """ + Invoke the instance-specific callback if it exists. + + Args: + value: The new parameter value + + """ + try: + if self._value_changed_callback is not None: + self._value_changed_callback(self, value) + except Exception as e: + LOG.exception(f"Exception while running parameter callback: {e}") + + def set_value_changed_callback( + self, callback: Callable[[ParameterBase, Any], None] | None + ) -> None: + """ + Set the callback for this specific parameter instance. + + Args: + callback: The callback function to be called when the parameter + value changes, or None to remove the callback. + + """ + if callback is not None and not callable(callback): + raise TypeError("Callback must be type callable or None") + self._value_changed_callback = callback + class GetLatest(DelegateAttributes): """ From 6bb5da82b0c51b939fcded5681832768f7f9adf3 Mon Sep 17 00:00:00 2001 From: mahmut-oqs Date: Tue, 25 Feb 2025 08:32:00 +0100 Subject: [PATCH 04/15] Delete tests/parameter/parameter_callback --- tests/parameter/parameter_callback | 344 ----------------------------- 1 file changed, 344 deletions(-) delete mode 100644 tests/parameter/parameter_callback diff --git a/tests/parameter/parameter_callback b/tests/parameter/parameter_callback deleted file mode 100644 index 9ad6b5d30261..000000000000 --- a/tests/parameter/parameter_callback +++ /dev/null @@ -1,344 +0,0 @@ -import logging -import time -from collections.abc import Callable -from contextlib import contextmanager -from dataclasses import dataclass -from typing import Optional - -import pytest - -from qcodes.instrument_drivers.mock_instruments import DummyInstrument -from qcodes.parameters import Parameter, ParameterBase -from qcodes.utils.validators import Ints, Numbers - -CallbackFunction = Callable[[ParameterBase, float], None] - - -@dataclass(frozen=True) -class _Constants: - """Test constants centralized in a dataclass for better maintainability.""" - - VALUE_DEFAULT: int = 42 - VALUE_INSTRUMENT: int = 23 - PERFORMANCE_ITERATIONS: int = 1000 - PERFORMANCE_MAX_OVERHEAD: float = 2.0 - - -class _TestDataManager: - """Class to manage test data and state.""" - - def __init__(self) -> None: - self._latest_value: float = 0 - self.callback_called: bool = False - self.callback_count: int = 0 - - @property - def latest_value(self) -> float: - return self._latest_value - - @latest_value.setter - def latest_value(self, value: float) -> None: - self._latest_value = value - - def reset(self) -> None: - """Reset test data state.""" - self._latest_value = 0 - self.callback_called = False - self.callback_count = 0 - - -class ParameterTestHelper: - """Helper class for parameter-related test operations.""" - - def __init__(self) -> None: - self.test_data = _TestDataManager() - - def create_parameter(self) -> Parameter: - """Create a parameter with dummy get/set commands.""" - return Parameter( - name="test_param", - instrument=None, - set_cmd=self._dummy_set, - get_cmd=self._dummy_get, - ) - - def _dummy_set(self, value: float) -> None: - self.test_data.latest_value = value - - def _dummy_get(self) -> float: - return self.test_data.latest_value - - @contextmanager - def parameter_context(self) -> Parameter: - """Context manager for parameter cleanup.""" - param = self.create_parameter() - try: - yield param - finally: - ParameterBase._database_callback = None - self.test_data.reset() - - -class TestParameterCallbacks: - """Test suite for parameter database callbacks.""" - - CONSTANTS = _Constants() - - @pytest.fixture - def helper(self) -> ParameterTestHelper: - """Fixture providing a test helper instance.""" - return ParameterTestHelper() - - @pytest.fixture - def clean_parameter(self, helper: ParameterTestHelper) -> Parameter: - """Fixture providing a clean parameter instance.""" - with helper.parameter_context() as param: - yield param - - @pytest.fixture - def dummy_instrument(self, helper: ParameterTestHelper) -> DummyInstrument: - """Fixture providing a dummy instrument instance.""" - instrument = DummyInstrument("dummy") - instrument.add_parameter( - "test_param", - parameter_class=Parameter, - set_cmd=helper._dummy_set, - get_cmd=helper._dummy_get, - ) - try: - yield instrument - finally: - instrument.close() - - def create_test_callback( - self, test_data: _TestDataManager, expected_value: Optional[float] = None - ) -> CallbackFunction: - """Create a test callback function with validation.""" - - def callback(param: ParameterBase, value: float) -> None: - test_data.callback_called = True - test_data.callback_count += 1 - assert param.name == "test_param", ( - f"Expected parameter name 'test_param', got {param.name}" - ) - if expected_value is not None: - assert value == expected_value, ( - f"Expected value {expected_value}, got {value}" - ) - - return callback - - @pytest.mark.parametrize("test_value", [0, CONSTANTS.VALUE_DEFAULT, -1, 999]) - def test_callback_executes_with_correct_parameters( - self, clean_parameter: Parameter, helper: ParameterTestHelper, test_value: float - ) -> None: - """Test callback execution with various parameter values.""" - ParameterBase._database_callback = self.create_test_callback( - helper.test_data, test_value - ) - clean_parameter(test_value) - assert helper.test_data.callback_called, "Callback was not executed" - - def test_callback_with_instrument( - self, dummy_instrument: DummyInstrument, helper: ParameterTestHelper - ) -> None: - """Test callback functionality with instrument parameters.""" - ParameterBase._database_callback = self.create_test_callback( - helper.test_data, - self.CONSTANTS.VALUE_INSTRUMENT, - ) - param = dummy_instrument.parameters["test_param"] - param(self.CONSTANTS.VALUE_INSTRUMENT) - assert helper.test_data.callback_called, ( - "Instrument parameter callback was not executed" - ) - - def test_callback_error_handling( - self, clean_parameter: Parameter, caplog: pytest.LogCaptureFixture - ) -> None: - """Test error handling in callback execution.""" - error_message = "Test error" - - def failing_callback(param: ParameterBase, value: float) -> None: - raise RuntimeError(error_message) - - ParameterBase._database_callback = failing_callback - - with caplog.at_level(logging.ERROR): - clean_parameter(self.CONSTANTS.VALUE_DEFAULT) - assert "Exception while running parameter callback" in caplog.text - assert error_message in caplog.text - - def test_callback_performance( - self, clean_parameter: Parameter, helper: ParameterTestHelper - ) -> None: - """Test callback performance overhead.""" - - def measure_execution_time( - callback: Optional[CallbackFunction] = None, - ) -> float: - if callback: - ParameterBase._database_callback = callback - start_time = time.perf_counter() - for _ in range(self.CONSTANTS.PERFORMANCE_ITERATIONS): - clean_parameter(self.CONSTANTS.VALUE_DEFAULT) - return time.perf_counter() - start_time - - base_time = measure_execution_time() - assert base_time > 0, "Invalid negative base time measurement" - - callback_time = measure_execution_time( - self.create_test_callback(helper.test_data) - ) - assert callback_time > 0, "Invalid negative callback time measurement" - assert callback_time < base_time * self.CONSTANTS.PERFORMANCE_MAX_OVERHEAD, ( - f"Callback overhead too high: {callback_time:.2f}s vs {base_time:.2f}s base time" - ) - - def test_callback_thread_safety( - self, clean_parameter: Parameter, helper: ParameterTestHelper - ) -> None: - """Test thread safety of callback execution.""" - ParameterBase._database_callback = self.create_test_callback(helper.test_data) - clean_parameter(self.CONSTANTS.VALUE_DEFAULT) - assert helper.test_data.callback_count == 1, ( - f"Expected exactly one callback execution, got {helper.test_data.callback_count}" - ) - - def test_callback_none(self, clean_parameter: Parameter) -> None: - """Test parameter behavior without callback.""" - ParameterBase._database_callback = None - clean_parameter(self.CONSTANTS.VALUE_DEFAULT) - assert clean_parameter() == self.CONSTANTS.VALUE_DEFAULT - - def test_callback_independence(self, clean_parameter: Parameter) -> None: - """Test callback isolation between tests.""" - assert ParameterBase._database_callback is None, ( - "Database callback not properly reset from previous test" - ) - - def test_callback_multiple_parameters(self, helper: ParameterTestHelper) -> None: - """Test callback behavior with multiple parameters.""" - param1 = helper.create_parameter() - param2 = helper.create_parameter() - - ParameterBase._database_callback = self.create_test_callback(helper.test_data) - - param1(self.CONSTANTS.VALUE_DEFAULT) - param2(self.CONSTANTS.VALUE_DEFAULT) - - assert helper.test_data.callback_count == 2, ( - "Callback should be called for each parameter" - ) - - def test_callback_value_persistence(self, clean_parameter: Parameter) -> None: - """Test that callback doesn't interfere with parameter value persistence.""" - test_value = self.CONSTANTS.VALUE_DEFAULT - - def callback(param: ParameterBase, value: float) -> None: - value += 1 - - ParameterBase._database_callback = callback - clean_parameter(test_value) - - assert clean_parameter() == test_value, ( - "Parameter value should remain unchanged" - ) - - def test_callback_with_different_parameter_types( - self, helper: ParameterTestHelper - ) -> None: - """Test callback behavior with different parameter types.""" - int_param = Parameter( - name="test_param_int", - set_cmd=None, - get_cmd=None, - vals=Ints(), - ) - float_param = Parameter( - name="test_param_float", - set_cmd=None, - get_cmd=None, - vals=Numbers(), - ) - - callback_values = [] - - def type_checking_callback(param: ParameterBase, value: float) -> None: - callback_values.append((param.name, type(value))) - - ParameterBase._database_callback = type_checking_callback - - int_param(42) - float_param(42.5) - - assert len(callback_values) == 2, "Both callbacks should execute" - assert callback_values[0][1] is int, "Integer parameter should pass int" - assert callback_values[1][1] is float, "Float parameter should pass float" - - def test_callback_concurrent_updates(self, helper: ParameterTestHelper) -> None: - """Test callback behavior with rapid parameter updates.""" - from concurrent.futures import ThreadPoolExecutor - - param = helper.create_parameter() - update_count = 100 - - def update_parameter(value: float) -> None: - param(value) - - ParameterBase._database_callback = self.create_test_callback(helper.test_data) - - with ThreadPoolExecutor(max_workers=4) as executor: - futures = [ - executor.submit(update_parameter, i) for i in range(update_count) - ] - for future in futures: - future.result() - - assert helper.test_data.callback_count == update_count, ( - f"Expected {update_count} callbacks, got {helper.test_data.callback_count}" - ) - - def test_callback_chaining(self, helper: ParameterTestHelper) -> None: - """Test multiple callbacks can be chained together.""" - param = helper.create_parameter() - callback_order = [] - - def callback1(param: ParameterBase, value: float) -> None: - callback_order.append(1) - callback2(param, value) - - def callback2(param: ParameterBase, value: float) -> None: - callback_order.append(2) - - ParameterBase._database_callback = callback1 - param(self.CONSTANTS.VALUE_DEFAULT) - - assert callback_order == [1, 2], "Callbacks should execute in order" - - def test_callback_memory_cleanup(self, helper: ParameterTestHelper) -> None: - """Test that callbacks don't cause memory leaks.""" - import gc - import weakref - - param = helper.create_parameter() - - def create_callback() -> CallbackFunction: - local_data = [0] - - def callback(param: ParameterBase, value: float) -> None: - local_data[0] = value - - return callback - - callback = create_callback() - ref = weakref.ref(callback) - ParameterBase._database_callback = callback - - param(self.CONSTANTS.VALUE_DEFAULT) - - ParameterBase._database_callback = None - del callback - gc.collect() - - assert ref() is None, "Callback should be garbage collected" From 7f4748bc02adc99cd95ec7e3211dda13fa8c0f36 Mon Sep 17 00:00:00 2001 From: mahmut-oqs Date: Tue, 25 Feb 2025 08:35:54 +0100 Subject: [PATCH 05/15] Update parameter_base.py --- src/qcodes/parameters/parameter_base.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index c7a3533877d4..24db517c15bd 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -13,6 +13,7 @@ from qcodes.utils import DelegateAttributes, full_class, qcodes_abstractmethod from qcodes.validators import Enum, Ints, Validator +from ..utils.types import NumberType from .cache import _Cache, _CacheProtocol from .named_repr import named_repr from .permissive_range import permissive_range @@ -385,9 +386,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 @@ -788,8 +790,8 @@ def set_wrapper(value: ParamDataType, **kwargs: Any) -> None: return set_wrapper def get_ramp_values( - self, value: float | Sized, step: float | None = None - ) -> Sequence[float | Sized]: + self, value: NumberType | Sized, step: NumberType | None = None + ) -> Sequence[NumberType | Sized]: """ Return values to sweep from current value to target value. This method can be overridden to have a custom sweep behaviour. @@ -814,8 +816,7 @@ def get_ramp_values( self.get() start_value = self.get_latest() if not ( - isinstance(start_value, (int, float)) - and isinstance(value, (int, float)) + isinstance(start_value, NumberType) and isinstance(value, NumberType) ): # parameter is numeric but either one of the endpoints # is not or the starting point is unknown. The later @@ -864,7 +865,7 @@ def validate(self, value: ParamDataType) -> None: validator.validate(value, self._validate_context) @property - def step(self) -> float | None: + def step(self) -> NumberType | None: """ Stepsize that this Parameter uses during set operation. Stepsize must be a positive number or None. @@ -888,12 +889,12 @@ def step(self) -> float | None: return self._step @step.setter - def step(self, step: float | None) -> None: + def step(self, step: NumberType | None) -> None: if step is None: - self._step: float | None = step + self._step: NumberType | None = step elif not all(getattr(vals, "is_numeric", True) for vals in self._vals): raise TypeError("you can only step numeric parameters") - elif not isinstance(step, (int, float)): + elif not isinstance(step, NumberType): raise TypeError("step must be a number") elif step == 0: self._step = None @@ -933,7 +934,7 @@ def post_delay(self) -> float: @post_delay.setter def post_delay(self, post_delay: float) -> None: - if not isinstance(post_delay, (int, float)): + if not isinstance(post_delay, NumberType): raise TypeError(f"post_delay ({post_delay}) must be a number") if post_delay < 0: raise ValueError(f"post_delay ({post_delay}) must not be negative") @@ -962,7 +963,7 @@ def inter_delay(self) -> float: @inter_delay.setter def inter_delay(self, inter_delay: float) -> None: - if not isinstance(inter_delay, (int, float)): + if not isinstance(inter_delay, NumberType): raise TypeError(f"inter_delay ({inter_delay}) must be a number") if inter_delay < 0: raise ValueError(f"inter_delay ({inter_delay}) must not be negative") From 9e32e49d2e04fdf43200369def8ee4b37ab4228d Mon Sep 17 00:00:00 2001 From: mahmut-oqs Date: Tue, 25 Feb 2025 08:37:38 +0100 Subject: [PATCH 06/15] Create test_parameter_callback.py --- tests/parameter/test_parameter_callback.py | 212 +++++++++++++++++++++ 1 file changed, 212 insertions(+) create mode 100644 tests/parameter/test_parameter_callback.py diff --git a/tests/parameter/test_parameter_callback.py b/tests/parameter/test_parameter_callback.py new file mode 100644 index 000000000000..0ca8c16c53ad --- /dev/null +++ b/tests/parameter/test_parameter_callback.py @@ -0,0 +1,212 @@ +from __future__ import annotations + +import pytest + +from qcodes import validators +from qcodes.parameters import Parameter + + +# Basic functionality +def test_set_value_changed_callback(): + """Test setting and removing a value changed callback""" + called_with = [] + + def callback(param, value): + called_with.append((param, value)) + + p = Parameter("p", set_cmd=None, get_cmd=None) + + p.set_value_changed_callback(callback) + p.set(1) + assert len(called_with) == 1 + assert called_with[0][0] == p + assert called_with[0][1] == 1 + + p.set_value_changed_callback(None) + p.set(2) + assert len(called_with) == 1 + + +def test_callback_initial_value(): + """Test callback behavior with initial value""" + called_values = [] + + def callback(param, value): + called_values.append(value) + + p = Parameter("p", set_cmd=None, get_cmd=None, initial_value=5) + p.set_value_changed_callback(callback) + + assert called_values == [] + p.set(6) + assert called_values == [6] + + +# Multiple callbacks +def test_multiple_set_callbacks(): + """Test callback is called each time set is called""" + call_count = 0 + + def callback(param, value): + nonlocal call_count + call_count += 1 + + p = Parameter("p", set_cmd=None, get_cmd=None) + p.set_value_changed_callback(callback) + + p.set(1) + p.set(1) + p.set(2) + + assert call_count == 3 + + +def test_multiple_callbacks_replace(): + """Test that setting a new callback replaces the old one""" + calls_a = [] + calls_b = [] + + def callback_a(param, value): + calls_a.append(value) + + def callback_b(param, value): + calls_b.append(value) + + p = Parameter("p", set_cmd=None, get_cmd=None) + p.set_value_changed_callback(callback_a) + p.set(1) + p.set_value_changed_callback(callback_b) + p.set(2) + + assert calls_a == [1] + assert calls_b == [2] + + +def test_callback_lifecycle(): + """Test complete callback lifecycle: add, call, remove""" + calls_a = [] + calls_b = [] + + def callback_a(param, value): + calls_a.append(value) + + def callback_b(param, value): + calls_b.append(value) + + p = Parameter("p", set_cmd=None, get_cmd=None) + + p.set_value_changed_callback(callback_a) + p.set_value_changed_callback(callback_b) + p.set(1) + assert calls_a == [] and calls_b == [1] + + p.set_value_changed_callback(None) + p.set(2) + assert calls_a == [] and calls_b == [1] + + +# Edge cases +def test_callback_with_none_value(): + """Test callback behavior when setting None value""" + called_values = [] + + def callback(param, value): + called_values.append(value) + + p = Parameter("p", set_cmd=None, get_cmd=None) + p.set_value_changed_callback(callback) + p.set(None) + assert called_values == [None] + + +def test_remove_nonexistent_callback(): + """Test removing a callback that was never set""" + p = Parameter("p", set_cmd=None, get_cmd=None) + p.set_value_changed_callback(None) + p.set(1) + + +# Validation +def test_callback_with_validation(): + """Test callback is called only after validation passes""" + called_values = [] + + def callback(param, value): + called_values.append(value) + + p = Parameter("p", set_cmd=None, get_cmd=None, vals=validators.Numbers(0, 10)) + p.set_value_changed_callback(callback) + + with pytest.raises(ValueError): + p.set(20) + + assert len(called_values) == 0 + + +def test_invalid_callback(): + """Test setting an invalid callback raises TypeError""" + p = Parameter("p", set_cmd=None, get_cmd=None) + + with pytest.raises(TypeError, match="Callback must be type callable or None"): + p.set_value_changed_callback("not_a_callback") + + +def test_callback_exception_handling(caplog): + """Test that exceptions in callbacks are caught and logged""" + + def failing_callback(param, value): + raise ValueError("Callback failed") + + p = Parameter("p", set_cmd=None, get_cmd=None) + p.set_value_changed_callback(failing_callback) + + p.set(1) + + assert "Exception while running parameter callback" in caplog.text + + +# Advanced features +def test_callback_with_cache(): + """Test callback behavior with cache operations""" + called_values = [] + + def callback(param, value): + called_values.append(value) + + p = Parameter("p", set_cmd=None, get_cmd=None) + p.set_value_changed_callback(callback) + + p.cache.set(1) + assert called_values == [] + + p.set(2) + assert called_values == [2] + + +def test_callback_thread_safety(): + """Test callback behavior with rapid value changes""" + import threading + import time + + called_values = [] + + def callback(param, value): + time.sleep(0.01) + called_values.append(value) + + p = Parameter("p", set_cmd=None, get_cmd=None) + p.set_value_changed_callback(callback) + + def set_value(): + p.set(1) + p.set(2) + + t1 = threading.Thread(target=set_value) + t2 = threading.Thread(target=set_value) + + t1.start() + t2.start() + t1.join() + t2.join() + + assert len(called_values) == 4 From 1cdea0e6155a436730689867d798bd04f884a854 Mon Sep 17 00:00:00 2001 From: mahmut-oqs Date: Tue, 4 Mar 2025 11:40:01 +0100 Subject: [PATCH 07/15] refactor: migrate parameter value change callback from instance-level to global class-level with tests --- src/qcodes/parameters/parameter_base.py | 42 +-- tests/parameter/test_parameter_callback.py | 212 --------------- .../parameter/test_value_changed_callback.py | 243 ++++++++++++++++++ 3 files changed, 252 insertions(+), 245 deletions(-) delete mode 100644 tests/parameter/test_parameter_callback.py create mode 100644 tests/parameter/test_value_changed_callback.py diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index 24db517c15bd..d8d69e03c1e5 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -192,6 +192,8 @@ class ParameterBase(MetadatableWithName): """ + _value_changed_callback: Callable[[ParameterBase, Any], None] | None = None + def __init__( self, name: str, @@ -213,7 +215,6 @@ def __init__( abstract: bool | None = False, bind_to_instrument: bool = True, register_name: str | None = None, - value_changed_callback: Callable[[ParameterBase, Any], None] | None = None, ) -> None: super().__init__(metadata) if not str(name).isidentifier(): @@ -344,8 +345,6 @@ def __init__( instrument.parameters[name] = self - self._value_changed_callback = value_changed_callback - @property def _implements_get_raw(self) -> bool: implements_get_raw = hasattr(self, "get_raw") and not getattr( @@ -781,7 +780,13 @@ def set_wrapper(value: ParamDataType, **kwargs: Any) -> None: self.cache._update_with(value=val_step, raw_value=raw_val_step) - self._invoke_callback(value) + if ParameterBase._value_changed_callback is not None: + try: + ParameterBase._value_changed_callback(self, val_step) + except Exception as e: + LOG.warning( + f"{e} in value_changed_callback for {self.full_name} and its value {val_step}" + ) except Exception as e: e.args = (*e.args, f"setting {self} to {value}") @@ -1128,35 +1133,6 @@ def underlying_instrument(self) -> InstrumentBase | None: def abstract(self) -> bool | None: return self._abstract - def _invoke_callback(self, value: Any) -> None: - """ - Invoke the instance-specific callback if it exists. - - Args: - value: The new parameter value - - """ - try: - if self._value_changed_callback is not None: - self._value_changed_callback(self, value) - except Exception as e: - LOG.exception(f"Exception while running parameter callback: {e}") - - def set_value_changed_callback( - self, callback: Callable[[ParameterBase, Any], None] | None - ) -> None: - """ - Set the callback for this specific parameter instance. - - Args: - callback: The callback function to be called when the parameter - value changes, or None to remove the callback. - - """ - if callback is not None and not callable(callback): - raise TypeError("Callback must be type callable or None") - self._value_changed_callback = callback - class GetLatest(DelegateAttributes): """ diff --git a/tests/parameter/test_parameter_callback.py b/tests/parameter/test_parameter_callback.py deleted file mode 100644 index 0ca8c16c53ad..000000000000 --- a/tests/parameter/test_parameter_callback.py +++ /dev/null @@ -1,212 +0,0 @@ -from __future__ import annotations - -import pytest - -from qcodes import validators -from qcodes.parameters import Parameter - - -# Basic functionality -def test_set_value_changed_callback(): - """Test setting and removing a value changed callback""" - called_with = [] - - def callback(param, value): - called_with.append((param, value)) - - p = Parameter("p", set_cmd=None, get_cmd=None) - - p.set_value_changed_callback(callback) - p.set(1) - assert len(called_with) == 1 - assert called_with[0][0] == p - assert called_with[0][1] == 1 - - p.set_value_changed_callback(None) - p.set(2) - assert len(called_with) == 1 - - -def test_callback_initial_value(): - """Test callback behavior with initial value""" - called_values = [] - - def callback(param, value): - called_values.append(value) - - p = Parameter("p", set_cmd=None, get_cmd=None, initial_value=5) - p.set_value_changed_callback(callback) - - assert called_values == [] - p.set(6) - assert called_values == [6] - - -# Multiple callbacks -def test_multiple_set_callbacks(): - """Test callback is called each time set is called""" - call_count = 0 - - def callback(param, value): - nonlocal call_count - call_count += 1 - - p = Parameter("p", set_cmd=None, get_cmd=None) - p.set_value_changed_callback(callback) - - p.set(1) - p.set(1) - p.set(2) - - assert call_count == 3 - - -def test_multiple_callbacks_replace(): - """Test that setting a new callback replaces the old one""" - calls_a = [] - calls_b = [] - - def callback_a(param, value): - calls_a.append(value) - - def callback_b(param, value): - calls_b.append(value) - - p = Parameter("p", set_cmd=None, get_cmd=None) - p.set_value_changed_callback(callback_a) - p.set(1) - p.set_value_changed_callback(callback_b) - p.set(2) - - assert calls_a == [1] - assert calls_b == [2] - - -def test_callback_lifecycle(): - """Test complete callback lifecycle: add, call, remove""" - calls_a = [] - calls_b = [] - - def callback_a(param, value): - calls_a.append(value) - - def callback_b(param, value): - calls_b.append(value) - - p = Parameter("p", set_cmd=None, get_cmd=None) - - p.set_value_changed_callback(callback_a) - p.set_value_changed_callback(callback_b) - p.set(1) - assert calls_a == [] and calls_b == [1] - - p.set_value_changed_callback(None) - p.set(2) - assert calls_a == [] and calls_b == [1] - - -# Edge cases -def test_callback_with_none_value(): - """Test callback behavior when setting None value""" - called_values = [] - - def callback(param, value): - called_values.append(value) - - p = Parameter("p", set_cmd=None, get_cmd=None) - p.set_value_changed_callback(callback) - p.set(None) - assert called_values == [None] - - -def test_remove_nonexistent_callback(): - """Test removing a callback that was never set""" - p = Parameter("p", set_cmd=None, get_cmd=None) - p.set_value_changed_callback(None) - p.set(1) - - -# Validation -def test_callback_with_validation(): - """Test callback is called only after validation passes""" - called_values = [] - - def callback(param, value): - called_values.append(value) - - p = Parameter("p", set_cmd=None, get_cmd=None, vals=validators.Numbers(0, 10)) - p.set_value_changed_callback(callback) - - with pytest.raises(ValueError): - p.set(20) - - assert len(called_values) == 0 - - -def test_invalid_callback(): - """Test setting an invalid callback raises TypeError""" - p = Parameter("p", set_cmd=None, get_cmd=None) - - with pytest.raises(TypeError, match="Callback must be type callable or None"): - p.set_value_changed_callback("not_a_callback") - - -def test_callback_exception_handling(caplog): - """Test that exceptions in callbacks are caught and logged""" - - def failing_callback(param, value): - raise ValueError("Callback failed") - - p = Parameter("p", set_cmd=None, get_cmd=None) - p.set_value_changed_callback(failing_callback) - - p.set(1) - - assert "Exception while running parameter callback" in caplog.text - - -# Advanced features -def test_callback_with_cache(): - """Test callback behavior with cache operations""" - called_values = [] - - def callback(param, value): - called_values.append(value) - - p = Parameter("p", set_cmd=None, get_cmd=None) - p.set_value_changed_callback(callback) - - p.cache.set(1) - assert called_values == [] - - p.set(2) - assert called_values == [2] - - -def test_callback_thread_safety(): - """Test callback behavior with rapid value changes""" - import threading - import time - - called_values = [] - - def callback(param, value): - time.sleep(0.01) - called_values.append(value) - - p = Parameter("p", set_cmd=None, get_cmd=None) - p.set_value_changed_callback(callback) - - def set_value(): - p.set(1) - p.set(2) - - t1 = threading.Thread(target=set_value) - t2 = threading.Thread(target=set_value) - - t1.start() - t2.start() - t1.join() - t2.join() - - assert len(called_values) == 4 diff --git a/tests/parameter/test_value_changed_callback.py b/tests/parameter/test_value_changed_callback.py new file mode 100644 index 000000000000..2b453572d145 --- /dev/null +++ b/tests/parameter/test_value_changed_callback.py @@ -0,0 +1,243 @@ +import threading +import time +from collections import Counter +from contextlib import nullcontext +from typing import Any, Callable # noqa: UP035 + +import pytest + +from qcodes import validators +from qcodes.parameters import Parameter +from qcodes.parameters.parameter_base import ParameterBase + +DEFAULT_VALUE = 42 +DELAY_TIME = 0.1 +STEP_SIZE = 0.1 +THREAD_SLEEP = 0.01 + + +@pytest.fixture(autouse=True) +def _reset_callback(): + """Reset the callback after each test""" + yield + ParameterBase._value_changed_callback = None + + +@pytest.fixture +def basic_parameter(basic_callback: Callable[[ParameterBase, Any], None]) -> Parameter: + """Fixture providing a basic parameter with callback""" + param = Parameter("test_param", set_cmd=None, get_cmd=None) + ParameterBase._value_changed_callback = basic_callback + return param + + +@pytest.fixture(scope="function") +def basic_callback( + captured_params: list[tuple[ParameterBase, Any]], +) -> Callable[[ParameterBase, Any], None]: + """Fixture providing a standard callback function""" + + def callback(param: ParameterBase, value: Any) -> None: + captured_params.append((param, value)) + + return callback + + +@pytest.fixture(scope="function") +def captured_params() -> list[tuple[ParameterBase, Any]]: + """Fixture for capturing callback parameters""" + return [] + + +class TestBasicCallbackBehavior: + """Tests for basic callback functionality""" + + def test_value_changed_callback( + self, + basic_parameter: Parameter, + captured_params: list[tuple[ParameterBase, Any]], + ) -> None: + """Test basic callback functionality""" + basic_parameter(DEFAULT_VALUE) + assert len(captured_params) == 1 + assert captured_params[0][0] is basic_parameter + assert captured_params[0][1] == DEFAULT_VALUE + + def test_multiple_value_changes( + self, + basic_parameter: Parameter, + captured_params: list[tuple[ParameterBase, Any]], + ) -> None: + """Test callback is called for each value change""" + values = [1, 1, 2] + for val in values: + basic_parameter(val) + assert len(captured_params) == len(values) + + +class TestValidationBehavior: + """Tests for validation-related functionality""" + + @pytest.mark.parametrize( + "test_input,validator,should_callback", + [ + pytest.param(5, validators.Numbers(0, 10), True, id="valid_number"), + pytest.param(-1, validators.Numbers(0, 10), False, id="invalid_number"), + pytest.param("valid", validators.Strings(), True, id="valid_string"), + pytest.param(42, validators.Numbers(max_value=10), False, id="over_max"), + ], + ) + def test_callback_with_different_validators( + self, + captured_params: list[tuple[ParameterBase, Any]], + basic_callback: Callable[[ParameterBase, Any], None], + test_input: Any, + validator: Any, + should_callback: bool, + ) -> None: + """Test callback behavior with different validator types""" + param = Parameter("test_param", set_cmd=None, get_cmd=None, vals=validator) + ParameterBase._value_changed_callback = basic_callback + + with pytest.raises(ValueError) if not should_callback else nullcontext(): + param(test_input) + + assert bool(len(captured_params)) == should_callback + + +class TestErrorHandling: + """Tests for error handling and edge cases""" + + def test_callback_exception_handling(self, basic_parameter: Parameter) -> None: + """Test that callback exceptions are handled gracefully""" + + def failing_callback(param: ParameterBase, value: Any) -> None: + raise RuntimeError("Intentional failure") + + ParameterBase._value_changed_callback = failing_callback + basic_parameter(DEFAULT_VALUE) + assert basic_parameter() == DEFAULT_VALUE + + def test_callback_with_none_value( + self, + basic_parameter: Parameter, + captured_params: list[tuple[ParameterBase, Any]], + ) -> None: + """Test handling of None values""" + basic_parameter(None) + + assert len(captured_params) == 1, "Should handle None value" + assert captured_params[0][1] is None, "Should capture None value correctly" + + +class TestAdvancedFeatures: + """Tests for advanced parameter features""" + + def test_callback_thread_safety( + self, + ) -> None: + """Test thread safety of callbacks + + Tests concurrent parameter updates using multiple threads, + ensuring all callbacks are executed correctly. + """ + NUM_THREADS = 2 + TEST_VALUES = [1, 2] + captured_values = [] + + lock = threading.Lock() + + def thread_safe_callback(param: ParameterBase, value: Any) -> None: + time.sleep(THREAD_SLEEP) + with lock: + captured_values.append(value) + + ParameterBase._value_changed_callback = thread_safe_callback + param = Parameter("test_param", set_cmd=None, get_cmd=None) + + threads = [ + threading.Thread( + target=lambda: [param(val) for val in TEST_VALUES], + name=f"CallbackThread-{i}", + ) + for i in range(NUM_THREADS) + ] + + [t.start() for t in threads] + [t.join() for t in threads] + + value_counts = Counter(captured_values) + expected_count = NUM_THREADS * len(TEST_VALUES) + + assert len(captured_values) == expected_count, ( + f"Expected {expected_count} callback captures, got {len(captured_values)}" + ) + assert all(count == NUM_THREADS for count in value_counts.values()), ( + f"Uneven value distribution: {dict(value_counts)}" + ) + + def test_callback_with_steps( + self, + basic_callback: Callable[[ParameterBase, Any], None], + captured_params: list[tuple[ParameterBase, Any]], + ) -> None: + """Test stepped parameter setting + + Verifies that parameters with step values correctly trigger + callbacks for each intermediate step. + """ + START_VALUE = 0.0 + TARGET_VALUE = 0.3 + expected_steps = [0.1, 0.2, 0.3] + + param = Parameter( + name="test_param", + set_cmd=None, + get_cmd=None, + step=STEP_SIZE, + initial_value=START_VALUE, + ) + ParameterBase._value_changed_callback = basic_callback + + param(TARGET_VALUE) + + actual_values = [val[1] for val in captured_params] + assert len(actual_values) == len(expected_steps), ( + f"Expected {len(expected_steps)} steps, got {len(actual_values)}" + ) + assert actual_values == expected_steps, ( + f"Expected steps {expected_steps}, got {actual_values}" + ) + + def test_nested_callbacks( + self, + ) -> None: + """Test nested callback behavior""" + param = Parameter("test_param", set_cmd=None, get_cmd=None) + + def callback(param: ParameterBase, value: Any) -> None: + param.cache.set(value) + + ParameterBase._value_changed_callback = callback + param(1) + assert param.cache.get() == 1 + + def test_callback_with_delay( + self, + basic_parameter: Parameter, + ) -> None: + """Test delayed parameter setting""" + captured_times = [] + start_time = time.time() + + def timing_callback(param: ParameterBase, value: Any) -> None: + captured_times.append(time.time() - start_time) + + basic_parameter.post_delay = DELAY_TIME + ParameterBase._value_changed_callback = timing_callback + + basic_parameter(1) + basic_parameter(2) + + assert len(captured_times) == 2 + assert captured_times[1] - captured_times[0] >= DELAY_TIME From 02ae8937d055b197806bad8b1bfe0d74ae28c0cf Mon Sep 17 00:00:00 2001 From: mahmut-oqs Date: Fri, 14 Mar 2025 15:09:27 +0100 Subject: [PATCH 08/15] Update parameter_base.py --- src/qcodes/parameters/parameter_base.py | 27 ++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index d8d69e03c1e5..36b3fbc6c85e 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -189,10 +189,11 @@ class ParameterBase(MetadatableWithName): register_name: Specifies if the parameter should be registered in datasets using a different name than the parameter's full_name - """ - _value_changed_callback: Callable[[ParameterBase, Any], None] | None = None + global_value_changed_callback: ClassVar[ + Callable[[ParameterBase, Any], None] | None + ] = None def __init__( self, @@ -780,12 +781,14 @@ def set_wrapper(value: ParamDataType, **kwargs: Any) -> None: self.cache._update_with(value=val_step, raw_value=raw_val_step) - if ParameterBase._value_changed_callback is not None: + if self.__class__.global_value_changed_callback is not None: try: - ParameterBase._value_changed_callback(self, val_step) + self.__class__.global_value_changed_callback(self, val_step) except Exception as e: LOG.warning( - f"{e} in value_changed_callback for {self.full_name} and its value {val_step}" + f"Exception {e} in global value_changed_callback " + f"for {self.full_name} with value {val_step}", + exc_info=True, ) except Exception as e: @@ -1133,6 +1136,20 @@ def underlying_instrument(self) -> InstrumentBase | None: def abstract(self) -> bool | None: return self._abstract + @classmethod + def set_global_value_changed_callback( + cls, callback: Callable[[ParameterBase, Any], None] | None + ) -> None: + """ + Set (or clear, if None) a single global callback that will be called + after *any* ParameterBase instance changes value. + + The callback must accept two arguments: + - The ParameterBase instance whose value changed + - The new value of that parameter + """ + cls.global_value_changed_callback = callback + class GetLatest(DelegateAttributes): """ From e199299d7a61fd69b40e02f37707ea468d1ce1e9 Mon Sep 17 00:00:00 2001 From: mahmut-oqs Date: Wed, 19 Mar 2025 10:29:38 +0100 Subject: [PATCH 09/15] Update parameter_base.py --- src/qcodes/parameters/parameter_base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index 36b3fbc6c85e..7e41317daa8c 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -189,6 +189,7 @@ class ParameterBase(MetadatableWithName): register_name: Specifies if the parameter should be registered in datasets using a different name than the parameter's full_name + """ global_value_changed_callback: ClassVar[ @@ -751,7 +752,7 @@ def set_wrapper(value: ParamDataType, **kwargs: Any) -> None: # a list containing only `value`. steps = self.get_ramp_values(value, step=self.step) - for step_index, val_step in enumerate(steps): + for val_step in steps: # even if the final value is valid we may be generating # steps that are not so validate them too self.validate(val_step) @@ -1143,7 +1144,6 @@ def set_global_value_changed_callback( """ Set (or clear, if None) a single global callback that will be called after *any* ParameterBase instance changes value. - The callback must accept two arguments: - The ParameterBase instance whose value changed - The new value of that parameter From 3af760eede3f12857b5f3a024be88e2223255dd7 Mon Sep 17 00:00:00 2001 From: mahmut-oqs Date: Wed, 19 Mar 2025 10:31:26 +0100 Subject: [PATCH 10/15] Update test_value_changed_callback.py --- .../parameter/test_value_changed_callback.py | 90 ++++++++++++++----- 1 file changed, 68 insertions(+), 22 deletions(-) diff --git a/tests/parameter/test_value_changed_callback.py b/tests/parameter/test_value_changed_callback.py index 2b453572d145..8b9904ca15da 100644 --- a/tests/parameter/test_value_changed_callback.py +++ b/tests/parameter/test_value_changed_callback.py @@ -1,8 +1,10 @@ +import gc +import sqlite3 import threading import time from collections import Counter from contextlib import nullcontext -from typing import Any, Callable # noqa: UP035 +from typing import TYPE_CHECKING, Any import pytest @@ -10,31 +12,37 @@ from qcodes.parameters import Parameter from qcodes.parameters.parameter_base import ParameterBase +if TYPE_CHECKING: + from collections.abc import Callable, Generator + + DEFAULT_VALUE = 42 DELAY_TIME = 0.1 STEP_SIZE = 0.1 THREAD_SLEEP = 0.01 -@pytest.fixture(autouse=True) -def _reset_callback(): +@pytest.fixture(autouse=True) # type: ignore[misc] +def _reset_callback() -> "Generator[None, None, None]": """Reset the callback after each test""" yield - ParameterBase._value_changed_callback = None + ParameterBase.set_global_value_changed_callback(None) -@pytest.fixture -def basic_parameter(basic_callback: Callable[[ParameterBase, Any], None]) -> Parameter: +@pytest.fixture() # type: ignore[misc] +def basic_parameter( + basic_callback: "Callable[[ParameterBase, Any], None]", +) -> Parameter: """Fixture providing a basic parameter with callback""" param = Parameter("test_param", set_cmd=None, get_cmd=None) - ParameterBase._value_changed_callback = basic_callback + ParameterBase.set_global_value_changed_callback(basic_callback) return param -@pytest.fixture(scope="function") +@pytest.fixture(scope="function") # type: ignore[misc] def basic_callback( captured_params: list[tuple[ParameterBase, Any]], -) -> Callable[[ParameterBase, Any], None]: +) -> "Callable[[ParameterBase, Any], None]": """Fixture providing a standard callback function""" def callback(param: ParameterBase, value: Any) -> None: @@ -43,12 +51,31 @@ def callback(param: ParameterBase, value: Any) -> None: return callback -@pytest.fixture(scope="function") +@pytest.fixture(scope="function") # type: ignore[misc] def captured_params() -> list[tuple[ParameterBase, Any]]: """Fixture for capturing callback parameters""" return [] +@pytest.fixture(autouse=True, scope="function") # type: ignore[misc] +def cleanup_db_connections(): + """Clean up any open SQLite connections after each test""" + yield + gc.collect() + + open_connections = [ + obj for obj in gc.get_objects() if isinstance(obj, sqlite3.Connection) + ] + + for conn in open_connections: + try: + conn.close() + except Exception: + pass + + gc.collect() + + class TestBasicCallbackBehavior: """Tests for basic callback functionality""" @@ -74,11 +101,28 @@ def test_multiple_value_changes( basic_parameter(val) assert len(captured_params) == len(values) + def test_set_global_callback(self) -> None: + """Test setting and clearing global callback""" + param = Parameter("test_param", set_cmd=None, get_cmd=None) + captured = [] + + def test_callback(p: ParameterBase, value: Any) -> None: + captured.append((p, value)) + + ParameterBase.set_global_value_changed_callback(test_callback) + param(1) + assert len(captured) == 1 + assert captured[0] == (param, 1) + + ParameterBase.set_global_value_changed_callback(None) + param(2) + assert len(captured) == 1 + class TestValidationBehavior: """Tests for validation-related functionality""" - @pytest.mark.parametrize( + @pytest.mark.parametrize( # type: ignore[misc] "test_input,validator,should_callback", [ pytest.param(5, validators.Numbers(0, 10), True, id="valid_number"), @@ -90,14 +134,14 @@ class TestValidationBehavior: def test_callback_with_different_validators( self, captured_params: list[tuple[ParameterBase, Any]], - basic_callback: Callable[[ParameterBase, Any], None], + basic_callback: "Callable[[ParameterBase, Any], None]", test_input: Any, validator: Any, should_callback: bool, ) -> None: """Test callback behavior with different validator types""" param = Parameter("test_param", set_cmd=None, get_cmd=None, vals=validator) - ParameterBase._value_changed_callback = basic_callback + ParameterBase.set_global_value_changed_callback(basic_callback) with pytest.raises(ValueError) if not should_callback else nullcontext(): param(test_input) @@ -108,15 +152,17 @@ def test_callback_with_different_validators( class TestErrorHandling: """Tests for error handling and edge cases""" - def test_callback_exception_handling(self, basic_parameter: Parameter) -> None: + def test_callback_exception_handling(self) -> None: """Test that callback exceptions are handled gracefully""" def failing_callback(param: ParameterBase, value: Any) -> None: raise RuntimeError("Intentional failure") - ParameterBase._value_changed_callback = failing_callback - basic_parameter(DEFAULT_VALUE) - assert basic_parameter() == DEFAULT_VALUE + param = Parameter("test_param", set_cmd=None, get_cmd=None) + ParameterBase.set_global_value_changed_callback(failing_callback) + + param(DEFAULT_VALUE) + assert param() == DEFAULT_VALUE def test_callback_with_none_value( self, @@ -152,8 +198,8 @@ def thread_safe_callback(param: ParameterBase, value: Any) -> None: with lock: captured_values.append(value) - ParameterBase._value_changed_callback = thread_safe_callback param = Parameter("test_param", set_cmd=None, get_cmd=None) + ParameterBase.set_global_value_changed_callback(thread_safe_callback) threads = [ threading.Thread( @@ -178,7 +224,7 @@ def thread_safe_callback(param: ParameterBase, value: Any) -> None: def test_callback_with_steps( self, - basic_callback: Callable[[ParameterBase, Any], None], + basic_callback: "Callable[[ParameterBase, Any], None]", captured_params: list[tuple[ParameterBase, Any]], ) -> None: """Test stepped parameter setting @@ -197,7 +243,7 @@ def test_callback_with_steps( step=STEP_SIZE, initial_value=START_VALUE, ) - ParameterBase._value_changed_callback = basic_callback + ParameterBase.set_global_value_changed_callback(basic_callback) param(TARGET_VALUE) @@ -218,7 +264,7 @@ def test_nested_callbacks( def callback(param: ParameterBase, value: Any) -> None: param.cache.set(value) - ParameterBase._value_changed_callback = callback + ParameterBase.set_global_value_changed_callback(callback) param(1) assert param.cache.get() == 1 @@ -234,7 +280,7 @@ def timing_callback(param: ParameterBase, value: Any) -> None: captured_times.append(time.time() - start_time) basic_parameter.post_delay = DELAY_TIME - ParameterBase._value_changed_callback = timing_callback + ParameterBase.set_global_value_changed_callback(timing_callback) basic_parameter(1) basic_parameter(2) From acd12fde85ed2e2255194dba0384ab11aefa8df9 Mon Sep 17 00:00:00 2001 From: mahmut-oqs Date: Wed, 19 Mar 2025 12:34:26 +0100 Subject: [PATCH 11/15] Create 6934.new --- docs/changes/newsfragments/6934.new | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changes/newsfragments/6934.new diff --git a/docs/changes/newsfragments/6934.new b/docs/changes/newsfragments/6934.new new file mode 100644 index 000000000000..86d9bd8cd576 --- /dev/null +++ b/docs/changes/newsfragments/6934.new @@ -0,0 +1,5 @@ +Added a global callback mechanism to `ParameterBase` that enables users to +supply custom callback functions to handle parameter changes. This new feature +allows for flexible integrations—such as logging changes, updating dashboards, or +other custom processing—without modifying full snapshot behavior. +See the PR for details on a usage example. From 37ecda356b2c8be16b7b92b0b26c6b6e1b65d8ff Mon Sep 17 00:00:00 2001 From: Jinnapat Indrapiromkul Date: Fri, 28 Mar 2025 13:46:15 +0100 Subject: [PATCH 12/15] Enable setting on_set_callback on instance --- src/qcodes/parameters/parameter_base.py | 43 ++++++++--------- .../parameter/test_value_changed_callback.py | 46 +++++++++++++------ 2 files changed, 51 insertions(+), 38 deletions(-) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index 7e41317daa8c..2e73b0174f1f 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -192,9 +192,7 @@ class ParameterBase(MetadatableWithName): """ - global_value_changed_callback: ClassVar[ - Callable[[ParameterBase, Any], None] | None - ] = None + global_on_set_callback: ClassVar[Callable[[ParameterBase, Any], None] | None] = None def __init__( self, @@ -217,6 +215,7 @@ def __init__( abstract: bool | None = False, bind_to_instrument: bool = True, register_name: str | None = None, + on_set_callback: Callable[[ParameterBase, Any], None] | None = None, ) -> None: super().__init__(metadata) if not str(name).isidentifier(): @@ -232,6 +231,7 @@ def __init__( self._snapshot_get = snapshot_get self._snapshot_value = snapshot_value self.snapshot_exclude = snapshot_exclude + self.on_set_callback = on_set_callback if not isinstance(vals, (Validator, type(None))): raise TypeError("vals must be None or a Validator") @@ -377,6 +377,7 @@ def _build__doc__(self) -> str | None: @property def vals(self) -> Validator | None: """ + N The first validator of the parameter. None if no validators are set for this parameter. @@ -782,15 +783,7 @@ def set_wrapper(value: ParamDataType, **kwargs: Any) -> None: self.cache._update_with(value=val_step, raw_value=raw_val_step) - if self.__class__.global_value_changed_callback is not None: - try: - self.__class__.global_value_changed_callback(self, val_step) - except Exception as e: - LOG.warning( - f"Exception {e} in global value_changed_callback " - f"for {self.full_name} with value {val_step}", - exc_info=True, - ) + self._call_on_set_callback(val_step) except Exception as e: e.args = (*e.args, f"setting {self} to {value}") @@ -798,6 +791,19 @@ def set_wrapper(value: ParamDataType, **kwargs: Any) -> None: return set_wrapper + def _call_on_set_callback(self, value: NumberType | Sized) -> None: + try: + if self.on_set_callback is not None: + self.on_set_callback(self, value) + elif self.global_on_set_callback is not None: + self.global_on_set_callback(value) + except Exception as e: + LOG.warning( + f"Exception {e} in on set callback " + f"for {self.full_name} with value {value}", + exc_info=True, + ) + def get_ramp_values( self, value: NumberType | Sized, step: NumberType | None = None ) -> Sequence[NumberType | Sized]: @@ -1137,19 +1143,6 @@ def underlying_instrument(self) -> InstrumentBase | None: def abstract(self) -> bool | None: return self._abstract - @classmethod - def set_global_value_changed_callback( - cls, callback: Callable[[ParameterBase, Any], None] | None - ) -> None: - """ - Set (or clear, if None) a single global callback that will be called - after *any* ParameterBase instance changes value. - The callback must accept two arguments: - - The ParameterBase instance whose value changed - - The new value of that parameter - """ - cls.global_value_changed_callback = callback - class GetLatest(DelegateAttributes): """ diff --git a/tests/parameter/test_value_changed_callback.py b/tests/parameter/test_value_changed_callback.py index 8b9904ca15da..0fde2410f735 100644 --- a/tests/parameter/test_value_changed_callback.py +++ b/tests/parameter/test_value_changed_callback.py @@ -26,7 +26,7 @@ def _reset_callback() -> "Generator[None, None, None]": """Reset the callback after each test""" yield - ParameterBase.set_global_value_changed_callback(None) + ParameterBase.global_on_set_callback = None @pytest.fixture() # type: ignore[misc] @@ -35,7 +35,7 @@ def basic_parameter( ) -> Parameter: """Fixture providing a basic parameter with callback""" param = Parameter("test_param", set_cmd=None, get_cmd=None) - ParameterBase.set_global_value_changed_callback(basic_callback) + ParameterBase.global_on_set_callback = basic_callback return param @@ -109,12 +109,12 @@ def test_set_global_callback(self) -> None: def test_callback(p: ParameterBase, value: Any) -> None: captured.append((p, value)) - ParameterBase.set_global_value_changed_callback(test_callback) + ParameterBase.global_on_set_callback = test_callback param(1) assert len(captured) == 1 assert captured[0] == (param, 1) - ParameterBase.set_global_value_changed_callback(None) + ParameterBase.global_on_set_callback = None param(2) assert len(captured) == 1 @@ -141,7 +141,7 @@ def test_callback_with_different_validators( ) -> None: """Test callback behavior with different validator types""" param = Parameter("test_param", set_cmd=None, get_cmd=None, vals=validator) - ParameterBase.set_global_value_changed_callback(basic_callback) + ParameterBase.global_on_set_callback = basic_callback with pytest.raises(ValueError) if not should_callback else nullcontext(): param(test_input) @@ -159,7 +159,7 @@ def failing_callback(param: ParameterBase, value: Any) -> None: raise RuntimeError("Intentional failure") param = Parameter("test_param", set_cmd=None, get_cmd=None) - ParameterBase.set_global_value_changed_callback(failing_callback) + ParameterBase.global_on_set_callback = failing_callback param(DEFAULT_VALUE) assert param() == DEFAULT_VALUE @@ -199,7 +199,7 @@ def thread_safe_callback(param: ParameterBase, value: Any) -> None: captured_values.append(value) param = Parameter("test_param", set_cmd=None, get_cmd=None) - ParameterBase.set_global_value_changed_callback(thread_safe_callback) + ParameterBase.global_on_set_callback = thread_safe_callback threads = [ threading.Thread( @@ -243,7 +243,7 @@ def test_callback_with_steps( step=STEP_SIZE, initial_value=START_VALUE, ) - ParameterBase.set_global_value_changed_callback(basic_callback) + ParameterBase.global_on_set_callback = basic_callback param(TARGET_VALUE) @@ -255,16 +255,14 @@ def test_callback_with_steps( f"Expected steps {expected_steps}, got {actual_values}" ) - def test_nested_callbacks( - self, - ) -> None: + def test_nested_callbacks(self) -> None: """Test nested callback behavior""" param = Parameter("test_param", set_cmd=None, get_cmd=None) def callback(param: ParameterBase, value: Any) -> None: param.cache.set(value) - ParameterBase.set_global_value_changed_callback(callback) + ParameterBase.global_on_set_callback = callback param(1) assert param.cache.get() == 1 @@ -280,10 +278,32 @@ def timing_callback(param: ParameterBase, value: Any) -> None: captured_times.append(time.time() - start_time) basic_parameter.post_delay = DELAY_TIME - ParameterBase.set_global_value_changed_callback(timing_callback) + ParameterBase.global_on_set_callback = timing_callback basic_parameter(1) basic_parameter(2) assert len(captured_times) == 2 assert captured_times[1] - captured_times[0] >= DELAY_TIME + + +def test_set_callback_for_instance( + basic_callback: "Callable[[ParameterBase, Any], None]", + captured_params: list[tuple[ParameterBase, Any]], +): + param_a = Parameter("test_param_a", set_cmd=None, get_cmd=None) + param_b = Parameter("test_param_b", set_cmd=None, get_cmd=None) + captured_instance_params = [] + + def callback(param: ParameterBase, val): + if param.global_on_set_callback: + param.global_on_set_callback(val) + captured_instance_params.append(val) + + ParameterBase.global_on_set_callback = basic_callback + param_a.on_set_callback = callback + param_a(1) + param_b(2) + + assert captured_params == [(param_a, 1), (param_b, 2)] + assert captured_instance_params == [1] From 5261a2cafab4096198da300ad87ab20ea6651c57 Mon Sep 17 00:00:00 2001 From: Jinnapat Indrapiromkul Date: Fri, 28 Mar 2025 14:34:14 +0100 Subject: [PATCH 13/15] Rename test file --- src/qcodes/parameters/parameter_base.py | 1 - ...lue_changed_callback.py => test_parameter_on_set_callback.py} | 0 2 files changed, 1 deletion(-) rename tests/parameter/{test_value_changed_callback.py => test_parameter_on_set_callback.py} (100%) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index 2e73b0174f1f..35a32b9e20fd 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -377,7 +377,6 @@ def _build__doc__(self) -> str | None: @property def vals(self) -> Validator | None: """ - N The first validator of the parameter. None if no validators are set for this parameter. diff --git a/tests/parameter/test_value_changed_callback.py b/tests/parameter/test_parameter_on_set_callback.py similarity index 100% rename from tests/parameter/test_value_changed_callback.py rename to tests/parameter/test_parameter_on_set_callback.py From ac021a34044acf99bcc5afac7d2d1c93ce6e474c Mon Sep 17 00:00:00 2001 From: Jinnapat Indrapiromkul Date: Fri, 28 Mar 2025 17:05:58 +0100 Subject: [PATCH 14/15] modify callback call --- src/qcodes/parameters/parameter_base.py | 4 ++-- tests/parameter/test_parameter_on_set_callback.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index 35a32b9e20fd..2779725d12dd 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -794,8 +794,8 @@ def _call_on_set_callback(self, value: NumberType | Sized) -> None: try: if self.on_set_callback is not None: self.on_set_callback(self, value) - elif self.global_on_set_callback is not None: - self.global_on_set_callback(value) + elif self.__class__.global_on_set_callback is not None: + self.__class__.global_on_set_callback(self, value) except Exception as e: LOG.warning( f"Exception {e} in on set callback " diff --git a/tests/parameter/test_parameter_on_set_callback.py b/tests/parameter/test_parameter_on_set_callback.py index 0fde2410f735..951b8973a148 100644 --- a/tests/parameter/test_parameter_on_set_callback.py +++ b/tests/parameter/test_parameter_on_set_callback.py @@ -296,8 +296,8 @@ def test_set_callback_for_instance( captured_instance_params = [] def callback(param: ParameterBase, val): - if param.global_on_set_callback: - param.global_on_set_callback(val) + if ParameterBase.global_on_set_callback: + ParameterBase.global_on_set_callback(param, val) captured_instance_params.append(val) ParameterBase.global_on_set_callback = basic_callback From c9624714f73593be94824e4046981439b46b0639 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Thu, 3 Apr 2025 10:30:58 +0200 Subject: [PATCH 15/15] Use ParamDataType for callback type --- src/qcodes/parameters/parameter_base.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index 2779725d12dd..157f74d16f89 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -192,7 +192,9 @@ class ParameterBase(MetadatableWithName): """ - global_on_set_callback: ClassVar[Callable[[ParameterBase, Any], None] | None] = None + global_on_set_callback: ClassVar[ + Callable[[ParameterBase, ParamDataType], None] | None + ] = None def __init__( self, @@ -215,7 +217,7 @@ def __init__( abstract: bool | None = False, bind_to_instrument: bool = True, register_name: str | None = None, - on_set_callback: Callable[[ParameterBase, Any], None] | None = None, + on_set_callback: Callable[[ParameterBase, ParamDataType], None] | None = None, ) -> None: super().__init__(metadata) if not str(name).isidentifier(): @@ -790,7 +792,7 @@ def set_wrapper(value: ParamDataType, **kwargs: Any) -> None: return set_wrapper - def _call_on_set_callback(self, value: NumberType | Sized) -> None: + def _call_on_set_callback(self, value: ParamDataType) -> None: try: if self.on_set_callback is not None: self.on_set_callback(self, value)