Type-cleanup follow-up: drop 208 `# type: ignore` comments in `src/`

[marcosfrenkel]

Context

The mypy adoption (PR #127) leaves the source tree green but with 187 # type: ignore comments in src/. This issue tracks reducing them to ~30. A detailed catalogue lives in TODO_type_cleanup.md at the root of the branch (untracked there). Summary below.

Four named structural problems

Problem 1 — Mutable default args + wrong Optional on InstrumentModelBase

src/instrumentserver/gui/base_instrument.py (~line 195). itemsStar, itemsTrash, itemsHide declared as Optional[List[str]] = []. Two bugs:

1. Mutable default — all instances share the same list.
2. Wrong Optional — annotation says nullable but consumer requires List[str].

Fix: default None, branch on assignment, store as List[str] (non-Optional).

Problem 2 — parent overloaded as model OR item in addItem/insertItemTo

base_instrument.py (~line 298). insertItemTo declares parent: QStandardItem but is called with self (the model) at the root. Function body already handles both. Fix: make signature honest with Union[\"InstrumentModelBase\", QtGui.QStandardItem] and annotate the local parent in addItem / fillCollapsedDict likewise.

Problem 3 — Lock vs RLock clash in _createInstrument

src/instrumentserver/server/core.py:448-455. _get_lock_for_target returns Optional[RLock]; fallback uses _instrument_locks_lock which is Lock. Two ignores stack up. Fix: change _instrument_locks_lock to RLock (behavior-preserving — only acquired non-recursively at line 631) and simplify the call site.

Problem 4 — isinstance(obj, tuple(LIST_CONST)) defeats narrowing

server/core.py:518-529 and blueprints.py:340. Mypy can't inspect runtime-constructed tuples, so obj doesn't narrow. Fix: inline class tuples as literals at narrowing call sites (e.g. isinstance(obj, (Instrument, InstrumentChannel, InstrumentBase))). Module-level *_BASE_CLASSES lists can stay for the matching loops.

Catalog by root-cause bucket

Bucket	Pattern	Approx count	Fix sketch
A	Qt event-handler [override] (PyQt5 declares QEvent | None)	16	Change signature to Optional[QEvent], add early return
B	PyQt5 "Optional return" widget/item stubs (addToolBar, header, parentWidget, itemFromIndex, verticalScrollBar, etc.)	~89	Assign to local, assert is not None once, use locally
C	Qt int-flag OR for non-setAlignment/findItems sites (DropAction, OpenModeFlag, etc.)	~5	cast(\"QtCore.Qt.SomeFlags\", flag_a | flag_b) — see recipe note below
D	pyqtBoundSignal.connect(Callable[..., bool]) arg-type	~5	Wrap in discarding lambda
E	ServerGui.stationServer/stationServerThread initialized to None	17	Construct eagerly, or add _require_server() helper
F	ProxyMixin.cli: Optional[Client] not narrowed in subclass	~5	Redeclare cli: Client on ProxyInstrumentModule, or use property
G	QStandardItem | None vs custom ItemBase subclass	~19	typing.cast(ItemBase, item) at the boundary (one site already done in server/application.py:411-422 via cast(PossibleInstrumentDisplayItem, raw_item))
H	Cross-references to Problems 1–4 above	—	See above
I	qcodes TSubmodule TypeVar bound rejects ProxyInstrumentModule/ParameterManager	3	Won't go away — upstream qcodes constraint
J	self.layout = QVBoxLayout(...) shadows QWidget.layout() method	4	Rename self.layout → self._layout in ServerStatus
K	PollingWorker.pollingRates: Optional[Dict] always-present-at-runtime	5	Store as Dict[str, int] with {} default
L	ClientStation.disconnect sets client = None over non-Optional declaration	1	Declare as Optional[Client] and narrow at use sites
M	BaseClient.socket: Optional[zmq.Socket] — narrowing not tracked	2	Early assert self.socket is not None in ask()
N	Misc small issues (method-assign, dict-lookup cast, Optional paths through paramsToFile, blueprints.py Optional dict iter, broadcast socket Optionals on the non-_broadcastParameterChange paths, etc.)	~8	Case-by-case

Recipe note for Bucket C

PyQt5 stubs do not have __or__ overloads on flag enums (AlignmentFlag, MatchFlag, DropAction, OpenModeFlag), so flag_a | flag_b evaluates to int. The Qt.Alignment(...) / Qt.MatchFlags(...) constructor only accepts Alignment | AlignmentFlag / MatchFlags | MatchFlag — not int — so wrapping with the constructor doesn't help. Use typing.cast instead:

from typing import cast
self.setAlignment(
    cast(
        \"QtCore.Qt.Alignment\",
        QtCore.Qt.AlignmentFlag.AlignRight | QtCore.Qt.AlignmentFlag.AlignVCenter,
    )
)

Same shape for findItems(..., cast(\"QtCore.Qt.MatchFlags\", flag_a | flag_b), ...).

Recommended priority

1. High value, low risk (target: ~−40 ignores). Buckets J, M, the rest of G + Problems 1–4. Mostly mechanical, no wider refactor.
2. Medium value, some refactor (target: ~−90 ignores). Buckets E (require-helper), F (subclass redeclare), B (narrow-once locals — bulk).
3. Low value / project-style decisions. Buckets A, C (remaining sites), D — uniform but verbose changes; decide consistency rules first.
4. Won't go away. Bucket I (qcodes upstream), the handful of intentional ignores in Bucket N (method-assign for an intentional monkey-patch in client/proxy.py, import-not-found for optional influxdb_client, no-untyped-def on getWorkingDirectory per existing notes).

Target: 187 → ~30 ignores after (1) + (2) + Problems 1–4.

Test gap to close before refactoring InstrumentModelBase

Nothing in test/pytest/ directly exercises InstrumentModelBase.addItem / insertItemTo, the itemsHide/itemsStar/itemsTrash filter branches, or fillCollapsedDict/restoreCollapsedDict. Before doing Problems 1–2 it is worth adding:

1. InstrumentParameters constructed with parameters-hide=[...] — assert filter works.
2. Two ModelParameters instances with no itemsStar kwarg — assert m1.itemsStar is not m2.itemsStar (guards the mutable-default fix).

Suggested PR breakdown

To keep diffs reviewable, ship as small successive PRs:

• PR 1: Bucket J + M + rest of G (low risk, ~−25)
• PR 2: Problems 1–4 + new InstrumentModelBase tests (~−15)
• PR 3: Bucket E (stationServer require-helper, ~−17)
• PR 4: Bucket F (ProxyInstrumentModule.cli redeclare, ~−5)
• PR 5: Bucket B (bulk widget-return narrowing, ~−70)
• PR 6: Decide and apply Buckets A/C/D project-wide

Related

• PR Modernizing packaging #127 — initial packaging modernization