Uh oh!
There was an error while loading. Please reload this page.
Support type checking with TY - #8441
Draft
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 53 commits into
Draft
Support type checking with TY#8441Jens Hedegaard Nielsen (jenshnielsen) wants to merge 53 commits into
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 53 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@## main #8441 +/- ##
==========================================
- Coverage 70.15% 70.15% -0.01%
==========================================
Files 305 305 Lines 31931 31977 +46 ==========================================
+ Hits 22402 22433 +31 - Misses 9529 9544 +15 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Aug 25, 2026
Closed
Jens Hedegaard Nielsen (jenshnielsen)force-pushed
the
ty_0_73_support_1
branch
from
August 25, 2026 11:05
9937cb8 to
a7ccd46CompareScope ty to src and tests and exclude the legacy Decadac driver, mirroring the existing pyright config. Disable import resolution rules for the drivers that depend on optional packages, as already done for mypy. Check against all platforms so that Windows only drivers are type checked independently of the platform ty runs on.
Parameter used to replace its own get_raw/set_raw methods with the implementation generated from get_cmd/set_cmd. Assigning over a method makes type checkers infer get_raw/set_raw to be instance attributes of Parameter, which made every subclass implementing them as regular methods an invalid override. Store the generated implementation on the instance and let get_raw and set_raw dispatch to it. They stay marked abstract so that _implements_get_raw keeps reporting False for Parameter itself. Clears 66 ty diagnostics.
add_parameter always binds the new parameter to self, so defaulting TParameter to a bare Parameter, which expands to Parameter[Any, InstrumentBase | None], wrongly claimed the instrument was InstrumentBase | None. As InstrumentTypeVar_co is covariant this made the result unassignable to the Parameter[SomeType, Self] annotations drivers use. ty applies a PEP 696 typevar default before considering the return type context, so it hit the default rather than solving from the declared type. mypy and pyright were unaffected. Clears 33 ty diagnostics.
store_array_to_database asserted that the measured array has an array_id, but passed the array_id of its setpoint arrays straight to add_result without checking them. Those are different arrays, so a legacy dataset with an unnamed setpoint array failed deep inside the data saver. Raise a clear ValueError instead. Hoist the setpoint arrays and their ids out of the loops rather than re-indexing set_arrays on every iteration, and drop a pyright suppression that the qcodes_loop annotations make unnecessary.
The scale and offset conversions assume a numeric data type and rely on catching TypeError, which does not fit the generic parameter data type. They were expressed inline, so the working variable was repeatedly narrowed to whatever the last branch assigned, and each step needed a suppression. Move them into four module level helpers that take and return Any. This keeps the arithmetic out of the generic class, deduplicates the iterable and scalar branches, and drops the suppressions in this file from 15 to 3. Also stop routing issuperset through __contains__, which ty cannot resolve on Self when the class type parameter has a bound.
self.parameter was a lambda with name, full_name, label and unit attached to it. A small dataclass expresses that directly and drops seven type checker suppressions. It is still marked as a hack: CombinedParameter does not inherit from Parameter or ParameterBase, so it has to fake the parts of their api that it is expected to provide. The object stays callable, returning None as the lambda did, in case external code relies on it. The units deprecation warning now runs before the object is built, which is safe because the class has no custom __repr__.
ty does not recognise a TypedDict that is generic over more than one type variable as a mapping when one of those type variables has a PEP 696 default, so it rejects re-expanding the kwargs with **. A TypedDict generic over a single such type variable is accepted, so this is a bug rather than something the code should work around. Suppress it at the five subclasses that forward their kwargs on, and document the reason once on ParameterBaseKWArgs.
Reverts routing issuperset through _dict. The underlying problem is astral-sh/ty#4303, passing a bounded class scoped type variable to the object parameter of __contains__, which is a known ty bug awaiting a fix. Keep the natural spelling and suppress with a link, so the ignore can be dropped once ty is fixed.
Further reduction showed the trigger is not a TypedDict being generic over more than one type variable. One type parameter with any non-Any PEP 696 default is enough, and the problem is not specific to ** expansion: ty computes the upper bound of the synthesized Self as the default specialization, so every other specialization is rejected by the members that bind Self.
The driver narrowed root_instrument and instrument to the concrete driver classes with an annotation plus a suppression, and worked around pyvisa typing the return of read_binary_values and query_binary_values as Sequence[float] regardless of the requested container. Spell both as cast instead, which all three type checkers accept and which drops five suppressions.
_finalize_res_dict_standalones built intermediate lists whose element type was inferred from the branch that built them rather than from the declaration. dict is invariant in its value type, so a list of dict[str, str] is not assignable to a list of dict[str, VALUE]. Append and extend directly instead, which gives the dict literals the declared element type as context. Note that spelling this as res_list += [...] is not enough, pyright does not propagate the element type through the augmented assignment.
_check_error_code read __name__ off a Callable, which the type system does not guarantee. Annotating the parameter more precisely would risk breaking the assignment to c_func.errcheck, since the parameter is contravariant against ctypes own typing, so fall back to repr instead. This also keeps the log line useful if errcheck is ever handed something that is not a function.
set_colorbar_extend deliberately writes to a private matplotlib attribute, as the surrounding docstring explains, because Colorbar has no setter for extend. Extend the existing mypy suppression to ty.
The decorator tags the decorated function with a marker attribute that ParameterBase later reads. A Callable has no such attribute as far as the type system is concerned, so extend the existing mypy suppression to ty.
Without an annotation ty infers the value type of the dictionary as Any | None | tuple[str, int], picking up the None from the later pop(sock, None), which then makes indexing the address tuple an error. Declare the intended type instead.
dict.fromkeys with no value is typed as dict[str, Any | None], so every read of the processed data had to be suppressed. The loop below assigns every key anyway, so start from an empty dict with the intended type and drop the two suppressions.
numpy_ints and numpy_floats were tuples of bare type, so the element type carried no information and registering sqlite adapters for them could not be checked. Narrowing them surfaced that _adapt_float only declared float, even though it is registered for the numpy float types as well. Annotate it like _adapt_complex next to it, which already accepts its numpy counterpart. The two changes are in one commit because the adapter signature is only wrong once the tuples are narrowed.
ParamSpec._from_dict narrows the parameter to ParamSpecDict, which carries the extra depends_on and inferred_from fields that the base ParamSpecBaseDict does not. That is a deliberate Liskov violation which already carried a mypy suppression, so extend it to ty.
IPToVisa deliberately injects VisaInstrument ahead of IPInstrument in the MRO so that an IPInstrument can be driven by the pyvisa-sim backend, as the class docstring explains. The two bases declare set_address incompatibly, which already carried a mypy suppression, so extend it to ty.
The Alazar boards report a CPLD version as an int, so get_idn widens the value type of the returned dict. The existing TODO records that this is inconsistent with the base class, and the override already carried a mypy suppression, so extend it to ty.
The override named its parameter name while DelegateAttributes.__getattr__ names it key, so the two differ for a caller passing it by keyword. Python only ever calls __getattr__ positionally, so this is a real but harmless Liskov violation and is simpler to fix than to suppress.
increment only works for parameters whose data type supports addition, which the generic data type variable does not express, as the comment above it already records. Extend the existing mypy suppression to ty.
Assigning the class that matches the default of the covariant channel type variable is rejected by mypy and pyright already, and ty agrees. Extend the existing suppression and note that all three flag it.
The on_cache_change mixin wraps the _update_with method of the cache of the parameter it is mixed into, so that it can detect changes. Patching a method on another object is inherently dynamic and already carried a mypy suppression, so extend it to ty.
Filtering the parameter chain with isinstance against the type variable should narrow the elements to C. ty instead widens the result to a union of C with the unnarrowed parameter type, but only when the class being narrowed is generic. mypy and pyright both narrow it correctly.
The keys of the extra columns are supplied by the caller at runtime, so they cannot be part of the closed RunOverviewDict definition, as the comment above already records. Extend the existing mypy suppression to ty.
The parameter was annotated as a bare type while ChannelList, which it forwards to, declares type[MultiChannelInstrumentParameter]. The docstring already states that it must be a subclass of that, so say so in the annotation.
Narrowing the value with isinstance does not tell either checker that it is the element type of the list, because the element type is a TypeVar bound to InstrumentModule. Extend the existing mypy suppression to ty and move the explanatory comment above the line it applies to.
The two exec mappings are built in mutually exclusive branches but shared a name, and only the first carried an annotation. ty takes the inferred type of the second, whose keys are plain bool tuples, so looking up a key that may be the literal "multi" was an error. Give the second mapping its own name and the same annotation.
Narrowing numpy_floats to a tuple of type[np.floating] made mypy join the element type of (float, *numpy_floats) to object, which is not a valid argument to register_adapter. ty and pyright both kept the union. Register float on its own so the loop element type stays a numpy float for all three checkers.
astral-sh/ty#4303 is fixed in 0.0.74, so passing the bounded class scoped type variable to the object parameter of __contains__ is no longer reported and ty flags the directive as unused.
get_ramp_values works in numbers while the value being set has the generic parameter data type, which the comment above already records and which mypy has always reported. The constraint solver changes in 0.0.74 mean ty now reports it too, so extend the existing suppression.
Both still reproduce on 0.0.74, unlike astral-sh/ty#4303 which that release fixes. Keeping the drafts alongside the suppressions they explain, so the repros stay with the code that needs them.
ty documents putting a ty rule into a mypy type: ignore comment by prefixing it with ty:. mypy does not recognise the prefixed code and reports it as unused when warn_unused_ignores is enabled, which we enable, so we use two comments on one line instead. Record the test case and the commands to run it, so the conclusion can be rechecked when either checker changes.
pyright honours mypy's type: ignore as a blanket suppression of its own rules, ignoring the codes in it, which is why removing a mypy suppression can surface a pyright error on the same line. It does not read ty: ignore at all. Also record why we cannot enable reportUnnecessaryTypeIgnoreComment: it calls a comment unnecessary whenever pyright itself has nothing to report, so every mypy only suppression would be flagged.
The templates are heterogeneous dict literals, so the inferred value type was a union of str and the nested dicts. Callers fill the template in by indexing into it, which meant every such assignment was an error because the str member of the union is not subscriptable. Annotate them as dict[str, Any], matching how export_data_as_json_linear and export_data_as_json_heatmap already type the state. This clears 18 of the 20 diagnostics in the subscriber json exporter notebook.
subscribe declared its callback as taking exactly three arguments, which contradicts its own callback_kwargs argument: those are bound onto the callback with functools.partial, so a callback using them takes more. Any documented use of callback_kwargs was therefore a type error. Type it as Callable[..., None], which is what _Subscriber, the thing subscribe forwards to, already uses.
self.module was built with dict.fromkeys, so its values were typed as possibly None even though scan_slots fills in every slot, either with the driver for the installed module or with a generic submodule. Every use of instrument.module[slot] therefore had to account for a None that cannot occur. Start from an empty dict of the submodule type and test membership rather than None, which keeps the behaviour of scan_slots unchanged for a repeated call. The notebook keeps one suppression: it sets _is_locked to demonstrate the safety interlock, and that attribute belongs to the 34934A driver rather than to the shared submodule base class.
The docstring states that the returned tuple matches the call signature of make_send_and_load_awg_file, but the declared type did not, so the documented round trip of parsing a file and sending it back was a type error throughout. The waveform and marker entries were declared as lists of dicts, but _parser3 appends parsed_wfmdict["wfm"], which _parser2 types as an ndarray. The loop counts and sequencing values were declared as possibly str when the parser only ever puts ints in them. Confirmed both by reading _parser2 and by running the parsers over a synthetic waveform.
instrument.parameters is a dict of ParameterBase, which does not carry a label. Parameter and ArrayParameter do, but MultiParameter has labels instead, so the listing would raise for an instrument holding one. Read it with getattr and a default, and say why in the notebook.
ty understands Jupyter notebooks, which mypy and pyright do not, so adding docs to the checked paths gives coverage of the examples that we have no other way to get. Also ignore unresolved imports in the plottr notebook, since plottr is a separate package that the notebook demonstrates integrating with rather than a dependency of qcodes. Note that this leaves ty reporting on the notebooks until the remaining findings are worked through.
by_kind and by_channel are keyed by ModuleKind and ChNr. Those are a StrEnum and an IntEnum, so a plain string or int is the same key at runtime, but the dicts are typed as taking the enums. Use constants.ModuleKind.SMU for the by_kind lookup, which is what the markdown just above it points at. The by_channel cell deliberately shows both the enum and the plain int and asserts they select the same module, so keep that and record why the second form is not typed.
The cell called run_iv_staircase_sweep.measurement_status(), which does not exist: measurement_status is a property of the SMU spot measurement parameters, while IVSweepMeasurement only gets status_summary from StatusMixin. The cell therefore raised AttributeError. It also did not do what the text around it says. The markdown before it asks for all channel outputs to be enabled before performing phase compensation, and the markdown after it continues with the second prerequisite, so call enable_channels instead. The old line looks copied from the status_summary cell earlier in the notebook.
The class exposes its two measured values as attributes named after the names of the measurement function, so capacitance exists for CPD and inductance for LPD. The class docstring documents this, but no checker can know the names, so the documented usage was an error everywhere it appeared. Declare a __getattr__ under TYPE_CHECKING. It is not defined at runtime, so accessing an attribute the current measurement function does not provide still raises the usual AttributeError, which the notebook prints in a cell demonstrating exactly that.
makeSEQXFile documents its wfms argument as the waveform arrays packed in lists, per channel and then per element. The notebook wrapped them in two further numpy arrays instead, which is not a Sequence of Sequences. Use lists, which is also clearer since the outer two levels are channel and element containers rather than numeric data. Verified that the method sees the same arrays either way, so the generated file is unchanged.
connect_paths, disconnect_paths and to_channel_list only iterate the paths once and never index them, so requiring a Sequence was stricter than the implementation. That made the example notebook, which passes a set of paths, a type error even though it works. Take an Iterable instead. Checked that a list, tuple, set and generator all produce a valid channel list. The order of the resulting list follows the iteration order of the argument, which does not matter for opening or closing a group of paths.
The path arguments were typed as list, which rejects even a tuple. Take a Collection instead, so a set works here as it now does on the B220X. Collection rather than Iterable because these methods walk the paths twice, once to validate each one and once to build the channel list, so a one shot iterator would be exhausted before the list was built. The 34934A override of to_channel_list is widened with the base, since an override may not accept less than what it overrides.
get_ydata is typed as returning ArrayLike, which includes Buffer and so is not necessarily sized, making len() on it a type error. Keep the appended array in a local and use that for both the y data and the length of the x axis. This also avoids reading the data back out of the line on every iteration, and gives the same lengths, which was checked against matplotlib. The same helper appears in the Lakeshore 325 notebook, so both are updated together.
plot_dataset returns a list of colorbars whose entries are None for the 1D plots, but its colorbars argument only accepted a sequence that was either all colorbars or all None. Passing the result back in, which is how the offline plotting tutorial plots into the same axes again, was therefore a type error. Take a Sequence[Colorbar | None]. A Sequence[Colorbar] is still one of those, so nothing that worked before stops working, and the body already built and handled lists containing None. plot_by_id forwards to plot_dataset and returns the same type, so it is widened with it.
The colorbar returned for a 1D plot is None, so the entry taken from the returned list has to be checked before its label is set. Doing that with an assert also documents that the entries are optional. Saving used Axes.figure, which matplotlib types as Figure or SubFigure, and a SubFigure has no savefig. Ask for the root figure instead.
snapshot_raw is documented as the way to get the snapshot of a run as a JSON string, and the snapshot notebooks use it, but it was declared only on DataSet. DataSetInMem carried the same data under the private _snapshot_raw, and the protocol declared only that, so reading it from the dataset a measurement hands back did not type check. Declare it on the protocol and add the public property to DataSetInMem, mirroring DataSet. This also removes the suppression that test_snapshot.py needed for exactly this, along with its comment saying the property is not part of the protocol.
A run only has a snapshot if one was recorded, so snapshot and snapshot_raw are both optional. The notebook indexed and passed them on without checking. Assert once where each is first read, which also tells the reader they are optional, and reuse the already checked value in the diff at the end rather than reading it from the dataset again.
Jens Hedegaard Nielsen (jenshnielsen)force-pushed
the
ty_0_73_support_1
branch
from
August 25, 2026 12:25
a7ccd46 to
f149aedCompare
This was referenced Aug 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WIP pr. Will be broken up to review in smaller bits