Merge #4536

4536: Upgrade AMI drivers to conform to our standard r=jenshnielsen a=jenshnielsen



Co-authored-by: Jens H. Nielsen <[email protected]>

docs/Makefile

@@ -26,6 +26,7 @@ clean:
 # generate api docs for instruments automatically
 genapi:
 	sphinx-apidoc  -o  _auto  -d 10 ../qcodes \
+		../qcodes/instrument_drivers/american_magnetics\* \
 		../qcodes/instrument_drivers/agilent \
 		../qcodes/instrument_drivers/AimTTi \
 		../qcodes/instrument_drivers/basel \

docs/drivers_api/American_magnetics.rst

@@ -0,0 +1,7 @@
+.. _AMI_api :
+
+American Magnetics Inc Drivers
+==============================
+
+.. automodule:: qcodes.instrument_drivers.american_magnetics
+    :autosummary:

docs/make.bat

@@ -38,11 +38,12 @@ REM a long time ago (i.e. "Keysight", the upper-case, is used
 REM for storing drivers, not the lower-case one).
 sphinx-apidoc  -o  _auto  -d 10 ..\qcodes ^
     ..\qcodes\instrument_drivers\agilent\* ^
-	..\qcodes\instrument_drivers\AimTTi ^
-	..\qcodes\instrument_drivers\basel ^
+    ..\qcodes\instrument_drivers\AimTTi ^
+    ..\qcodes\instrument_drivers\american_magnetics\* ^
+    ..\qcodes\instrument_drivers\basel ^
     ..\qcodes\instrument_drivers\HP ^
-	..\qcodes\instrument_drivers\ithaco ^
-	..\qcodes\instrument_drivers\Lakeshore ^
+    ..\qcodes\instrument_drivers\ithaco ^
+    ..\qcodes\instrument_drivers\Lakeshore ^
     ..\qcodes\instrument_drivers\QuantumDesign\DynaCoolPPMS\private\* ^
     ..\qcodes\instrument_drivers\stahl\* ^
     ..\qcodes\instrument_drivers\stanford_research\* ^

qcodes/instrument_drivers/american_magnetics/AMI430_visa.py

@@ -1,23 +1,14 @@
-import collections
-import collections.abc
+from __future__ import annotations
+
 import logging
 import numbers
 import time
 import warnings
 from collections import defaultdict
+from collections.abc import Iterable, Sequence
 from contextlib import ExitStack
 from functools import partial
-from typing import (
-    Any,
-    Callable,
-    Iterable,
-    List,
-    Optional,
-    Sequence,
-    Tuple,
-    TypeVar,
-    Union,
-)
+from typing import Any, Callable, TypeVar
 
 import numpy as np
 from pyvisa import VisaIOError
@@ -48,15 +39,15 @@ class _Decorators:
         @classmethod
         def check_enabled(cls, f: Callable[..., T]) -> Callable[..., T]:
             def check_enabled_decorator(
-                self: "AMI430SwitchHeater", *args: Any, **kwargs: Any
+                self: AMI430SwitchHeater, *args: Any, **kwargs: Any
             ) -> T:
                 if not self.check_enabled():
                     raise AMI430Exception("Switch not enabled")
                 return f(self, *args, **kwargs)
 
             return check_enabled_decorator
 
-    def __init__(self, parent: "AMI430") -> None:
+    def __init__(self, parent: AMIModel430) -> None:
         super().__init__(parent, "SwitchHeater")
 
         # Add state parameters
@@ -140,14 +131,14 @@ def check_state(self) -> bool:
         return bool(int(self.ask("PS?").strip()))
 
 
-class AMI430(VisaInstrument):
+class AMIModel430(VisaInstrument):
     """
     Driver for the American Magnetics Model 430 magnet power supply programmer.
 
     This class controls a single magnet power supply. In order to use two or
     three magnets simultaneously to set field vectors, first instantiate the
     individual magnets using this class and then pass them as arguments to
-    either the AMI430_2D or AMI430_3D virtual instrument classes.
+    the AMIModel4303D virtual instrument classes.
 
     Args:
         name: a name for the instrument
@@ -168,12 +159,12 @@ def __init__(
         address: str,
         reset: bool = False,
         terminator: str = "\r\n",
-        current_ramp_limit: Optional[float] = None,
+        current_ramp_limit: float | None = None,
         **kwargs: Any,
     ):
         if "has_current_rating" in kwargs.keys():
             warnings.warn(
-                "'has_current_rating' kwarg to AMI430 "
+                "'has_current_rating' kwarg to AMIModel430 "
                 "is deprecated and has no effect",
                 category=QCoDeSDeprecationWarning,
             )
@@ -243,7 +234,7 @@ def __init__(
         )
         if current_ramp_limit is None:
             self._update_ramp_rate_limit(
-                AMI430._DEFAULT_CURRENT_RAMP_LIMIT, update=False
+                AMIModel430._DEFAULT_CURRENT_RAMP_LIMIT, update=False
             )
         else:
             self._update_ramp_rate_limit(current_ramp_limit, update=False)
@@ -458,7 +449,7 @@ def _update_ramp_rate_limit(
             if self.ramp_rate() > field_ramp_limit:
                 self.ramp_rate(field_ramp_limit)
 
-    def _update_coil_constant(self, new_coil_constant: Optional[float] = None) -> float:
+    def _update_coil_constant(self, new_coil_constant: float | None = None) -> float:
         """
         Update the coil constant and relevant scaling factors.
         If new_coil_constant is none, query the coil constant from the
@@ -478,7 +469,7 @@ def _update_coil_constant(self, new_coil_constant: Optional[float] = None) -> fl
         return new_coil_constant
 
     def _update_units(
-        self, ramp_rate_units: Optional[int] = None, field_units: Optional[int] = None
+        self, ramp_rate_units: int | None = None, field_units: int | None = None
     ) -> None:
         # Get or set units on device
         if ramp_rate_units is None:
@@ -495,8 +486,8 @@ def _update_units(
             field_units_int = self.field_units.inverse_val_mapping[field_units]
 
         # Map to shortened unit names
-        ramp_rate_units_short = AMI430._SHORT_UNITS[ramp_rate_units_int]
-        field_units_short = AMI430._SHORT_UNITS[field_units_int]
+        ramp_rate_units_short = AMIModel430._SHORT_UNITS[ramp_rate_units_int]
+        field_units_short = AMIModel430._SHORT_UNITS[field_units_int]
 
         # And update all units
         self.coil_constant.unit = f"{field_units_short}/A"
@@ -561,31 +552,35 @@ def ask_raw(self, cmd: str) -> str:
         return result
 
 
-class AMI430_3D(Instrument):
+class AMI430(AMIModel430):
+    pass
+
+
+class AMIModel4303D(Instrument):
     def __init__(
         self,
         name: str,
-        instrument_x: Union[AMI430, str],
-        instrument_y: Union[AMI430, str],
-        instrument_z: Union[AMI430, str],
-        field_limit: Union[numbers.Real, Iterable[CartesianFieldLimitFunction]],
+        instrument_x: AMIModel430 | str,
+        instrument_y: AMIModel430 | str,
+        instrument_z: AMIModel430 | str,
+        field_limit: numbers.Real | Iterable[CartesianFieldLimitFunction],
         **kwargs: Any,
     ):
         """
         Driver for controlling three American Magnetics Model 430 magnet power
         supplies simultaneously for setting magnetic field vectors.
 
         The individual magnet power supplies can be passed in as either
-        instances of AMI430 driver or as names of existing AMI430 instances.
+        instances of AMIModel430 driver or as names of existing AMIModel430 instances.
         In the latter case, the instances will be found via the passed names.
 
         Args:
             name: a name for the instrument
-            instrument_x: AMI430 instance or a names of existing AMI430
+            instrument_x: AMIModel430 instance or a names of existing AMIModel430
                 instance for controlling the X axis of magnetic field
-            instrument_y: AMI430 instance or a names of existing AMI430
+            instrument_y: AMIModel430 instance or a names of existing AMIModel430
                 instance for controlling the Y axis of magnetic field
-            instrument_z: AMI430 instance or a names of existing AMI430
+            instrument_z: AMIModel430 instance or a names of existing AMIModel430
                 instance for controlling the Z axis of magnetic field
             field_limit: a number for maximum allows magnetic field or an
                 iterable of callable field limit functions that define
@@ -600,38 +595,38 @@ def __init__(
             (instrument_x, instrument_y, instrument_z),
             ("instrument_x", "instrument_y", "instrument_z"),
         ):
-            if not isinstance(instrument, (AMI430, str)):
+            if not isinstance(instrument, (AMIModel430, str)):
                 raise ValueError(
-                    f"Instruments need to be instances of the class AMI430 "
+                    f"Instruments need to be instances of the class AMIModel430 "
                     f"or be valid names of already instantiated instances "
-                    f"of AMI430 class; {arg_name} argument is "
+                    f"of AMIModel430 class; {arg_name} argument is "
                     f"neither of those"
                 )
 
-        def find_ami430_with_name(ami430_name: str) -> AMI430:
-            found_ami430 = AMI430.find_instrument(
-                name=ami430_name, instrument_class=AMI430
+        def find_ami430_with_name(ami430_name: str) -> AMIModel430:
+            found_ami430 = AMIModel430.find_instrument(
+                name=ami430_name, instrument_class=AMIModel430
             )
             return found_ami430
 
         self._instrument_x = (
             instrument_x
-            if isinstance(instrument_x, AMI430)
+            if isinstance(instrument_x, AMIModel430)
             else find_ami430_with_name(instrument_x)
         )
         self._instrument_y = (
             instrument_y
-            if isinstance(instrument_y, AMI430)
+            if isinstance(instrument_y, AMIModel430)
             else find_ami430_with_name(instrument_y)
         )
         self._instrument_z = (
             instrument_z
-            if isinstance(instrument_z, AMI430)
+            if isinstance(instrument_z, AMIModel430)
             else find_ami430_with_name(instrument_z)
         )
 
-        self._field_limit: Union[float, Iterable[CartesianFieldLimitFunction]]
-        if isinstance(field_limit, collections.abc.Iterable):
+        self._field_limit: float | Iterable[CartesianFieldLimitFunction]
+        if isinstance(field_limit, Iterable):
             self._field_limit = field_limit
         elif isinstance(field_limit, numbers.Real):
             # Conversion to float makes related driver logic simpler
@@ -888,16 +883,16 @@ def ramp_simultaneously(self, setpoint: FieldVector, duration: float) -> None:
     @staticmethod
     def calculate_axes_ramp_rates_for(
         start: FieldVector, setpoint: FieldVector, duration: float
-    ) -> Tuple[float, float, float]:
+    ) -> tuple[float, float, float]:
         """
         Given starting and setpoint fields and expected ramp time calculates
         required ramp rates for x, y, z axes (in this order) where axes are
         ramped simultaneously.
         """
-        vector_ramp_rate = AMI430_3D.calculate_vector_ramp_rate_from_duration(
+        vector_ramp_rate = AMIModel4303D.calculate_vector_ramp_rate_from_duration(
             start, setpoint, duration
         )
-        return AMI430_3D.calculate_axes_ramp_rates_from_vector_ramp_rate(
+        return AMIModel4303D.calculate_axes_ramp_rates_from_vector_ramp_rate(
             start, setpoint, vector_ramp_rate
         )
 
@@ -910,12 +905,12 @@ def calculate_vector_ramp_rate_from_duration(
     @staticmethod
     def calculate_axes_ramp_rates_from_vector_ramp_rate(
         start: FieldVector, setpoint: FieldVector, vector_ramp_rate: float
-    ) -> Tuple[float, float, float]:
+    ) -> tuple[float, float, float]:
         delta_field = setpoint - start
         ramp_rate_3d = delta_field / delta_field.norm() * vector_ramp_rate
         return abs(ramp_rate_3d["x"]), abs(ramp_rate_3d["y"]), abs(ramp_rate_3d["z"])
 
-    def _raise_if_not_same_field_and_ramp_rate_units(self) -> Tuple[str, str]:
+    def _raise_if_not_same_field_and_ramp_rate_units(self) -> tuple[str, str]:
         instruments = (self._instrument_x, self._instrument_y, self._instrument_z)
 
         field_units_of_instruments = defaultdict(set)
@@ -949,7 +944,7 @@ def _raise_if_not_same_field_and_ramp_rate_units(self) -> Tuple[str, str]:
         return common_field_units, common_ramp_rate_units
 
     def _verify_safe_setpoint(
-        self, setpoint_values: Tuple[float, float, float]
+        self, setpoint_values: tuple[float, float, float]
     ) -> bool:
         if isinstance(self._field_limit, (int, float)):
             return bool(np.linalg.norm(setpoint_values) < self._field_limit)
@@ -960,7 +955,7 @@ def _verify_safe_setpoint(
 
         return answer
 
-    def _adjust_child_instruments(self, values: Tuple[float, float, float]) -> None:
+    def _adjust_child_instruments(self, values: tuple[float, float, float]) -> None:
         """
         Set the fields of the x/y/z magnets. This function is called
         whenever the field is changed and performs several safety checks
@@ -993,7 +988,7 @@ def _adjust_child_instruments(self, values: Tuple[float, float, float]) -> None:
             self._perform_default_ramp(values)
 
     def _update_individual_axes_ramp_rates(
-        self, values: Tuple[float, float, float]
+        self, values: tuple[float, float, float]
     ) -> None:
         if self.vector_ramp_rate() is None or self.vector_ramp_rate() == 0:
             raise ValueError(
@@ -1015,7 +1010,7 @@ def _update_individual_axes_ramp_rates(
                 f"is {new_axis_ramp_rate} {instrument.ramp_rate.unit}"
             )
 
-    def _perform_simultaneous_ramp(self, values: Tuple[float, float, float]) -> None:
+    def _perform_simultaneous_ramp(self, values: tuple[float, float, float]) -> None:
         self._prepare_to_restore_individual_axes_ramp_rates()
 
         self._update_individual_axes_ramp_rates(values)
@@ -1050,8 +1045,8 @@ def _perform_simultaneous_ramp(self, values: Tuple[float, float, float]) -> None
 
         self.log.debug(f"Simultaneous ramp: returning from the ramp call")
 
-    def _perform_default_ramp(self, values: Tuple[float, float, float]) -> None:
-        operators: Tuple[Callable[[Any, Any], bool], ...] = (np.less, np.greater)
+    def _perform_default_ramp(self, values: tuple[float, float, float]) -> None:
+        operators: tuple[Callable[[Any, Any], bool], ...] = (np.less, np.greater)
         for operator in operators:
             # First ramp the coils that are decreasing in field strength.
             # This will ensure that we are always in a safe region as
@@ -1119,7 +1114,9 @@ def pause(self) -> None:
         ):
             axis_instrument.pause()
 
-    def _request_field_change(self, instrument: AMI430, value: numbers.Real) -> None:
+    def _request_field_change(
+        self, instrument: AMIModel430, value: numbers.Real
+    ) -> None:
         """
         This method is called by the child x/y/z magnets if they are set
         individually. It results in additional safety checks being
@@ -1142,7 +1139,7 @@ def _get_measured_field_vector(self) -> FieldVector:
             z=self._instrument_z.field(),
         )
 
-    def _get_measured(self, *names: str) -> Union[numbers.Real, List[numbers.Real]]:
+    def _get_measured(self, *names: str) -> numbers.Real | list[numbers.Real]:
         measured_field_vector = self._get_measured_field_vector()
 
         measured_values = measured_field_vector.get_components(*names)
@@ -1160,9 +1157,7 @@ def _get_measured(self, *names: str) -> Union[numbers.Real, List[numbers.Real]]:
 
         return return_value
 
-    def _get_setpoints(
-        self, names: Sequence[str]
-    ) -> Union[numbers.Real, List[numbers.Real]]:
+    def _get_setpoints(self, names: Sequence[str]) -> numbers.Real | list[numbers.Real]:
 
         measured_values = self._set_point.get_components(*names)
 
@@ -1192,3 +1187,7 @@ def _set_setpoints(self, names: Sequence[str], values: Sequence[float]) -> None:
         self._adjust_child_instruments(set_point.get_components("x", "y", "z"))
 
         self._set_point = set_point
+
+
+class AMI430_3D(AMIModel4303D):
+    pass

qcodes/instrument_drivers/american_magnetics/__init__.py

@@ -0,0 +1,15 @@
+from .AMI430_visa import (
+    AMI430Exception,
+    AMI430SwitchHeater,
+    AMI430Warning,
+    AMIModel430,
+    AMIModel4303D,
+)
+
+__all__ = [
+    "AMI430Exception",
+    "AMI430SwitchHeater",
+    "AMI430Warning",
+    "AMIModel430",
+    "AMIModel4303D",
+]