Merge pull request #6086 from jenshnielsen/deprecate_getters

Deprecate convenience methods on Instrument classes

Author: jenshnielsen

docs/changes/newsfragments/6086.breaking

@@ -0,0 +1,3 @@
+The methods `get`, `set`, `call` and `__getitem__` on the `InstrumentBase` class have been deprecated.
+Parameters can be looked up by name using the `Instrument.parameters` dict and functions using `instrument.functions`
+which is cleaner and fully equivalent.

docs/examples/DataSet/Offline Plotting Tutorial.ipynb

@@ -977,7 +977,7 @@
    "name": "python",
    "nbconvert_exporter": "python",
    "pygments_lexer": "ipython3",
-   "version": "3.12.3"
+   "version": "3.12.4"
   },
   "toc": {
    "base_numbering": 1,

src/qcodes/instrument/instrument.py

@@ -129,7 +129,7 @@ def connect_message(
         # start with an empty dict, just in case an instrument doesn't
         # heed our request to return all 4 fields.
         idn = {"vendor": None, "model": None, "serial": None, "firmware": None}
-        idn.update(self.get(idn_param))
+        idn.update(self.parameters[idn_param].get())
         t = time.time() - (begin_time or self._t0)
 
         con_msg = (

src/qcodes/instrument/instrument_base.py

@@ -660,13 +660,21 @@ def _is_abstract(self) -> bool:
     #
     delegate_attr_dicts: ClassVar[list[str]] = ["parameters", "functions", "submodules"]
 
+    @deprecated(
+        "Use attributes directly on the instrument object instead.",
+        category=QCoDeSDeprecationWarning,
+    )
     def __getitem__(self, key: str) -> Callable[..., Any] | Parameter:
         """Delegate instrument['name'] to parameter or function 'name'."""
         try:
             return self.parameters[key]
         except KeyError:
             return self.functions[key]
 
+    @deprecated(
+        "Call set directly on the parameter.",
+        category=QCoDeSDeprecationWarning,
+    )
     def set(self, param_name: str, value: Any) -> None:
         """
         Shortcut for setting a parameter from its name and new value.
@@ -677,6 +685,10 @@ def set(self, param_name: str, value: Any) -> None:
         """
         self.parameters[param_name].set(value)
 
+    @deprecated(
+        "Call get directly on the parameter.",
+        category=QCoDeSDeprecationWarning,
+    )
     def get(self, param_name: str) -> Any:
         """
         Shortcut for getting a parameter from its name.
@@ -689,6 +701,10 @@ def get(self, param_name: str) -> Any:
         """
         return self.parameters[param_name].get()
 
+    @deprecated(
+        "Call the function directly.",
+        category=QCoDeSDeprecationWarning,
+    )
     def call(self, func_name: str, *args: Any) -> Any:
         """
         Shortcut for calling a function from its name.

src/qcodes/instrument_drivers/tektronix/AWG5014.py

@@ -491,8 +491,8 @@ def __init__(
                     get_parser=float,
                 )
 
-        self.set("trigger_impedance", 50)
-        if self.get("clock_freq") != 1e9:
+        self.trigger_impedance.set(50)
+        if self.clock_freq.get() != 1e9:
             log.info("AWG clock freq not set to 1GHz")
 
         self.connect_message()
@@ -657,12 +657,12 @@ def all_channels_on(self) -> None:
         defined waveforms can be ON.
         """
         for i in range(1, self.num_channels + 1):
-            self.set(f"ch{i}_state", 1)
+            self.parameters[f"ch{i}_state"].set(1)
 
     def all_channels_off(self) -> None:
         """Set the state of all channels to be OFF."""
         for i in range(1, self.num_channels + 1):
-            self.set(f"ch{i}_state", 0)
+            self.parameters[f"ch{i}_state"].set(0)
 
     #####################
     # Sequences section #
@@ -947,7 +947,7 @@ def generate_sequence_cfg(self) -> dict[str, float]:
         log.info("Generating sequence_cfg")
 
         AWG_sequence_cfg = {
-            "SAMPLING_RATE": self.get("clock_freq"),
+            "SAMPLING_RATE": self.clock_freq.get(),
             "CLOCK_SOURCE": (
                 1 if self.clock_source().startswith("INT") else 2
             ),  # Internal | External
@@ -957,27 +957,27 @@ def generate_sequence_cfg(self) -> dict[str, float]:
             "EXTERNAL_REFERENCE_TYPE": 1,  # Fixed | Variable
             "REFERENCE_CLOCK_FREQUENCY_SELECTION": 1,
             # 10 MHz | 20 MHz | 100 MHz
-            "TRIGGER_SOURCE": 1 if self.get("trigger_source").startswith("EXT") else 2,
+            "TRIGGER_SOURCE": 1 if self.trigger_source.get().startswith("EXT") else 2,
             # External | Internal
             "TRIGGER_INPUT_IMPEDANCE": (
-                1 if self.get("trigger_impedance") == 50.0 else 2
+                1 if self.trigger_impedance.get() == 50.0 else 2
             ),  # 50 ohm | 1 kohm
             "TRIGGER_INPUT_SLOPE": (
-                1 if self.get("trigger_slope").startswith("POS") else 2
+                1 if self.trigger_slope.get().startswith("POS") else 2
             ),  # Positive | Negative
             "TRIGGER_INPUT_POLARITY": (
                 1 if self.ask("TRIGger:POLarity?").startswith("POS") else 2
             ),  # Positive | Negative
-            "TRIGGER_INPUT_THRESHOLD": self.get("trigger_level"),  # V
+            "TRIGGER_INPUT_THRESHOLD": self.trigger_level.get(),  # V
             "EVENT_INPUT_IMPEDANCE": (
-                1 if self.get("event_impedance") == 50.0 else 2
+                1 if self.event_impedance.get() == 50.0 else 2
             ),  # 50 ohm | 1 kohm
             "EVENT_INPUT_POLARITY": (
-                1 if self.get("event_polarity").startswith("POS") else 2
+                1 if self.event_polarity.get().startswith("POS") else 2
             ),  # Positive | Negative
-            "EVENT_INPUT_THRESHOLD": self.get("event_level"),  # V
+            "EVENT_INPUT_THRESHOLD": self.event_level(),  # V
             "JUMP_TIMING": (
-                1 if self.get("event_jump_timing").startswith("SYNC") else 2
+                1 if self.event_jump_timing.get().startswith("SYNC") else 2
             ),  # Sync | Async
             "RUN_MODE": 4,  # Continuous | Triggered | Gated | Sequence
             "RUN_STATE": 0,  # On | Off

tests/drivers/test_tektronix_AWG5014C.py

@@ -1,12 +1,12 @@
 import numpy as np
 import pytest
 
-from qcodes.instrument_drivers.tektronix.AWG5014 import Tektronix_AWG5014
+from qcodes.instrument_drivers.tektronix import TektronixAWG5014
 
 
 @pytest.fixture(scope="function")
 def awg():
-    awg_sim = Tektronix_AWG5014(
+    awg_sim = TektronixAWG5014(
         "awg_sim",
         address="GPIB0::1::INSTR",
         timeout=1,

tests/test_instrument.py

@@ -31,6 +31,7 @@
 )
 from qcodes.metadatable import Metadatable
 from qcodes.parameters import Function, Parameter
+from qcodes.utils import QCoDeSDeprecationWarning
 
 if TYPE_CHECKING:
     from collections.abc import Iterator
@@ -273,12 +274,20 @@ def test_add_remove_f_p(testdummy) -> None:
     testdummy.add_function("dac1", call_cmd="foo")
 
     # test custom __get_attr__ for functions
-    fcn = testdummy["function"]
-    assert isinstance(fcn, Function)
+    with pytest.warns(
+        QCoDeSDeprecationWarning,
+        match="Use attributes directly on the instrument object instead",
+    ):
+        fcn = testdummy["function"]
+        assert isinstance(fcn, Function)
     # by design, one gets the parameter if a function exists
     # and has same name
-    dac1 = testdummy["dac1"]
-    assert isinstance(dac1, Parameter)
+    with pytest.warns(
+        QCoDeSDeprecationWarning,
+        match="Use attributes directly on the instrument object instead",
+    ):
+        dac1 = testdummy["dac1"]
+        assert isinstance(dac1, Parameter)
 
 
 def test_instances(testdummy, parabola) -> None: