Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug: delegate parameter doesn't call its source to get value #6671

Merged
merged 3 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/qcodes/parameters/parameter.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,9 @@ def _set_manual_parameter(
**kwargs,
)

no_instrument_get = not self.gettable and (get_cmd is None or get_cmd is False)
# Note: use _gettable/_settable attributes in __init__ function.
# Avoid using gettable/settable properties here.
no_instrument_get = not self._gettable and (get_cmd is None or get_cmd is False)
# TODO: a matching check should be in ParameterBase but
# due to the current limited design the ParameterBase cannot
# know if this subclass will supply a get_cmd
Expand All @@ -261,13 +263,13 @@ def _set_manual_parameter(
# in the scope of this class.
# (previous call to `super().__init__` wraps existing
# get_raw/set_raw into get/set methods)
if self.gettable and get_cmd not in (None, False):
if self._gettable and get_cmd not in (None, False):
jenshnielsen marked this conversation as resolved.
Show resolved Hide resolved
raise TypeError(
"Supplying a not None or False `get_cmd` to a Parameter"
" that already implements"
" get_raw is an error."
)
elif not self.gettable and get_cmd is not False:
elif not self._gettable and get_cmd is not False:
if get_cmd is None:
# ignore typeerror since mypy does not allow setting a method dynamically
self.get_raw = MethodType(_get_manual_parameter, self) # type: ignore[method-assign]
Expand All @@ -293,13 +295,13 @@ def _set_manual_parameter(
# this may be resolvable if Command above is correctly wrapped in MethodType
self.get = self._wrap_get(self.get_raw) # type: ignore[arg-type]

if self.settable and set_cmd not in (None, False):
if self._settable and set_cmd not in (None, False):
raise TypeError(
"Supplying a not None or False `set_cmd` to a Parameter"
" that already implements"
" set_raw is an error."
)
elif not self.settable and set_cmd is not False:
elif not self._settable and set_cmd is not False:
if set_cmd is None:
# ignore typeerror since mypy does not allow setting a method dynamically
self.set_raw = MethodType(_set_manual_parameter, self) # type: ignore[method-assign]
Expand Down
17 changes: 17 additions & 0 deletions tests/parameter/test_delegate_parameter.py
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,23 @@ def test_delegate_cache_pristine_if_not_set() -> None:
assert gotten_delegate_cache is None


def test_delegate_get_instrument_val(numeric_val: int) -> None:
"""
Delegate should call its source to get value rather than just reading source cache
"""
initial_value = numeric_val
t = ObservableParam("observable_parameter", initial_value=initial_value)
# delegate has no source initially to make sure it's not gettable on initialization
d = DelegateParameter("delegate", source=None)
d.source = t

new_instr_value = 3
# Update instrument value without changing parameter cache
t.instr_val = new_instr_value
# This check fails if delegate only reads source cache
assert d() == new_instr_value


def test_delegate_get_updates_cache(
make_observable_parameter: Callable[..., ObservableParam], numeric_val: int
) -> None:
Expand Down
Loading