Repository navigation
Support type checking with TY - #8441
Draft
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 71 commits into
Draft
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 71 commits into
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 71 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8441 +/- ##
==========================================
+ Coverage 75.02% 75.04% +0.01%
==========================================
Files 307 307
Lines 32506 32523 +17
==========================================
+ Hits 24389 24407 +18
+ Misses 8117 8116 -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
4 times, most recently
from
October 26, 2026 06:05
dc239ae to
be73aed
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.
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.
The on/off parameters of these drivers are now annotated as bool, so pass True and False rather than the strings and integers that the on/off value mapping also accepts. The mapping sends the same command for both. The state of a Rigol DG1062 channel is annotated as str and sent as is, so pass "ON" to turn the output on.
These tests pass values that do not match the annotated value type of the parameter on purpose, either to check that they are rejected or that the strings and integers accepted by the on/off mapping still work. They are already suppressed for pyright.
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 28, 2026 06:04
be73aed to
b5fcdf0
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
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