Repository navigation
Support type checking with TY - #8441
Draft
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 69 commits into
Draft
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 69 commits into
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 69 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8441 +/- ##
==========================================
+ Coverage 72.59% 72.61% +0.01%
==========================================
Files 307 307
Lines 32431 32448 +17
==========================================
+ Hits 23543 23561 +18
+ Misses 8888 8887 -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
6 times, most recently
from
September 17, 2026 17:45
f190351 to
c56103c
Compare
Jens Hedegaard Nielsen (jenshnielsen)
force-pushed
the
ty_0_73_support_1
branch
2 times, most recently
from
September 30, 2026 19:10
b0de75f to
3465fef
Compare
Jens Hedegaard Nielsen (jenshnielsen)
force-pushed
the
ty_0_73_support_1
branch
3 times, most recently
from
October 19, 2026 05:27
f075e4b to
96875e6
Compare
Jens Hedegaard Nielsen (jenshnielsen)
force-pushed
the
ty_0_73_support_1
branch
2 times, most recently
from
October 22, 2026 19:13
f3ed192 to
45ca4e2
Compare
callable() does not narrow int | Callable[[], int], since a subclass of int could define __call__, so ty infers calling the element as returning object. Checking isinstance(s, int) first lets ty narrow the remaining branch to the callable. Elements that are not callable are still used as they are, so numpy integers in a shape keep working at runtime.
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.
_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.
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 unpacking the result in the AMI430 example does not type check. Pass squeeze=False and take the first dataset.
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.
get_parameter_data only uses the name of each parameter, so it already accepts any ParamSpecBase at runtime, but it was annotated as taking a ParamSpec. Widen the annotation on the protocol and both implementations, so that the example notebook can pass ParamSpecBase objects. Also drop the cell in that notebook that displays dataset.parameters. The dataset there is a DataSetProtocol, which has no parameters attribute, and the cell before it already shows dataset.get_parameters().
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.
docs/conf.py replaces InstrumentMeta with ABCMeta, so that sphinx documents the signature of __init__ rather than __call__. ty rejects assigning a different class to the module attribute, which is the whole point of the patch.
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.
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 lines deliberately pass invalid arguments or subscripts, or mock attributes that are not declared on the annotated type, to check the runtime error handling. They are already suppressed for pyright.
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.
ty requires a legacy TypeVar to be assigned to a variable matching its name, so it rejects the ones these tests store straight into a dict. They are values handed to _make_deprecated_typevars_getattr rather than type variables used to parametrize anything, and both mypy and pyright accept them.
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.
The test cells imported the Weinschel8320 driver from qcodes, replacing the class of the same name that the notebook defines, which ty reports as an invalid assignment. Remove those imports so the tests use the driver from the notebook, and annotate the fixtures and tests. test_init_v2 requested a driver fixture that does not exist, so request weinschel_driver_2 instead. Also clear the stale output of the first cell.
IDN returns the dict produced by get_idn. Annotate it as Parameter[dict[str, str], Self]. Strictly the values may be None, but including that would require asserts at every use. The Weinschel 8320 test treated the result as a string. It only runs with an instance of the driver, so this went unnoticed. Check the vendor and model instead.
ty warns that testing a callable for truthiness is likely a missing call. Checking for None instead states what is meant and matches how ParameterBase itself guards the callback.
ty reports the fixtures that overwrite get and set on a Parameter subclass as invalid overrides of a mutable attribute. ParameterBase rejects that at runtime, and asserting it is the whole point of the fixtures, so they are invalid on purpose. The invalid types assigned in test_parameter_mixin and the float passed to get_level_code are deliberate too and were already suppressed for mypy.
The type annotations added to the tests on main come with pyright and mypy suppressions on the lines that pass invalid types on purpose, mostly to check runtime error handling. Suppress the corresponding ty errors too.
The test inputs these casts were narrowing are now annotated, so the values already have the type the casts asserted.
Annotate the spectrum data as one dimensional so iterating it yields scalars, annotate the list of ParamSpecDicts so the dict literals are checked against the TypedDict, and mock the ask method of the Keysight 34465A with patch.object rather than assigning to the method.
The values are stored in a list, so integer keys already worked at runtime.
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.
A draft report for the ty issue that requires the wider TParameter default, and notes on how mypy, pyright and ty read each other's suppression comments, with the test cases used to establish that. They are kept in a single commit so that they are easy to drop before merging.
Jens Hedegaard Nielsen (jenshnielsen)
force-pushed
the
ty_0_73_support_1
branch
from
October 22, 2026 19:40
45ca4e2 to
dc239ae
Compare
This branch has not been deployed
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