From 1b3042154b3814d4ac7514bd838d4d18112aa8bf Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Fri, 17 May 2024 16:52:54 +0200 Subject: [PATCH 1/5] Deprecate rarely used convenience methods on instrument class The aim is to reduce the number of ways in which the same operation can be performed --- src/qcodes/instrument/instrument_base.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/qcodes/instrument/instrument_base.py b/src/qcodes/instrument/instrument_base.py index b7661c19dce..1209b893e08 100644 --- a/src/qcodes/instrument/instrument_base.py +++ b/src/qcodes/instrument/instrument_base.py @@ -660,6 +660,10 @@ 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: @@ -667,6 +671,10 @@ def __getitem__(self, key: str) -> Callable[..., Any] | Parameter: 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. From 862c8478b7ebc82319d777289aa089d528d63585 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Fri, 17 May 2024 16:54:35 +0200 Subject: [PATCH 2/5] Avoid using deprecated instrument attributes --- .../DataSet/Offline Plotting Tutorial.ipynb | 2 +- src/qcodes/instrument/instrument.py | 2 +- .../instrument_drivers/tektronix/AWG5014.py | 26 +++++++++---------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/docs/examples/DataSet/Offline Plotting Tutorial.ipynb b/docs/examples/DataSet/Offline Plotting Tutorial.ipynb index cb41afb5393..a347f62f972 100644 --- a/docs/examples/DataSet/Offline Plotting Tutorial.ipynb +++ b/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, diff --git a/src/qcodes/instrument/instrument.py b/src/qcodes/instrument/instrument.py index 7dc83e91e0b..def6e8aca0c 100644 --- a/src/qcodes/instrument/instrument.py +++ b/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 = ( diff --git a/src/qcodes/instrument_drivers/tektronix/AWG5014.py b/src/qcodes/instrument_drivers/tektronix/AWG5014.py index fd8c190abcb..b2190252c62 100644 --- a/src/qcodes/instrument_drivers/tektronix/AWG5014.py +++ b/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 From 7bb56797f607ee45731efca872750a0afa09d607 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Fri, 17 May 2024 20:38:00 +0200 Subject: [PATCH 3/5] silence deprecation warning --- tests/test_instrument.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/test_instrument.py b/tests/test_instrument.py index 7fd88152e32..84cdc187e14 100644 --- a/tests/test_instrument.py +++ b/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: From 1822a107c8ee2e8facf9b625b736dc563fbf4db4 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Fri, 17 May 2024 20:42:53 +0200 Subject: [PATCH 4/5] Update test to use public driver --- tests/drivers/test_tektronix_AWG5014C.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/drivers/test_tektronix_AWG5014C.py b/tests/drivers/test_tektronix_AWG5014C.py index 7d01ff69815..f30d4f87ecf 100644 --- a/tests/drivers/test_tektronix_AWG5014C.py +++ b/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, From f02bdc2d180423e7f81384c9edf7cf986ecf80f9 Mon Sep 17 00:00:00 2001 From: "Jens H. Nielsen" Date: Tue, 24 Sep 2024 13:41:20 +0200 Subject: [PATCH 5/5] Add changelog for 6086 --- docs/changes/newsfragments/6086.breaking | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 docs/changes/newsfragments/6086.breaking diff --git a/docs/changes/newsfragments/6086.breaking b/docs/changes/newsfragments/6086.breaking new file mode 100644 index 00000000000..0045d05966d --- /dev/null +++ b/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.