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

Deprecate convenience methods on Instrument classes #6086

Merged
merged 5 commits into from
Sep 25, 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
3 changes: 3 additions & 0 deletions docs/changes/newsfragments/6086.breaking
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion docs/examples/DataSet/Offline Plotting Tutorial.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.12.3"
"version": "3.12.4"
},
"toc": {
"base_numbering": 1,
Expand Down
2 changes: 1 addition & 1 deletion src/qcodes/instrument/instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand Down
16 changes: 16 additions & 0 deletions src/qcodes/instrument/instrument_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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.
Expand Down
26 changes: 13 additions & 13 deletions src/qcodes/instrument_drivers/tektronix/AWG5014.py
Original file line number Diff line number Diff line change
Expand Up @@ -491,8 +491,8 @@
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()
Expand Down Expand Up @@ -657,12 +657,12 @@
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)

Check warning on line 660 in src/qcodes/instrument_drivers/tektronix/AWG5014.py

View check run for this annotation

Codecov / codecov/patch

src/qcodes/instrument_drivers/tektronix/AWG5014.py#L660

Added line #L660 was not covered by tests

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)

Check warning on line 665 in src/qcodes/instrument_drivers/tektronix/AWG5014.py

View check run for this annotation

Codecov / codecov/patch

src/qcodes/instrument_drivers/tektronix/AWG5014.py#L665

Added line #L665 was not covered by tests

#####################
# Sequences section #
Expand Down Expand Up @@ -947,7 +947,7 @@
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
Expand All @@ -957,27 +957,27 @@
"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
Expand Down
4 changes: 2 additions & 2 deletions tests/drivers/test_tektronix_AWG5014C.py
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
17 changes: 13 additions & 4 deletions tests/test_instrument.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
Loading