Support type checking with TY - #8441
Draft
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 63 commits into
Draft
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 63 commits into
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 63 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8441 +/- ##
==========================================
+ Coverage 72.00% 72.02% +0.01%
==========================================
Files 305 305
Lines 32019 32036 +17
==========================================
+ Hits 23055 23073 +18
+ Misses 8964 8963 -1 ☔ 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
2 times, most recently
from
August 25, 2026 12:25
a7ccd46 to
f149aed
Compare
This was referenced Aug 26, 2026
Jens Hedegaard Nielsen (jenshnielsen)
force-pushed
the
ty_0_73_support_1
branch
5 times, most recently
from
August 29, 2026 06:00
948dee0 to
531515d
Compare
Jens Hedegaard Nielsen (jenshnielsen)
force-pushed
the
ty_0_73_support_1
branch
4 times, most recently
from
September 17, 2026 14:30
1be3142 to
c3f7a66
Compare
Scope 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.
Jens Hedegaard Nielsen (jenshnielsen)
force-pushed
the
ty_0_73_support_1
branch
from
September 17, 2026 14:34
c3f7a66 to
f190351
Compare
_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.
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.
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.
dond returns a single dataset rather than a tuple of them unless squeeze=False is passed, so notebooks that unpack the result could not be type checked. Pass squeeze=False and index into the result instead.
qc.config.current_config is optional, so the notebook has to establish that a configuration is loaded before reading values out of it.
Overriding set_raw with a differently named argument is an invalid override, since callers are free to pass value as a keyword.
The parameters in these examples read the point count off the instrument they belong to, which only the concrete instrument class declares. Override root_instrument to return that class, so that the examples state what they already assume.
set_netcdf_location exists on DataSetInMem but not on the protocol that load_by_run_spec returns, so the exporting notebook narrows the dataset before calling it. The walkthrough notebook loses the two cells reading DataSet.started and DataSet.parameters, which are likewise not part of the protocol.
A MultiParameter that measures more than one array returns a tuple of arrays. DataSaver.add_result has always unpacked such results, but ValuesType had no arm for them, so type checkers rejected the call.
Add ty to the test extra and run it next to mypy and pyright. ty understands Jupyter notebooks, which the other two do not, so this also covers the example notebooks in docs. The scipy stubs are added to the test extra for the notebooks that use scipy, and the paths to check are configured in pyproject.toml.
ty does not understand the error codes in a mypy type: ignore comment, so every deliberately wrong call in the test suite is reported twice. Add a matching ty: ignore comment on those lines. This is the mechanical part of getting the tests to type check with ty and leaves only the diagnostics that are not already suppressed for mypy.
These are calls that deliberately pass invalid arguments to check that they are rejected at runtime. They are only reported by pyright and ty since the enclosing test functions are untyped and therefore skipped by mypy.
The tuple only contains numpy types but was annotated as also containing the builtin complex. Since complex in an annotation implicitly means int or float or complex, that made calls such as complex_type(1 + 2j) be checked against int and float too.
Without an annotation ty infers the type of the module level constant from its default value, so reassigning WEBSOCKET_PORT to select another port, as the test suite does, is an error.
The element type of an unannotated dict or list literal is inferred from
its content, and containers are invariant, so a literal such as
{name: (11, 11)} is not a dict[str, tuple[int, ...]]. Declare the type
that the receiving function expects instead.
Parameter.step and DelegateParameter.source are optional and are read back through a property, so a type checker cannot know that the value assigned earlier in the test is still there.
The deprecation shim tests create TypeVar objects to hand to _make_deprecated_typevars_getattr and read a TypeVar that only the module level __getattr__ provides. Neither is something a type checker can follow, and mypy and pyright accept both without complaint.
One instrument in this test module deliberately assigns a parameter over a method, which is an error by design and now says so. The blanket ignore on the instrument that overrides a property is no longer needed by any of the three type checkers.
The wrapped callable is a function in practice but its declared type does not guarantee that. mypy and pyright both accept the attribute access.
Jens Hedegaard Nielsen (jenshnielsen)
force-pushed
the
ty_0_73_support_1
branch
from
September 17, 2026 17:45
f190351 to
c56103c
Compare
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 free
to 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