diff --git a/src/qcodes/parameters/command.py b/src/qcodes/parameters/command.py index 18815f6beb4..81459430aa4 100644 --- a/src/qcodes/parameters/command.py +++ b/src/qcodes/parameters/command.py @@ -144,7 +144,7 @@ def __init__( else: raise TypeError( - f"cmd must be a string or function with arg_count={arg_count} args" + f"cmd must be a string or function with arg_count={arg_count} args not {cmd} of type {type(cmd)}" ) # Wrappers that may or may not be used in constructing call diff --git a/src/qcodes/parameters/parameter.py b/src/qcodes/parameters/parameter.py index 29565201fe9..bff2605a7bd 100644 --- a/src/qcodes/parameters/parameter.py +++ b/src/qcodes/parameters/parameter.py @@ -5,16 +5,16 @@ import logging import os +from collections.abc import Callable from types import MethodType -from typing import TYPE_CHECKING, Any, Literal +from typing import TYPE_CHECKING, Any, Literal, cast + +from qcodes.utils import is_function -from .command import Command from .parameter_base import ParamDataType, ParameterBase, ParamRawDataType from .sweep_values import SweepFixedValues if TYPE_CHECKING: - from collections.abc import Callable - from qcodes.instrument.base import InstrumentBase from qcodes.logger.instrument_logger import InstrumentLoggerAdapter from qcodes.validators import Validator @@ -23,6 +23,129 @@ log = logging.getLogger(__name__) +def _set_parameter_factory( + function: Callable[[str], ParamRawDataType] | None, + cmd: str | Callable[[ParamRawDataType], None] | None, + parameter_name: str, +) -> Callable[[Parameter, ParamRawDataType], None]: + if cmd is None: + + def _set_manual_parameter( + self: Parameter, x: ParamRawDataType + ) -> ParamRawDataType: + if self.root_instrument is not None: + mylogger: InstrumentLoggerAdapter | logging.Logger = ( + self.root_instrument.log + ) + else: + mylogger = log + mylogger.debug( + "Setting raw value of parameter: %s to %s", self.full_name, x + ) + self.cache._set_from_raw_value(x) + return x + + return _set_manual_parameter + elif isinstance(cmd, str) and is_function(function, 1): + # cast is safe since we just checked this above using is_function + function = cast(Callable[[str], ParamRawDataType], function) + + def set_parameter_write(self: Parameter, *args: Any) -> None: + # for some reason mypy does not understand + # that cmd is a str even if this is defined inside + # an if isinstance block + assert isinstance(cmd, str) + # TODO it is possible to format str with more than one arg. + # this does not seem to have been tested + formatted_cmd = cmd.format(*args) + return function(formatted_cmd) + + return set_parameter_write + + elif is_function(cmd, 1): + # cast is safe since we just checked this above using is_function + cmd = cast(Callable[[ParamRawDataType], None], cmd) + + def set_parameter_func(self: Parameter, value: ParamRawDataType) -> None: + return cmd(value) + + return set_parameter_func + + elif isinstance(cmd, str) and function is None: + raise TypeError( + f"Cannot use a str set_cmd without " + f"binding to an instrument. " + f"Got: set_cmd {cmd} for parameter {parameter_name}" + ) + else: + raise TypeError( + "Unexpected options for parameter set. " + f"Got: set_cmd {cmd} for parameter {parameter_name}" + ) + + +def _get_parameter_factory( + function: Callable[[str], ParamRawDataType] | None, + cmd: str | Callable[[], ParamRawDataType] | None, + parameter_name: str, +) -> Callable[[Parameter], ParamRawDataType]: + if cmd is None: + + def get_manual_parameter(self: Parameter) -> ParamRawDataType: + if self.root_instrument is not None: + mylogger: InstrumentLoggerAdapter | logging.Logger = ( + self.root_instrument.log + ) + else: + mylogger = log + mylogger.debug( + "Getting raw value of parameter: %s as %s", + self.full_name, + self.cache.raw_value, + ) + return self.cache.raw_value + + return get_manual_parameter + + elif isinstance(cmd, str) and is_function(function, 1): + # cast is safe since we just checked this above using is_function + function = cast(Callable[[str], ParamRawDataType], function) + + def get_parameter_ask(self: Parameter, *args: Any) -> ParamRawDataType: + # for some reason mypy does not understand + # that cmd is a str even if this is defined inside + # an if isinstance block + assert isinstance(cmd, str) + # TODO it is possible to format str with additional args. + # this does not seem to have been tested + formatted_cmd = cmd.format(*args) + return function(formatted_cmd) + + return get_parameter_ask + + elif is_function(cmd, 0): + # cast is safe since we just checked this above using is_function + cmd = cast(Callable[[], ParamRawDataType], cmd) + + def get_parameter_func(self: Parameter) -> ParamRawDataType: + return cmd() + + return get_parameter_func + + elif isinstance(cmd, str) and function is None: + raise TypeError( + f"Cannot use a str get_cmd without " + f"binding to an instrument. " + f"Got: get_cmd {cmd} for parameter {parameter_name}" + ) + + else: + raise TypeError( + "Unexpected options for parameter get. " + f"Got: get_cmd {cmd} for parameter {parameter_name}" + ) + + class Parameter(ParameterBase): """ A parameter represents a single degree of freedom. Most often, @@ -174,7 +297,7 @@ def __init__( instrument: InstrumentBase | None = None, label: str | None = None, unit: str | None = None, - get_cmd: str | Callable[..., Any] | Literal[False] | None = None, + get_cmd: str | Callable[[], ParamRawDataType] | Literal[False] | None = None, set_cmd: str | Callable[..., Any] | Literal[False] | None = False, initial_value: float | str | None = None, max_val_age: float | None = None, @@ -184,35 +307,6 @@ def __init__( bind_to_instrument: bool = True, **kwargs: Any, ) -> None: - def _get_manual_parameter(self: Parameter) -> ParamRawDataType: - if self.root_instrument is not None: - mylogger: InstrumentLoggerAdapter | logging.Logger = ( - self.root_instrument.log - ) - else: - mylogger = log - mylogger.debug( - "Getting raw value of parameter: %s as %s", - self.full_name, - self.cache.raw_value, - ) - return self.cache.raw_value - - def _set_manual_parameter( - self: Parameter, x: ParamRawDataType - ) -> ParamRawDataType: - if self.root_instrument is not None: - mylogger: InstrumentLoggerAdapter | logging.Logger = ( - self.root_instrument.log - ) - else: - mylogger = log - mylogger.debug( - "Setting raw value of parameter: %s to %s", self.full_name, x - ) - self.cache._set_from_raw_value(x) - return x - if instrument is not None and bind_to_instrument: existing_parameter = instrument.parameters.get(name, None) @@ -264,62 +358,37 @@ def _set_manual_parameter( if self.gettable 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." + " that already implements get_raw is an error." ) 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] - else: - if isinstance(get_cmd, str) and instrument is None: - raise TypeError( - f"Cannot use a str get_cmd without " - f"binding to an instrument. " - f"Got: get_cmd {get_cmd} for parameter {name}" - ) + exec_str_ask: Callable[[str], ParamRawDataType] | None = ( + getattr(instrument, "ask", None) if instrument else None + ) + # ignore typeerror since mypy does not allow setting a method dynamically + self.get_raw = MethodType( # type: ignore[method-assign] + _get_parameter_factory(exec_str_ask, cmd=get_cmd, parameter_name=name), + self, + ) - exec_str_ask = getattr(instrument, "ask", None) if instrument else None - # TODO get_raw should also be a method here. This should probably be done by wrapping - # it with MethodType like above - # ignore typeerror since mypy does not allow setting a method dynamically - self.get_raw = Command( # type: ignore[method-assign] - arg_count=0, - cmd=get_cmd, - exec_str=exec_str_ask, - ) self._gettable = True - # mypy resolves the type of self.get_raw to object here. - # this may be resolvable if Command above is correctly wrapped in MethodType - self.get = self._wrap_get(self.get_raw) # type: ignore[arg-type] + self.get = self._wrap_get(self.get_raw) 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." + " that already implements set_raw is an error." ) 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] - else: - if isinstance(set_cmd, str) and instrument is None: - raise TypeError( - f"Cannot use a str set_cmd without " - f"binding to an instrument. " - f"Got: set_cmd {set_cmd} for parameter {name}" - ) - - exec_str_write = ( - getattr(instrument, "write", None) if instrument else None - ) - # TODO get_raw should also be a method here. This should probably be done by wrapping - # it with MethodType like above - # ignore typeerror since mypy does not allow setting a method dynamically - self.set_raw = Command( # type: ignore[assignment] - arg_count=1, cmd=set_cmd, exec_str=exec_str_write - ) + exec_str_write: Any | None = ( + getattr(instrument, "write", None) if instrument else None + ) + # ignore typeerror since mypy does not allow setting a method dynamically + self.set_raw = MethodType( # type: ignore[method-assign] + _set_parameter_factory( + exec_str_write, cmd=set_cmd, parameter_name=name + ), + self, + ) self._settable = True self.set = self._wrap_set(self.set_raw) diff --git a/tests/parameter/test_parameter_basics.py b/tests/parameter/test_parameter_basics.py index a9264ab9b0c..6944596b252 100644 --- a/tests/parameter/test_parameter_basics.py +++ b/tests/parameter/test_parameter_basics.py @@ -1,6 +1,10 @@ +import logging +from typing import TYPE_CHECKING, Any + import pytest import qcodes.validators as vals +from qcodes.instrument import Instrument from qcodes.parameters import Function, Parameter, ParameterBase, ParamRawDataType from .conftest import ( @@ -10,6 +14,27 @@ named_instrument, ) +if TYPE_CHECKING: + from collections.abc import Generator + +_LOG = logging.getLogger(__name__) + + +class LoggingInstrument(Instrument): + def ask(self, cmd: str) -> Any: + _LOG.info(f"Received ask str {cmd}") + return 1 + + def write(self, cmd: str) -> None: + _LOG.info(f"Received write str {cmd}") + + +@pytest.fixture(name="logging_instrument") +def _make_logging_instrument() -> "Generator[LoggingInstrument, None, None]": + inst = LoggingInstrument("logging_instr") + yield inst + inst.close() + def test_no_name() -> None: with pytest.raises(TypeError): @@ -265,3 +290,43 @@ def test_set_cmd_str_no_instrument_raises() -> None: TypeError, match="Cannot use a str set_cmd without binding to an instrument." ): Parameter(name="test", instrument=None, set_cmd="set_me") + + +def test_str_get_set( + logging_instrument: LoggingInstrument, caplog: pytest.LogCaptureFixture +) -> None: + logging_instrument.add_parameter( + "my_param", get_cmd="my_param?", set_cmd="my_param{}" + ) + + caplog.clear() + + with caplog.at_level(logging.INFO): + logging_instrument.my_param.get() + + assert caplog.records[0].message == "Received ask str my_param?" + + caplog.clear() + + with caplog.at_level(logging.INFO): + logging_instrument.my_param.set(100) + + assert caplog.records[0].message == "Received write str my_param100" + + +def test_str_set_multi_arg( + logging_instrument: LoggingInstrument, caplog: pytest.LogCaptureFixture +) -> None: + logging_instrument.add_parameter("my_param", get_cmd="my_param?", set_cmd="{}{}") + + with caplog.at_level(logging.INFO): + logging_instrument.my_param.get() + + assert caplog.records[0].message == "Received ask str my_param?" + + caplog.clear() + + with caplog.at_level(logging.INFO): + logging_instrument.my_param.set("this_command", 2344) + + assert caplog.records[0].message == "Received write str this_command2344"