Make command caller get/set_raw a method

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

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)
 

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"