diff --git a/docs/changes/newsfragments/8678.improved_driver b/docs/changes/newsfragments/8678.improved_driver new file mode 100644 index 000000000000..04c48a003eba --- /dev/null +++ b/docs/changes/newsfragments/8678.improved_driver @@ -0,0 +1,9 @@ +``CopperMountainM5xxx.averages``, ``AimTTiChannel.curr_range``, ``RigolDG1062Burst.ncycles``, +``Keithley7510Sense.average_count`` and the ``dwell``, ``pause``, ``curve_number`` and +``excitation_range_number`` of ``LakeshoreModel372Channel`` now reject non-integer values. +Previously these were sent to the instrument, or for ``CopperMountainM5xxx.averages`` silently +truncated. Whole-number floats such as ``2.0`` are still accepted and sent as integers. +Also removed the incorrect unit of ``AimTTiChannel.curr_range``, which is a range number, fixed +the label of ``LakeshoreModel372Channel.temperature_coefficient``, and removed incorrect units +from ``RigolDS1074Z.trigger_mode``, ``waveform_npoints``, ``waveform_yorigin`` and +``waveform_yref``. diff --git a/src/qcodes/instrument_drivers/AimTTi/_AimTTi_PL_P.py b/src/qcodes/instrument_drivers/AimTTi/_AimTTi_PL_P.py index f30fdbdbb7c7..36b7d55165fc 100644 --- a/src/qcodes/instrument_drivers/AimTTi/_AimTTi_PL_P.py +++ b/src/qcodes/instrument_drivers/AimTTi/_AimTTi_PL_P.py @@ -80,14 +80,15 @@ def __init__( ) """Parameter curr""" - self.curr_range: Parameter = self.add_parameter( + self.curr_range: Parameter[int, Self] = self.add_parameter( "curr_range", get_cmd=f"IRANGE{channel}?", get_parser=int, set_cmd=self._set_current_range, label="Current Range", - unit="A", - vals=vals.Numbers(1, 2), + # accept whole-number floats such as 2.0 but send them as int + vals=vals.PermissiveInts(1, 2), + set_parser=round, docstring="Set the current range of the output. " "Here, the integer 1 is for the Low range, " "and integer 2 is for the High range.", diff --git a/src/qcodes/instrument_drivers/CopperMountain/_M5xxx.py b/src/qcodes/instrument_drivers/CopperMountain/_M5xxx.py index 6cb6b0c966ef..41a496ab307c 100644 --- a/src/qcodes/instrument_drivers/CopperMountain/_M5xxx.py +++ b/src/qcodes/instrument_drivers/CopperMountain/_M5xxx.py @@ -11,7 +11,7 @@ ParamRawDataType, create_on_off_val_mapping, ) -from qcodes.validators import Bool, Enum, Ints, Numbers +from qcodes.validators import Bool, Enum, Ints, Numbers, PermissiveInts if TYPE_CHECKING: from typing import Unpack @@ -114,17 +114,18 @@ def __init__( ) """Turns the averaging trigger function ON/OFF.""" - self.averages: Parameter = self.add_parameter( + self.averages: Parameter[int, Self] = self.add_parameter( "averages", label="Averages", get_cmd="SENS1:AVER:COUN?", set_cmd="SENS1:AVER:COUN {}", get_parser=int, - set_parser=int, + set_parser=round, unit="", docstring="Sets or reads out the averaging factor " "when the averaging function is turned on.", - vals=Numbers(min_value=1, max_value=999), + # accept whole-number floats such as 2.0 but send them as int + vals=PermissiveInts(min_value=1, max_value=999), ) """Sets or reads out the averaging factor when the averaging function is turned on.""" diff --git a/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py b/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py index 3be951318c60..ab5bbe5d6a29 100644 --- a/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py +++ b/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py @@ -17,7 +17,7 @@ create_on_off_val_mapping, invert_val_mapping, ) -from qcodes.validators import Arrays, Enum, Ints, Lists, Numbers +from qcodes.validators import Arrays, Enum, Ints, Lists, Numbers, PermissiveInts if TYPE_CHECKING: from collections.abc import Sequence @@ -589,7 +589,9 @@ def __init__( get_cmd=f":SENSe:{self._proper_function}:AVERage:COUNt?", set_cmd=f":SENSe:{self._proper_function}:AVERage:COUNt {{}}", get_parser=int, - vals=Numbers(1, 100), + # accept whole-number floats such as 10.0 but send them as int + vals=PermissiveInts(1, 100), + set_parser=round, docstring="Set the number of measurements that are averaged when " "filtering is enabled.", ) diff --git a/src/qcodes/instrument_drivers/Lakeshore/Lakeshore_model_372.py b/src/qcodes/instrument_drivers/Lakeshore/Lakeshore_model_372.py index b2d29d6b40b4..f975f451eff7 100644 --- a/src/qcodes/instrument_drivers/Lakeshore/Lakeshore_model_372.py +++ b/src/qcodes/instrument_drivers/Lakeshore/Lakeshore_model_372.py @@ -279,7 +279,9 @@ def __init__( docstring="Specifies a value for the autoscanning dwell time.", unit="s", get_parser=int, - vals=vals.Numbers(1, 200), + # accept whole-number floats such as 10.0 but send them as int + vals=vals.PermissiveInts(1, 200), + set_parser=round, parameter_class=GroupParameter, ) """Specifies a value for the autoscanning dwell time.""" @@ -289,7 +291,8 @@ def __init__( docstring="Specifies a value for the change pause time", unit="s", get_parser=int, - vals=vals.Numbers(3, 200), + vals=vals.PermissiveInts(3, 200), + set_parser=round, parameter_class=GroupParameter, ) """Specifies a value for the change pause time""" @@ -301,7 +304,8 @@ def __init__( "curves. Do not change this parameter " "unless you know what you are doing.", get_parser=int, - vals=vals.Numbers(0, 59), + vals=vals.PermissiveInts(0, 59), + set_parser=round, parameter_class=GroupParameter, ) """ @@ -310,7 +314,7 @@ def __init__( """ self.temperature_coefficient: GroupParameter = self.add_parameter( "temperature_coefficient", - label="Change pause time", + label="Temperature coefficient", docstring="Sets the temperature coefficient that " "will be used for temperature control if " "no curve is selected (negative or " @@ -363,7 +367,8 @@ def __init__( "current excitation); refer to the manual " "for the table of ranges", get_parser=int, - vals=vals.Numbers(1, 22), + vals=vals.PermissiveInts(1, 22), + set_parser=round, parameter_class=GroupParameter, ) """ diff --git a/src/qcodes/instrument_drivers/rigol/Rigol_DG1062.py b/src/qcodes/instrument_drivers/rigol/Rigol_DG1062.py index edfce3d5653b..314166499b68 100644 --- a/src/qcodes/instrument_drivers/rigol/Rigol_DG1062.py +++ b/src/qcodes/instrument_drivers/rigol/Rigol_DG1062.py @@ -76,7 +76,9 @@ def __init__( set_cmd=f":SOUR{channel}:BURS:NCYC {{}}", # returned in scientific notation, e.g. 1.000000E+01 get_parser=lambda s: int(float(s)), - vals=vals.Numbers(min_value=1, max_value=500000), + # accept whole-number floats such as 10.0 but send them as int + vals=vals.PermissiveInts(min_value=1, max_value=500000), + set_parser=round, ) """Parameter ncycles""" diff --git a/src/qcodes/instrument_drivers/rigol/Rigol_DS1074Z.py b/src/qcodes/instrument_drivers/rigol/Rigol_DS1074Z.py index 9403baab7acc..4fa46ad0dd07 100644 --- a/src/qcodes/instrument_drivers/rigol/Rigol_DS1074Z.py +++ b/src/qcodes/instrument_drivers/rigol/Rigol_DS1074Z.py @@ -120,13 +120,15 @@ def __init__( "waveform_npoints", get_cmd="WAV:POIN?", set_cmd="WAV:POIN {}", - unit="s", get_parser=int, ) """Parameter waveform_npoints""" self.waveform_yorigin: Parameter[float, Self] = self.add_parameter( - "waveform_yorigin", get_cmd="WAVeform:YORigin?", unit="V", get_parser=float + # vertical offset in digitizing levels + "waveform_yorigin", + get_cmd="WAVeform:YORigin?", + get_parser=float, ) """Parameter waveform_yorigin""" @@ -139,7 +141,10 @@ def __init__( """Parameter waveform_yincrem""" self.waveform_yref: Parameter[float, Self] = self.add_parameter( - "waveform_yref", get_cmd=":WAVeform:YREFerence?", unit="V", get_parser=float + # vertical reference position in digitizing levels + "waveform_yref", + get_cmd=":WAVeform:YREFerence?", + get_parser=float, ) """Parameter waveform_yref""" @@ -147,7 +152,6 @@ def __init__( "trigger_mode", get_cmd=":TRIGger:MODE?", set_cmd=":TRIGger:MODE {}", - unit="V", vals=Enum("edge", "pulse", "video", "pattern"), get_parser=str, ) diff --git a/tests/drivers/test_aimtti_pl601p.py b/tests/drivers/test_aimtti_pl601p.py index 6c5582aa6e44..df6bd6026e12 100644 --- a/tests/drivers/test_aimtti_pl601p.py +++ b/tests/drivers/test_aimtti_pl601p.py @@ -1,5 +1,6 @@ from typing import TYPE_CHECKING +import numpy as np import pytest from qcodes.instrument_drivers.AimTTi import AimTTiPL601 @@ -7,6 +8,8 @@ if TYPE_CHECKING: from collections.abc import Generator + from pytest_mock import MockerFixture + @pytest.fixture(scope="function", name="driver") def _make_driver() -> "Generator[AimTTiPL601, None, None]": @@ -25,3 +28,31 @@ def test_idn(driver: AimTTiPL601) -> None: "serial": "514710", "vendor": "THURLBY THANDAR", } == driver.IDN() + + +@pytest.mark.parametrize( + "value", [2, 2.0, np.float64(2)], ids=["int", "float", "numpy_float"] +) +def test_curr_range_accepts_whole_numbers( + driver: AimTTiPL601, mocker: "MockerFixture", value: float +) -> None: + # the range is 1 (low) or 2 (high); whole-number floats are sent as int + channel = driver.ch1 + # the output is switched off while changing the range; start from off so + # that its state does not have to be queried + channel.output.cache.set(False) + write_raw = mocker.patch.object(driver, "write_raw") + + channel.curr_range(value) # pyright: ignore[reportArgumentType] + + write_raw.assert_any_call("IRANGE1 2") + + +def test_curr_range_rejects_non_integer( + driver: AimTTiPL601, mocker: "MockerFixture" +) -> None: + write_raw = mocker.patch.object(driver, "write_raw") + + with pytest.raises(TypeError): + driver.ch1.curr_range(1.5) # pyright: ignore[reportArgumentType] + write_raw.assert_not_called() diff --git a/tests/drivers/test_coppermountain_m5065.py b/tests/drivers/test_coppermountain_m5065.py index 04b9577216a6..2d8d31091f51 100644 --- a/tests/drivers/test_coppermountain_m5065.py +++ b/tests/drivers/test_coppermountain_m5065.py @@ -1,5 +1,6 @@ from typing import TYPE_CHECKING +import numpy as np import pytest from qcodes.instrument_drivers.CopperMountain import CopperMountainM5065 @@ -7,6 +8,8 @@ if TYPE_CHECKING: from collections.abc import Generator + from pytest_mock import MockerFixture + class DummyM5065: def __init__(self, *args: object, **kwargs: object) -> None: @@ -66,6 +69,30 @@ def test_averages_param(vna: CopperMountainM5065) -> None: assert vna.averages() == 2 +@pytest.mark.parametrize("value", [3.0, np.float64(3)], ids=["float", "numpy_float"]) +def test_averages_accepts_whole_floats( + vna: CopperMountainM5065, mocker: "MockerFixture", value: float +) -> None: + # whole-number floats, e.g. from numpy, are accepted and sent as int + write_raw = mocker.spy(vna, "write_raw") + + vna.averages(value) # pyright: ignore[reportArgumentType] + + write_raw.assert_called_once_with("SENS1:AVER:COUN 3") + assert vna.averages() == 3 + + +def test_averages_rejects_non_integer( + vna: CopperMountainM5065, mocker: "MockerFixture" +) -> None: + # previously 2.5 was silently truncated to 2 by set_parser=int + write_raw = mocker.spy(vna, "write_raw") + + with pytest.raises(TypeError): + vna.averages(2.5) # pyright: ignore[reportArgumentType] + write_raw.assert_not_called() + + def test_averages_enabled_enables_averages_trigger( vna: CopperMountainM5065, ) -> None: diff --git a/tests/drivers/test_keithley_7510.py b/tests/drivers/test_keithley_7510.py index 14863896fced..06e9d0921ea6 100644 --- a/tests/drivers/test_keithley_7510.py +++ b/tests/drivers/test_keithley_7510.py @@ -1,6 +1,7 @@ from typing import TYPE_CHECKING import hypothesis.strategies as st +import numpy as np import pytest from hypothesis import HealthCheck, given, settings @@ -9,6 +10,8 @@ if TYPE_CHECKING: from collections.abc import Generator + from pytest_mock import MockerFixture + @pytest.fixture(scope="function") def dmm_7510_driver() -> "Generator[Keithley7510, None, None]": @@ -82,6 +85,30 @@ def test_average_count_is_int(dmm_7510_driver: Keithley7510) -> None: assert average_count == 25 +@pytest.mark.parametrize("value", [25.0, np.float64(25)], ids=["float", "numpy_float"]) +def test_average_count_accepts_whole_floats( + dmm_7510_driver: Keithley7510, mocker: "MockerFixture", value: float +) -> None: + # whole-number floats, e.g. from numpy, are accepted and sent as int + sense = dmm_7510_driver.sense + write_raw = mocker.spy(dmm_7510_driver, "write_raw") + + sense.average_count(value) # pyright: ignore[reportArgumentType] + + write_raw.assert_called_once_with(":SENSe:voltage:AVERage:COUNt 25") + assert sense.average_count() == 25 + + +def test_average_count_rejects_non_integer( + dmm_7510_driver: Keithley7510, mocker: "MockerFixture" +) -> None: + write_raw = mocker.spy(dmm_7510_driver, "write_raw") + + with pytest.raises(TypeError): + dmm_7510_driver.sense.average_count(2.5) # pyright: ignore[reportArgumentType] + write_raw.assert_not_called() + + def test_digitize_acq_rate_is_int(dmm_7510_driver: Keithley7510) -> None: acq_rate = dmm_7510_driver.digi_sense.acq_rate() assert isinstance(acq_rate, int) diff --git a/tests/drivers/test_lakeshore_372.py b/tests/drivers/test_lakeshore_372.py index dca80adbec8c..8a893f29784e 100644 --- a/tests/drivers/test_lakeshore_372.py +++ b/tests/drivers/test_lakeshore_372.py @@ -6,6 +6,7 @@ from contextlib import suppress from typing import TYPE_CHECKING, Any, Literal +import numpy as np import pytest from qcodes.instrument import Instrument, InstrumentBase @@ -19,6 +20,8 @@ if TYPE_CHECKING: from collections.abc import Callable, Generator + from pytest_mock import MockerFixture + log = logging.getLogger(__name__) VISA_LOGGER = f"{InstrumentBase.__module__}.com.visa" @@ -296,6 +299,47 @@ def test_setpoint(lakeshore_372: LakeshoreModel372Mock) -> None: assert h.setpoint() == setpoint +# the simulated channel 1 starts with INSET 1,100,3,0,1 and INTYPE 0,1,0,5,0,1 +@pytest.mark.parametrize( + ("name", "command"), + [ + ("dwell", "INSET 1, 1, 10, 3, 0, 1"), + ("pause", "INSET 1, 1, 100, 10, 0, 1"), + ("curve_number", "INSET 1, 1, 100, 3, 10, 1"), + ("excitation_range_number", "INTYPE 1, 0, 10, 0, 5, 0, 1"), + ], +) +@pytest.mark.parametrize("value", [10.0, np.float64(10)], ids=["float", "numpy_float"]) +def test_channel_integer_settings_accept_whole_floats( + lakeshore_372: LakeshoreModel372Mock, + mocker: MockerFixture, + name: str, + command: str, + value: float, +) -> None: + # whole-number floats, e.g. from numpy, are accepted and sent as int + param = lakeshore_372.channels[0].parameters[name] + write_raw = mocker.patch.object(lakeshore_372, "write_raw") + + param(value) + + write_raw.assert_called_once_with(command) + + +@pytest.mark.parametrize( + "name", ["dwell", "pause", "curve_number", "excitation_range_number"] +) +def test_channel_integer_settings_reject_non_integer( + lakeshore_372: LakeshoreModel372Mock, mocker: MockerFixture, name: str +) -> None: + param = lakeshore_372.channels[0].parameters[name] + write_raw = mocker.patch.object(lakeshore_372, "write_raw") + + with pytest.raises(TypeError): + param(10.5) + write_raw.assert_not_called() + + def test_select_range_limits(lakeshore_372: LakeshoreModel372Mock) -> None: h = lakeshore_372.sample_heater ranges = list(range(1, 9)) diff --git a/tests/drivers/test_rigol_dg1062.py b/tests/drivers/test_rigol_dg1062.py index 957b8d9d2ef0..1013b1e8026d 100644 --- a/tests/drivers/test_rigol_dg1062.py +++ b/tests/drivers/test_rigol_dg1062.py @@ -1,5 +1,6 @@ from typing import TYPE_CHECKING +import numpy as np import pytest from qcodes.instrument_drivers.rigol import RigolDG1062 @@ -7,6 +8,8 @@ if TYPE_CHECKING: from collections.abc import Generator + from pytest_mock import MockerFixture + @pytest.fixture(scope="function", name="dg1062") def _make_dg1062() -> "Generator[RigolDG1062, None, None]": @@ -29,6 +32,30 @@ def test_burst_ncycles_is_int(dg1062: RigolDG1062) -> None: assert ncycles == 10 +@pytest.mark.parametrize("value", [10.0, np.float64(10)], ids=["float", "numpy_float"]) +def test_burst_ncycles_accepts_whole_floats( + dg1062: RigolDG1062, mocker: "MockerFixture", value: float +) -> None: + # whole-number floats, e.g. from numpy, are accepted and sent as int + burst = dg1062.ch1.burst + write_raw = mocker.spy(dg1062, "write_raw") + + burst.ncycles(value) # pyright: ignore[reportArgumentType] + + write_raw.assert_called_once_with(":SOUR1:BURS:NCYC 10") + assert burst.ncycles() == 10 + + +def test_burst_ncycles_rejects_non_integer( + dg1062: RigolDG1062, mocker: "MockerFixture" +) -> None: + write_raw = mocker.spy(dg1062, "write_raw") + + with pytest.raises(TypeError): + dg1062.ch1.burst.ncycles(2.5) # pyright: ignore[reportArgumentType] + write_raw.assert_not_called() + + def test_burst_phase_is_float(dg1062: RigolDG1062) -> None: burst = dg1062.ch1.burst assert burst.phase() == 0.0