Make ChannelTuple covariant in its channel type

docs/changes/newsfragments/+channeltuple-covariant.improved

@@ -0,0 +1,4 @@
+``ChannelTuple`` is now covariant in its channel type, so type checkers accept a
+``ChannelTuple[SubChannel]`` where a ``ChannelTuple[BaseChannel]`` is expected.
+``ChannelTuple.index`` and ``ChannelTuple.count`` now accept any object, matching
+``collections.abc.Sequence``. ``ChannelList`` remains invariant because it is mutable.

pyproject.toml

@@ -210,6 +210,14 @@ reportUnnecessaryTypeIgnoreComment = "none"
 # but not type ignore comments since mypy already checks if type ignores are necessary
 reportUnnecessaryContains = "none"
 
+# mypy does not check the tests, so pyright is the only checker that can
+# report unnecessary type ignore comments there.
+[[tool.pyright.executionEnvironments]]
+root = "tests"
+# keep resolving ``import tests.…`` from the project root
+extraPaths = ["."]
+reportUnnecessaryTypeIgnoreComment = "error"
+
 [tool.pytest.ini_options]
 minversion = "7.2"
 testpaths = "tests"

src/qcodes/instrument/channel.py

@@ -3,7 +3,14 @@
 from __future__ import annotations
 
 import sys
-from collections.abc import Callable, Iterable, Iterator, MutableSequence, Sequence
+from collections.abc import (
+    Callable,
+    Iterable,
+    Iterator,
+    Mapping,
+    MutableSequence,
+    Sequence,
+)
 from typing import TYPE_CHECKING, Any, Generic, Self, cast, overload
 
 from typing_extensions import TypeVar
@@ -193,13 +200,16 @@ def __init__(
         self._snapshotable = snapshotable
         self._paramclass = multichan_paramclass
 
-        self._channel_mapping: dict[str, InstrumentModuleType] = {}
+        # The storage is typed as read-only (``Sequence`` and ``Mapping``) so
+        # that type checkers infer ``ChannelTuple`` as covariant in its
+        # channel type. ``ChannelList`` keeps its own mutable references to
+        # the same objects.
+        self._channels: Sequence[InstrumentModuleType]
         # provide lookup of channels by name
-        # If a list of channels is not provided, define a list to store
-        # channels. This will eventually become a locked tuple.
-        self._channels: list[InstrumentModuleType]
+        self._channel_mapping: Mapping[str, InstrumentModuleType]
         if chan_list is None:
             self._channels = []
+            self._channel_mapping = {}
         else:
             self._channels = list(chan_list)
             self._channel_mapping = {
@@ -331,7 +341,7 @@ def name_parts(self) -> list[str]:
 
     def index(
         self,
-        obj: InstrumentModuleType,
+        obj: object,
         start: int = 0,
         stop: int = sys.maxsize,
     ) -> int:
@@ -346,7 +356,7 @@ def index(
         """
         return self._channels.index(obj, start, stop)
 
-    def count(self, obj: InstrumentModuleType) -> int:
+    def count(self, obj: object) -> int:
         """Returns number of instances of the given object in the list
 
         Args:
@@ -703,6 +713,15 @@ def __init__(
         super().__init__(
             parent, name, chan_type, chan_list, snapshotable, multichan_paramclass
         )
+        # Mutable views of the storage that ``ChannelTuple`` exposes read-only.
+        # ``_channels`` and ``_channel_mapping`` refer to these same objects,
+        # so they must only be modified in place, never reassigned.
+        self._mutable_channels: list[InstrumentModuleType] = list(self._channels)
+        self._mutable_channel_mapping: dict[str, InstrumentModuleType] = dict(
+            self._channel_mapping
+        )
+        self._channels = self._mutable_channels
+        self._channel_mapping = self._mutable_channel_mapping
         if len(self._channels) > 0:
             self._locked = True
         else:
@@ -717,10 +736,8 @@ def __delitem__(self, key: slice) -> None: ...
     def __delitem__(self, key: int | slice) -> None:
         if self._locked:
             raise AttributeError("Cannot delete from a locked channel list")
-        self._channels.__delitem__(key)
-        self._channel_mapping = {
-            channel.short_name: channel for channel in self._channels
-        }
+        self._mutable_channels.__delitem__(key)
+        self._rebuild_channel_mapping()
 
     @overload
     def __setitem__(self, index: int, value: InstrumentModuleType) -> None: ...
@@ -741,15 +758,20 @@ def __setitem__(
         # asserts added to work around https://github.com/python/mypy/issues/7858
         if isinstance(index, int):
             assert isinstance(value, InstrumentModule)
-            self._channels[index] = value  # type: ignore[assignment]
+            self._mutable_channels[index] = value  # type: ignore[assignment]
             # mypy does not know that InstrumentModuleType is a TypeVar bound to
             # InstrumentModule so complains here
         else:
             assert not isinstance(value, InstrumentModule)
-            self._channels[index] = value
-        self._channel_mapping = {
-            channel.short_name: channel for channel in self._channels
-        }
+            self._mutable_channels[index] = value
+        self._rebuild_channel_mapping()
+
+    def _rebuild_channel_mapping(self) -> None:
+        # Update in place so that ``_channel_mapping`` stays the same object.
+        self._mutable_channel_mapping.clear()
+        self._mutable_channel_mapping.update(
+            {channel.short_name: channel for channel in self._mutable_channels}
+        )
 
     def append(self, obj: InstrumentModuleType) -> None:
         """
@@ -768,18 +790,17 @@ def append(self, obj: InstrumentModuleType) -> None:
                 f"type. Adding {type(obj).__name__} to a "
                 f"list of {self._chan_type.__name__}."
             )
-        self._channel_mapping[obj.short_name] = obj
-        self._channels.append(obj)
+        self._mutable_channel_mapping[obj.short_name] = obj
+        self._mutable_channels.append(obj)
 
     def clear(self) -> None:
         """
         Clear all items from the ChannelList.
         """
         if self._locked:
             raise AttributeError("Cannot clear a locked ChannelList")
-        # when not locked the _channels seq is a list
-        self._channels.clear()
-        self._channel_mapping.clear()
+        self._mutable_channels.clear()
+        self._mutable_channel_mapping.clear()
 
     def remove(self, obj: InstrumentModuleType) -> None:
         """
@@ -792,8 +813,8 @@ def remove(self, obj: InstrumentModuleType) -> None:
         if self._locked:
             raise AttributeError("Cannot remove from a locked channel list")
         else:
-            self._channels.remove(obj)
-            self._channel_mapping.pop(obj.short_name)
+            self._mutable_channels.remove(obj)
+            self._mutable_channel_mapping.pop(obj.short_name)
 
     def extend(self, objects: Iterable[InstrumentModuleType]) -> None:
         """
@@ -811,8 +832,10 @@ def extend(self, objects: Iterable[InstrumentModuleType]) -> None:
         objects_tuple = tuple(objects)
         if not all(isinstance(obj, self._chan_type) for obj in objects_tuple):
             raise TypeError("All items in a channel list must be of the same type.")
-        self._channels.extend(objects_tuple)
-        self._channel_mapping.update({obj.short_name: obj for obj in objects_tuple})
+        self._mutable_channels.extend(objects_tuple)
+        self._mutable_channel_mapping.update(
+            {obj.short_name: obj for obj in objects_tuple}
+        )
 
     def insert(self, index: int, obj: InstrumentModuleType) -> None:
         """
@@ -830,8 +853,8 @@ def insert(self, index: int, obj: InstrumentModuleType) -> None:
                 f"All items in a channel list must be of the same "
                 f"type. Adding {type(obj).__name__} to a list of {self._chan_type.__name__}."
             )
-        self._channels.insert(index, obj)
-        self._channel_mapping[obj.short_name] = obj
+        self._mutable_channels.insert(index, obj)
+        self._mutable_channel_mapping[obj.short_name] = obj
 
     def get_validator(self) -> ChannelTupleValidator:
         """

tests/dataset/measurement/test_measurement_context_manager.py

@@ -2132,10 +2132,10 @@ def test_datasaver_2d_multi_parameters_array(
 
     # 30 points in each setpoint value list
     this_sp_val: npt.NDArray = np.array(
-        reduce(list.__add__, [[n] * 3 for n in range(5, 10)], [])  # type: ignore[arg-type]
+        reduce(list.__add__, [[n] * 3 for n in range(5, 10)], [])
     )
     that_sp_val: npt.NDArray = np.array(
-        reduce(list.__add__, [[n] for n in range(9, 12)], []) * 5  # type: ignore[arg-type]
+        reduce(list.__add__, [[n] for n in range(9, 12)], []) * 5
     )
 
     assert isinstance(ds, DataSet)

tests/dataset/test_converters.py

@@ -182,7 +182,7 @@ def test_construct_current_rundescriber_from_fake_v4(some_interdeps) -> None:
     rds1 = RunDescriber._from_dict(v4)
     rds_upgraded = from_dict_to_current(v4)
     v3 = v4.copy()
-    v3.pop("foobar")  # type: ignore[typeddict-item]
+    v3.pop("foobar")
     v3["version"] = 3
     assert rds1._to_dict() == v3
     assert rds_upgraded._to_dict() == v3

tests/dataset/test_dataset_basic.py

@@ -324,7 +324,7 @@ def test_load_by_id_for_none() -> None:
     experiment_name=hst.text(min_size=1),
     sample_name=hst.text(min_size=1),
     dataset_name=hst.text(
-        hst.characters(categories=_unicode_categories),  # type: ignore[arg-type]
+        hst.characters(categories=_unicode_categories),
         min_size=1,
     ),
 )

tests/dataset/test_dependencies.py

@@ -101,8 +101,8 @@ def test_init_validation_raises(some_paramspecbases) -> None:
     for inv in invalid_trees_2:
         with pytest.raises(ValueError, match="already exists"):
             InterDependencies_(
-                dependencies=inv["deps"],  # type: ignore[arg-type]
-                inferences=inv["inffs"],  # type: ignore[arg-type]
+                dependencies=inv["deps"],
+                inferences=inv["inffs"],
             )
 
 

tests/dataset/test_parameter_with_setpoints_has_control.py

@@ -24,7 +24,7 @@ def _make_controlled_setpoints(
     """Create a ParameterWithSetpoints that infers ``controlled`` via unpack_self."""
 
     class _ControlledSetpoints(ParameterWithSetpoints):
-        def unpack_self(self, value):  # type: ignore[override]
+        def unpack_self(self, value):
             res = super().unpack_self(value)
             res.append((controlled, controlled()))
             return res

tests/drivers/test_lakeshore_325.py

@@ -13,9 +13,7 @@
         min_size=1,
         max_size=5,
         unique=True,
-    ).map(
-        sorted  # type: ignore[arg-type]
-    )
+    ).map(sorted)
 )
 def test_decode_sensor_status(list_of_codes) -> None:
     """

tests/helpers/test_delegate_attribues.py

@@ -195,7 +195,7 @@ def prop(self) -> int:
             # ``missing`` does not exist; accessing it raises AttributeError.
             # Before this fix, Python fell through to ``__getattr__`` which
             # reported "no attribute 'prop'" and hid the real cause.
-            return self.missing  # type: ignore[attr-defined]
+            return self.missing
 
     obj = WithFaultyProperty()
     with pytest.raises(AttributeError, match="missing"):
@@ -238,7 +238,7 @@ class Plain(DelegateAttributes):
     # ``__name__`` is not defined on ``Plain`` instances, so accessing it
     # should raise ``AttributeError``, never ``TypeError``.
     with pytest.raises(AttributeError):
-        _ = obj.__name__  # type: ignore[attr-defined]
+        _ = obj.__name__
 
     # ``inspect.iscoroutinefunction`` internally does
     # ``getattr(obj, '__name__', None)`` — this must not raise.

tests/parameter/test_delegate_parameter.py

@@ -79,12 +79,12 @@ def get_cmd() -> Any:
 
             p = Parameter(
                 *args,
-                **kwargs,  # type: ignore[misc]
+                **kwargs,
                 set_cmd=set_cmd,
                 get_cmd=get_cmd,
             )
             param = cast("ObservableParam", p)
-            param.get_instr_val = get_cmd  # type: ignore[method-assign]
+            param.get_instr_val = get_cmd
         return param
 
     yield make_parameter
@@ -380,7 +380,7 @@ def test_initial_cache_value_not_applied_to_temporary_base_cache() -> None:
     cache_set_call_count = 0
     original_cache_set = _Cache.set
 
-    def counting_set(self: _Cache, value: object) -> None:  # type: ignore[type-arg]
+    def counting_set(self: _Cache, value: object) -> None:
         nonlocal cache_set_call_count
         cache_set_call_count += 1
         original_cache_set(self, value)

tests/parameter/test_get_set_wrapping.py

@@ -69,7 +69,7 @@ def test_gettable_settable_attributes_with_get_set_raw(
     """Test that parameters that have get_raw,set_raw are
     listed as gettable/settable and reverse."""
 
-    class GetSetParam(baseclass):  # type: ignore[valid-type,misc]
+    class GetSetParam(baseclass):
         def __init__(self, *args: Any, initial_value: Any = None, **kwargs: Any):
             self._value = initial_value
             super().__init__(*args, **kwargs)

tests/parameter/test_parameter_basics.py

@@ -252,7 +252,7 @@ def test_underlying_instrument_for_virtual_parameter() -> None:
     p._instrument = named_instrument  # type: ignore[assignment]
     vp = VirtualParameter("test_param", param=p)
 
-    assert vp.underlying_instrument is named_instrument  # type: ignore[comparison-overlap]
+    assert vp.underlying_instrument is named_instrument
 
 
 def test_get_cmd_str_no_instrument_raises() -> None:

tests/parameter/test_parameter_on_set_callback.py

@@ -22,14 +22,14 @@
 THREAD_SLEEP = 0.01
 
 
-@pytest.fixture(autouse=True)  # type: ignore[misc]
+@pytest.fixture(autouse=True)
 def _reset_callback() -> "Generator[None, None, None]":
     """Reset the callback after each test"""
     yield
     ParameterBase.global_on_set_callback = None
 
 
-@pytest.fixture()  # type: ignore[misc]
+@pytest.fixture()
 def basic_parameter(
     basic_callback: "Callable[[ParameterBase, Any], None]",
 ) -> Parameter:
@@ -39,7 +39,7 @@ def basic_parameter(
     return param
 
 
-@pytest.fixture(scope="function")  # type: ignore[misc]
+@pytest.fixture(scope="function")
 def basic_callback(
     captured_params: list[tuple[ParameterBase, Any]],
 ) -> "Callable[[ParameterBase, Any], None]":
@@ -51,13 +51,13 @@ def callback(param: ParameterBase, value: Any) -> None:
     return callback
 
 
-@pytest.fixture(scope="function")  # type: ignore[misc]
+@pytest.fixture(scope="function")
 def captured_params() -> list[tuple[ParameterBase, Any]]:
     """Fixture for capturing callback parameters"""
     return []
 
 
-@pytest.fixture(autouse=True, scope="function")  # type: ignore[misc]
+@pytest.fixture(autouse=True, scope="function")
 def cleanup_db_connections():
     """Clean up any open SQLite connections after each test"""
     yield
@@ -120,7 +120,7 @@ def test_callback(p: ParameterBase, value: Any) -> None:
 class TestValidationBehavior:
     """Tests for validation-related functionality"""
 
-    @pytest.mark.parametrize(  # type: ignore[misc]
+    @pytest.mark.parametrize(
         "test_input,validator,should_callback",
         [
             pytest.param(5, validators.Numbers(0, 10), True, id="valid_number"),