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 all 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
2 changes: 2 additions & 0 deletions docs/changes/newsfragments/6671.improved
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix a regression introduced in 0.50.0 where a DelegateParameter initialized with a None source
would not correctly call get/set on the source parameter when this has been set.
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)
no_instrument_get = not self._implements_get_raw 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._implements_get_raw and get_cmd not in (None, False):
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._implements_get_raw 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._implements_set_raw 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._implements_set_raw 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
24 changes: 16 additions & 8 deletions src/qcodes/parameters/parameter_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,11 +263,8 @@ def __init__(
self.get_latest = GetLatest(self)

self.get: Callable[..., ParamDataType]
implements_get_raw = hasattr(self, "get_raw") and not getattr(
self.get_raw, "__qcodes_is_abstract_method__", False
)
self._gettable = False
if implements_get_raw:
if self._implements_get_raw:
self.get = self._wrap_get(self.get_raw)
self._gettable = True
elif hasattr(self, "get"):
Expand All @@ -278,11 +275,8 @@ def __init__(
)

self.set: Callable[..., None]
implements_set_raw = hasattr(self, "set_raw") and not getattr(
self.set_raw, "__qcodes_is_abstract_method__", False
)
self._settable: bool = False
if implements_set_raw:
if self._implements_set_raw:
self.set = self._wrap_set(self.set_raw)
self._settable = True
elif hasattr(self, "set"):
Expand Down Expand Up @@ -347,6 +341,20 @@ def __init__(

instrument.parameters[name] = self

@property
def _implements_get_raw(self) -> bool:
implements_get_raw = hasattr(self, "get_raw") and not getattr(
self.get_raw, "__qcodes_is_abstract_method__", False
)
return implements_get_raw

@property
def _implements_set_raw(self) -> bool:
implements_set_raw = hasattr(self, "set_raw") and not getattr(
self.set_raw, "__qcodes_is_abstract_method__", False
)
return implements_set_raw

def _build__doc__(self) -> str | None:
return self.__doc__

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