diff --git a/docs/changes/newsfragments/8360.underthehood b/docs/changes/newsfragments/8360.underthehood new file mode 100644 index 000000000000..b7ee1dd573a6 --- /dev/null +++ b/docs/changes/newsfragments/8360.underthehood @@ -0,0 +1,9 @@ +:class:`.Parameter` no longer replaces its own ``get_raw``/``set_raw`` methods +with the implementation generated from ``get_cmd``/``set_cmd``. The generated +implementation is stored on the parameter instead, and ``get_raw``/``set_raw`` +are now regular methods that dispatch to it. Assigning over the methods made +static type checkers infer ``get_raw``/``set_raw`` to be instance attributes of +:class:`.Parameter`, which in turn made every subclass implementing them as +regular methods an invalid override. There is no change in behaviour; note only +that ``parameter.get_raw`` is now always a bound method rather than, depending +on the arguments, a ``Command`` instance. diff --git a/docs/changes/newsfragments/8361.underthehood b/docs/changes/newsfragments/8361.underthehood new file mode 100644 index 000000000000..674cd1067ae2 --- /dev/null +++ b/docs/changes/newsfragments/8361.underthehood @@ -0,0 +1,9 @@ +The ``TParameter`` type variable used by :meth:`.InstrumentBase.add_parameter` +now defaults to ``Parameter[Any, Any]`` rather than to a bare ``Parameter``. +When ``add_parameter`` is called without an explicit ``parameter_class`` the +returned parameter is bound to the instrument it is added to, so the previous +default (which expands to ``Parameter[Any, InstrumentBase | None]``) wrongly +claimed that the instrument was ``InstrumentBase | None``. This made the result +unassignable to the ``Parameter[SomeType, Self]`` annotations that drivers use. +Code that relies on the inferred type of an unannotated +``instrument.add_parameter("name")`` will now see ``Parameter[Any, Any]``. diff --git a/docs/changes/newsfragments/8362.underthehood b/docs/changes/newsfragments/8362.underthehood new file mode 100644 index 000000000000..b1e9b3fd772c --- /dev/null +++ b/docs/changes/newsfragments/8362.underthehood @@ -0,0 +1,5 @@ +The legacy dataset importer now raises a clear :class:`ValueError` when a +setpoint array has no ``array_id``, instead of passing ``None`` on to +``add_result`` where a parameter name is expected. This was found by annotating +``DataArray`` in ``qcodes_loop``, which also removes the need for a ``pyright`` +suppression on the array shape. diff --git a/docs/changes/newsfragments/8363.underthehood b/docs/changes/newsfragments/8363.underthehood new file mode 100644 index 000000000000..c85e622e9290 --- /dev/null +++ b/docs/changes/newsfragments/8363.underthehood @@ -0,0 +1,6 @@ +The duck typed scale and offset conversions in ``ParameterBase`` have been +factored out into dedicated module level helpers. The conversions assume the +data type is numeric and rely on catching ``TypeError``, which does not fit the +generic parameter data type. Giving them an explicit boundary lets the rest of +the class stay properly typed and removes twelve type checker suppressions. +There is no change in behaviour. diff --git a/docs/changes/newsfragments/8364.underthehood b/docs/changes/newsfragments/8364.underthehood new file mode 100644 index 000000000000..168eaff90ec2 --- /dev/null +++ b/docs/changes/newsfragments/8364.underthehood @@ -0,0 +1,4 @@ +``CombinedParameter.parameter`` is now a small dataclass holding the ``name``, +``full_name``, ``label`` and ``unit`` of the combined parameter, replacing a +lambda that had those attributes attached to it. It remains callable, and +calling it returns ``None`` as before. diff --git a/docs/changes/newsfragments/8365.underthehood b/docs/changes/newsfragments/8365.underthehood new file mode 100644 index 000000000000..492f371ee712 --- /dev/null +++ b/docs/changes/newsfragments/8365.underthehood @@ -0,0 +1,6 @@ +Subclasses of ``ParameterBase`` that forward ``**kwargs`` on to their super +class now carry a ``ty: ignore[invalid-argument-type]``. When a generic TypedDict +declares a PEP 696 default for a type parameter, 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``, including expanding +with ``**``. The reason is documented on ``ParameterBaseKWArgs``. diff --git a/docs/changes/newsfragments/8366.underthehood b/docs/changes/newsfragments/8366.underthehood new file mode 100644 index 000000000000..21ba0d20655c --- /dev/null +++ b/docs/changes/newsfragments/8366.underthehood @@ -0,0 +1,5 @@ +The Infiniium driver now uses ``cast`` where it narrows ``root_instrument`` and +``instrument`` to the concrete driver classes, and where ``pyvisa`` types the +return of ``read_binary_values``/``query_binary_values`` as a ``Sequence[float]`` +regardless of the requested ``container``. This replaces five type checker +suppressions and has no effect at runtime. diff --git a/docs/changes/newsfragments/8367.underthehood b/docs/changes/newsfragments/8367.underthehood new file mode 100644 index 000000000000..81eef08c23d8 --- /dev/null +++ b/docs/changes/newsfragments/8367.underthehood @@ -0,0 +1,5 @@ +``DataSet._finalize_res_dict_standalones`` now appends to its result list +directly instead of building intermediate lists. The intermediate lists took +their element type from the branch that built them rather than from the +declaration, and ``dict`` is invariant in its value type, so the result was not +assignable back. There is no change in behaviour. diff --git a/docs/changes/newsfragments/8368.underthehood b/docs/changes/newsfragments/8368.underthehood new file mode 100644 index 000000000000..7cab22c1c784 --- /dev/null +++ b/docs/changes/newsfragments/8368.underthehood @@ -0,0 +1,4 @@ +The Alazar DLL wrapper no longer assumes that the callable handed to a ctypes +``errcheck`` has a ``__name__``, which a plain ``Callable`` does not guarantee. +The error message falls back to the repr of the callable instead. This only +affects the text of an error that should not occur in practice. diff --git a/docs/changes/newsfragments/8370.underthehood b/docs/changes/newsfragments/8370.underthehood new file mode 100644 index 000000000000..3317ec3768c5 --- /dev/null +++ b/docs/changes/newsfragments/8370.underthehood @@ -0,0 +1,5 @@ +``numpy_ints`` and ``numpy_floats`` in ``qcodes.utils.types`` are now annotated +as tuples of ``type[np.integer]`` and ``type[np.floating]`` rather than of bare +``type``. As a consequence ``_adapt_float``, which is registered as a sqlite +adapter for the numpy float types as well as for ``float``, now declares that it +accepts ``np.floating`` too. Its behaviour is unchanged. diff --git a/docs/changes/newsfragments/8371.underthehood b/docs/changes/newsfragments/8371.underthehood new file mode 100644 index 000000000000..10becb126c81 --- /dev/null +++ b/docs/changes/newsfragments/8371.underthehood @@ -0,0 +1,5 @@ +The ``__getattr__`` that provides backwards-compatible access to the old flat +parameter names on the Tektronix AWG5014 now names its parameter ``key``, +matching ``DelegateAttributes.__getattr__`` which it overrides and delegates to. +Python only ever calls ``__getattr__`` positionally, so this has no effect at +runtime. diff --git a/docs/changes/newsfragments/8373.underthehood b/docs/changes/newsfragments/8373.underthehood new file mode 100644 index 000000000000..e519a10b0793 --- /dev/null +++ b/docs/changes/newsfragments/8373.underthehood @@ -0,0 +1,5 @@ +``json_template_linear`` and ``json_template_heatmap`` in +``qcodes.dataset.json_exporter`` are now annotated as ``dict[str, Any]``. They +are templates for a JSON document, so their values are deliberately +heterogeneous, and without the annotation the inferred value type made indexing +into them an error for callers filling the template in. diff --git a/docs/changes/newsfragments/8374.improved b/docs/changes/newsfragments/8374.improved new file mode 100644 index 000000000000..e0c385c8e161 --- /dev/null +++ b/docs/changes/newsfragments/8374.improved @@ -0,0 +1,6 @@ +The ``callback`` argument of :meth:`.DataSet.subscribe` is now typed as +``Callable[..., None]``. The previous annotation described a callback taking +exactly three arguments, which contradicted ``callback_kwargs``: those are bound +onto the callback with ``functools.partial``, so a callback using them takes +further arguments. ``_Subscriber``, which ``subscribe`` forwards to, already +typed it this way. diff --git a/docs/changes/newsfragments/8375.improved b/docs/changes/newsfragments/8375.improved new file mode 100644 index 000000000000..b90716a9f224 --- /dev/null +++ b/docs/changes/newsfragments/8375.improved @@ -0,0 +1,7 @@ +``Keysight34980A.module`` is now a ``dict`` of +``Keysight34980ASwitchMatrixSubModule`` rather than one built with +``dict.fromkeys``, whose values were typed as possibly ``None``. ``scan_slots`` +puts an entry in for every slot, either the driver for the installed module or a +generic submodule, so the values were never ``None`` once the instrument was +constructed. Code using ``instrument.module[slot]`` no longer has to account for +a ``None`` that cannot occur. diff --git a/docs/changes/newsfragments/8376.improved b/docs/changes/newsfragments/8376.improved new file mode 100644 index 000000000000..0437c9e88e28 --- /dev/null +++ b/docs/changes/newsfragments/8376.improved @@ -0,0 +1,6 @@ +The return type of :func:`.parse_awg_file` has been corrected. The waveform and +marker entries were declared as lists of dicts, but the parser returns the arrays +from inside those dicts, and the loop counts and sequencing values were declared +as possibly ``str`` when they are always ``int``. The type now matches the call +signature of :meth:`.TektronixAWG5014.make_send_and_load_awg_file`, which the +docstring already promised and which is how the function is meant to be used. diff --git a/docs/changes/newsfragments/8377.underthehood b/docs/changes/newsfragments/8377.underthehood new file mode 100644 index 000000000000..73cf4961e3be --- /dev/null +++ b/docs/changes/newsfragments/8377.underthehood @@ -0,0 +1,3 @@ +``ty`` now also type checks the example notebooks in ``docs``. Unlike mypy and +pyright it understands Jupyter notebooks, so this is coverage that the other two +checkers do not provide. diff --git a/docs/changes/newsfragments/8381.improved b/docs/changes/newsfragments/8381.improved new file mode 100644 index 000000000000..b124c324651d --- /dev/null +++ b/docs/changes/newsfragments/8381.improved @@ -0,0 +1,6 @@ +The Keysight B1500 example notebook called +``b1500.run_iv_staircase_sweep.measurement_status()`` in the phase compensation +section. ``IVSweepMeasurement`` has no such method, so the cell raised +``AttributeError``. The surrounding text asks for all channel outputs to be +enabled before performing phase compensation, so the cell now calls +``b1500.enable_channels()``. diff --git a/docs/changes/newsfragments/8382.improved b/docs/changes/newsfragments/8382.improved new file mode 100644 index 000000000000..3727910a2a5c --- /dev/null +++ b/docs/changes/newsfragments/8382.improved @@ -0,0 +1,7 @@ +``KeysightE4980AMeasurementPair`` now declares a ``__getattr__`` for type +checkers. The two measured values are exposed as attributes named after the +``names`` of the measurement function, for example ``capacitance`` for ``CPD`` +and ``inductance`` for ``LPD``, so which attributes exist is only known at +runtime. The declaration lets this documented usage be written in typed code. It +is not defined at runtime, so accessing an attribute that the current +measurement function does not provide still raises the usual ``AttributeError``. diff --git a/docs/changes/newsfragments/8384.improved b/docs/changes/newsfragments/8384.improved new file mode 100644 index 000000000000..786b261bb836 --- /dev/null +++ b/docs/changes/newsfragments/8384.improved @@ -0,0 +1,4 @@ +The Stanford SR86x buffered readout example notebook now packs the waveforms for +:meth:`.TektronixAWG70000Base.makeSEQXFile` in lists rather than wrapping them in +further numpy arrays, which is the shape the method documents. The two forms +behave the same at runtime. diff --git a/docs/changes/newsfragments/8385.improved b/docs/changes/newsfragments/8385.improved new file mode 100644 index 000000000000..ffc16e01e346 --- /dev/null +++ b/docs/changes/newsfragments/8385.improved @@ -0,0 +1,6 @@ +``connect_paths``, ``disconnect_paths`` and ``to_channel_list`` on the Keysight +B220X switch matrix drivers now accept any iterable of paths rather than only a +``Sequence``. They iterate the paths once and do not index them, so passing a +set, as the example notebook does, is fine. Note that the order in which the +paths appear in the channel list then follows the iteration order of the +argument. diff --git a/docs/changes/newsfragments/8386.improved b/docs/changes/newsfragments/8386.improved new file mode 100644 index 000000000000..60fc859b5b8c --- /dev/null +++ b/docs/changes/newsfragments/8386.improved @@ -0,0 +1,6 @@ +The path arguments of ``connect_paths``, ``disconnect_paths``, ``are_closed``, +``are_open`` and ``to_channel_list`` on the Keysight 34980A switch matrix +submodules are now typed as a ``Collection`` rather than a ``list``, so a set or +a tuple of paths is accepted as well. A ``Collection`` rather than an +``Iterable`` because these methods walk the paths twice, once to validate them +and once to build the channel list, which a one shot iterator would not survive. diff --git a/docs/changes/newsfragments/8387.improved b/docs/changes/newsfragments/8387.improved new file mode 100644 index 000000000000..7d3920c0da1d --- /dev/null +++ b/docs/changes/newsfragments/8387.improved @@ -0,0 +1,5 @@ +The live temperature plot helper in the two Lakeshore example notebooks keeps +the appended y data in a local variable instead of reading it back with +``Line2D.get_ydata``. The return of ``get_ydata`` is typed as ``ArrayLike``, +which is not necessarily sized, so taking its length was a type error. This also +avoids reading the data back from the line on every iteration. diff --git a/docs/changes/newsfragments/8388.improved b/docs/changes/newsfragments/8388.improved new file mode 100644 index 000000000000..83aa40c75dd6 --- /dev/null +++ b/docs/changes/newsfragments/8388.improved @@ -0,0 +1,5 @@ +The ``colorbars`` argument of :func:`.plot_dataset` and :func:`.plot_by_id` now +accepts a sequence that may contain ``None``. Both functions return a list of +colorbars in which the entries for 1D plots are ``None``, so passing the result +back in, which is how you plot into the same axes again, did not match the +declared argument type. A sequence of colorbars is still accepted. diff --git a/docs/changes/newsfragments/8389.improved b/docs/changes/newsfragments/8389.improved new file mode 100644 index 000000000000..b2f6f471d3d8 --- /dev/null +++ b/docs/changes/newsfragments/8389.improved @@ -0,0 +1,3 @@ +The offline plotting tutorial now checks the optional values it gets back from +:func:`.plot_dataset` before using them, and asks for the root figure when +saving. ``Axes.figure`` may be a ``SubFigure``, which has no ``savefig``. diff --git a/docs/changes/newsfragments/8390.improved b/docs/changes/newsfragments/8390.improved new file mode 100644 index 000000000000..080e53462aae --- /dev/null +++ b/docs/changes/newsfragments/8390.improved @@ -0,0 +1,5 @@ +``snapshot_raw`` is now part of :class:`.DataSetProtocol` and is available on +:class:`.DataSetInMem` as well as on :class:`.DataSet`. It is documented as the +way to get the snapshot of a run as a JSON string, and is used as such in the +example notebooks, but it was only declared on one of the two dataset classes, +so reading it from a dataset returned by a measurement did not type check. diff --git a/docs/changes/newsfragments/8391.improved b/docs/changes/newsfragments/8391.improved new file mode 100644 index 000000000000..04a44aad1a7c --- /dev/null +++ b/docs/changes/newsfragments/8391.improved @@ -0,0 +1,3 @@ +The snapshot example notebook now checks that the snapshots it reads back from +the datasets are present before using them. A run only has a snapshot if one was +recorded, so both ``snapshot`` and ``snapshot_raw`` are optional. diff --git a/docs/examples/DataSet/Offline Plotting Tutorial.ipynb b/docs/examples/DataSet/Offline Plotting Tutorial.ipynb index dfddf0e41b06..164fafe2ed99 100644 --- a/docs/examples/DataSet/Offline Plotting Tutorial.ipynb +++ b/docs/examples/DataSet/Offline Plotting Tutorial.ipynb @@ -477,6 +477,8 @@ "outputs": [], "source": [ "colorbar = colorbars[0]\n", + "# 2D plots have a colorbar, 1D plots do not, so the entries are optional\n", + "assert colorbar is not None\n", "colorbar.set_label(\"Correct science label\")" ] }, @@ -939,9 +941,11 @@ "source": [ "%%time\n", "axeslist, _ = plot_dataset(dataset)\n", - "axeslist[0].figure.savefig(\n", - " Path.cwd().parent / \"example_output\" / f\"test_plot_dataset_{dataid}.pdf\"\n", - ")" + "# Axes.figure may be a SubFigure, which cannot be saved, so ask for the\n", + "# root figure\n", + "figure = axeslist[0].get_figure(root=True)\n", + "assert figure is not None\n", + "figure.savefig(Path.cwd().parent / \"example_output\" / f\"test_plot_dataset_{dataid}.pdf\")" ] }, { @@ -971,9 +975,11 @@ "source": [ "%%time\n", "axeslist, _ = plot_dataset(dataset, rasterized=False)\n", - "axeslist[0].figure.savefig(\n", - " Path.cwd().parent / \"example_output\" / f\"test_plot_dataset_{dataid}.pdf\"\n", - ")" + "# Axes.figure may be a SubFigure, which cannot be saved, so ask for the\n", + "# root figure\n", + "figure = axeslist[0].get_figure(root=True)\n", + "assert figure is not None\n", + "figure.savefig(Path.cwd().parent / \"example_output\" / f\"test_plot_dataset_{dataid}.pdf\")" ] } ], diff --git a/docs/examples/DataSet/Working with snapshots.ipynb b/docs/examples/DataSet/Working with snapshots.ipynb index 49d42c62d4e6..801126e0a1b4 100644 --- a/docs/examples/DataSet/Working with snapshots.ipynb +++ b/docs/examples/DataSet/Working with snapshots.ipynb @@ -593,7 +593,9 @@ "metadata": {}, "outputs": [], "source": [ - "snapshot_of_run = dataset.snapshot" + "snapshot_of_run = dataset.snapshot\n", + "# a run only has a snapshot if one was recorded, this one has\n", + "assert snapshot_of_run is not None" ] }, { @@ -602,7 +604,8 @@ "metadata": {}, "outputs": [], "source": [ - "snapshot_of_run_in_json_format = dataset.snapshot_raw" + "snapshot_of_run_in_json_format = dataset.snapshot_raw\n", + "assert snapshot_of_run_in_json_format is not None" ] }, { @@ -881,7 +884,10 @@ "metadata": {}, "outputs": [], "source": [ - "diff_param_values(dataset.snapshot, bad_dataset.snapshot).changed" + "snapshot_of_bad_run = bad_dataset.snapshot\n", + "assert snapshot_of_bad_run is not None\n", + "\n", + "diff_param_values(snapshot_of_run, snapshot_of_bad_run).changed" ] }, { diff --git a/docs/examples/driver_examples/Qcodes example with Keysight 34980A Switch Mainframe and Modules.ipynb b/docs/examples/driver_examples/Qcodes example with Keysight 34980A Switch Mainframe and Modules.ipynb index 3726074aa3ee..b1a64ba32a6a 100644 --- a/docs/examples/driver_examples/Qcodes example with Keysight 34980A Switch Mainframe and Modules.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Keysight 34980A Switch Mainframe and Modules.ipynb @@ -428,9 +428,10 @@ "metadata": {}, "outputs": [], "source": [ - "switch_matrix.module[\n", - " 2\n", - "]._is_locked = True # DO NOT perform this action in real situation" + "# DO NOT perform this action in a real situation. ``_is_locked`` is defined\n", + "# on the 34934A driver rather than on the shared submodule base class that\n", + "# ``module`` is typed as, hence the suppression.\n", + "switch_matrix.module[2]._is_locked = True # ty: ignore[unresolved-attribute]" ] }, { diff --git a/docs/examples/driver_examples/Qcodes example with Keysight B1500 Parameter Analyzer.ipynb b/docs/examples/driver_examples/Qcodes example with Keysight B1500 Parameter Analyzer.ipynb index fffbf3d12229..e5ee5a34304d 100644 --- a/docs/examples/driver_examples/Qcodes example with Keysight B1500 Parameter Analyzer.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Keysight B1500 Parameter Analyzer.ipynb @@ -288,7 +288,7 @@ "metadata": {}, "outputs": [], "source": [ - "b1500.by_kind[\"SMU\"]" + "b1500.by_kind[constants.ModuleKind.SMU]" ] }, { @@ -331,8 +331,9 @@ "# Selecting a module by channel number using the Enum\n", "m1 = b1500.by_channel[constants.ChNr.SLOT_01_CH1]\n", "\n", - "# Without enum\n", - "m2 = b1500.by_channel[1]\n", + "# Without enum. ChNr is an IntEnum, so a plain int is the same key at\n", + "# runtime, but the dict is typed as taking ChNr.\n", + "m2 = b1500.by_channel[1] # ty: ignore[invalid-argument-type]\n", "\n", "# And we assert that we selected the same module:\n", "assert m1 is m2" @@ -1118,7 +1119,8 @@ "metadata": {}, "outputs": [], "source": [ - "b1500.run_iv_staircase_sweep.measurement_status()" + "# enable all channel outputs\n", + "b1500.enable_channels()" ] }, { diff --git a/docs/examples/driver_examples/Qcodes example with Lakeshore 325.ipynb b/docs/examples/driver_examples/Qcodes example with Lakeshore 325.ipynb index 44a36f8baed8..042ed8d79c90 100644 --- a/docs/examples/driver_examples/Qcodes example with Lakeshore 325.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Lakeshore 325.ipynb @@ -517,8 +517,9 @@ " text.value = f\"T = {channel_to_read.temperature()}\"\n", "\n", " # Add new point to the data that is being plotted\n", - " line.set_ydata(numpy.append(line.get_ydata(), channel_to_read.temperature()))\n", - " line.set_xdata(numpy.arange(0, len(line.get_ydata()), 1) * read_period)\n", + " ydata = numpy.append(line.get_ydata(), channel_to_read.temperature())\n", + " line.set_ydata(ydata)\n", + " line.set_xdata(numpy.arange(0, len(ydata), 1) * read_period)\n", "\n", " ax.relim() # Recalculate limits\n", " ax.autoscale_view(True, True, True) # Autoscale\n", diff --git a/docs/examples/driver_examples/Qcodes example with Lakeshore 336 or 372 - Bluefors T control.ipynb b/docs/examples/driver_examples/Qcodes example with Lakeshore 336 or 372 - Bluefors T control.ipynb index 8837ebb9e9fe..b8c566172e2a 100644 --- a/docs/examples/driver_examples/Qcodes example with Lakeshore 336 or 372 - Bluefors T control.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Lakeshore 336 or 372 - Bluefors T control.ipynb @@ -508,8 +508,9 @@ " text.value = f\"T = {channel_to_read.temperature()}\"\n", "\n", " # Add new point to the data that is being plotted\n", - " line.set_ydata(numpy.append(line.get_ydata(), channel_to_read.temperature()))\n", - " line.set_xdata(numpy.arange(0, len(line.get_ydata()), 1) * read_period)\n", + " ydata = numpy.append(line.get_ydata(), channel_to_read.temperature())\n", + " line.set_ydata(ydata)\n", + " line.set_xdata(numpy.arange(0, len(ydata), 1) * read_period)\n", "\n", " ax.relim() # Recalculate limits\n", " ax.autoscale_view(True, True, True) # Autoscale\n", diff --git a/docs/examples/driver_examples/Qcodes example with Stanford SR86x with buffered readout.ipynb b/docs/examples/driver_examples/Qcodes example with Stanford SR86x with buffered readout.ipynb index 4adf9855f8f2..1f8aae138e10 100644 --- a/docs/examples/driver_examples/Qcodes example with Stanford SR86x with buffered readout.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Stanford SR86x with buffered readout.ipynb @@ -683,8 +683,10 @@ "# (3000 samples for 3000 S/s sample rate)\n", "waveform_ch1[1, :-1500] = 1 # falling from 1 to 0 (a.u.),\n", "# at 0.5s after the start of the waveform\n", - "elements = numpy.array([waveform_ch1]) # we only have one element in the sequence\n", - "waveforms = numpy.array([elements]) # we will use only 1 channel\n", + "# makeSEQXFile takes the waveform arrays packed in lists, per channel and\n", + "# then per element, rather than in a further numpy array\n", + "elements = [waveform_ch1] # we only have one element in the sequence\n", + "waveforms = [elements] # we will use only 1 channel\n", "\n", "# Create a sequence file from the \"waveform\" array\n", "seq_name = \"single_trigger_marker_1\"\n", @@ -929,8 +931,10 @@ " n_trigger_pulses,\n", ") # falling from 1 to 0 (a.u.) every 0.01s after the start of the waveform\n", "\n", - "elements = numpy.array([waveform_ch1]) # we only have one element in the sequence\n", - "waveforms = numpy.array([elements]) # we will use only 1 channel\n", + "# makeSEQXFile takes the waveform arrays packed in lists, per channel and\n", + "# then per element, rather than in a further numpy array\n", + "elements = [waveform_ch1] # we only have one element in the sequence\n", + "waveforms = [elements] # we will use only 1 channel\n", "\n", "# Create a sequence file from the \"waveform\" array\n", "seq_name = \"single_trigger_marker_1\"\n", diff --git a/docs/examples/driver_examples/Qcodes example with Tektronix AWG5014C.ipynb b/docs/examples/driver_examples/Qcodes example with Tektronix AWG5014C.ipynb index abe47966ccc7..494e9a4697a2 100644 --- a/docs/examples/driver_examples/Qcodes example with Tektronix AWG5014C.ipynb +++ b/docs/examples/driver_examples/Qcodes example with Tektronix AWG5014C.ipynb @@ -100,19 +100,23 @@ "metadata": {}, "outputs": [], "source": [ + "# ``instrument.parameters`` is a dict of ``ParameterBase``, and not every\n", + "# parameter type carries a ``label``: ``MultiParameter`` has ``labels`` instead.\n", + "# Fall back to an empty string so this works for any parameter.\n", + "\n", "# Top-level parameters\n", "for name in sorted(awg1.parameters):\n", - " print(name, \": \", awg1.parameters[name].label)\n", + " print(name, \": \", getattr(awg1.parameters[name], \"label\", \"\"))\n", "\n", "# Channel parameters (e.g. ch1)\n", "print(\"\\nChannel 1 parameters:\")\n", "for name in sorted(awg1.ch1.parameters):\n", - " print(f\" ch1.{name}: \", awg1.ch1.parameters[name].label)\n", + " print(f\" ch1.{name}: \", getattr(awg1.ch1.parameters[name], \"label\", \"\"))\n", "\n", "# Marker parameters (e.g. ch1.m1)\n", "print(\"\\nChannel 1 Marker 1 parameters:\")\n", "for name in sorted(awg1.ch1.m1.parameters):\n", - " print(f\" ch1.m1.{name}: \", awg1.ch1.m1.parameters[name].label)" + " print(f\" ch1.m1.{name}: \", getattr(awg1.ch1.m1.parameters[name], \"label\", \"\"))" ] }, { diff --git a/pyproject.toml b/pyproject.toml index 0ff7239f30e9..7b0055e36fd8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -293,6 +293,48 @@ quote-annotations = true sdist = "versioningit.cmdclass.sdist" build_py = "versioningit.cmdclass.build_py" +[tool.ty.environment] +# a number of drivers are only usable on Windows. Checking against all +# platforms means that these are type checked no matter which platform ty +# runs on, and that the result does not depend on the platform of the developer. +python-platform = "all" + +[tool.ty.src] +# unlike pyright above, ty also understands Jupyter notebooks, so the example +# notebooks in docs are checked too. That is coverage we get from ty alone. +include = ["src", "tests", "docs"] +exclude = [ + "src/qcodes/instrument_drivers/Harvard/Decadac.py", + ] + +# these are packages that we import +# but don't have installed by default. +# Compare with ignore_missing_imports in the mypy config above +[[tool.ty.overrides]] +include = [ + "src/qcodes/instrument_drivers/Galil/dmc_41x3.py", + "src/qcodes/instrument_drivers/Minicircuits/USBHIDMixin.py", + "src/qcodes/instrument_drivers/Minicircuits/_minicircuits_usb_spdt.py", +] +[tool.ty.overrides.rules] +unresolved-import = "ignore" + +# clr is provided by pythonnet which is not installed by default +# so its members cannot be resolved either +[[tool.ty.overrides]] +include = ["src/qcodes/instrument_drivers/Minicircuits/_minicircuits_usb_spdt.py"] +[tool.ty.overrides.rules] +unresolved-attribute = "ignore" + +# plottr is a separate package that this notebook demonstrates integrating with, +# it is not a dependency of qcodes +[[tool.ty.overrides]] +include = [ + "docs/examples/plotting/How-to-use-Plottr-with-QCoDeS-for-live-plotting.ipynb", +] +[tool.ty.overrides.rules] +unresolved-import = "ignore" + [tool.towncrier] package = "qcodes" name = "QCoDeS" diff --git a/src/qcodes/dataset/data_set.py b/src/qcodes/dataset/data_set.py index f36ad008772f..868eed6705d5 100644 --- a/src/qcodes/dataset/data_set.py +++ b/src/qcodes/dataset/data_set.py @@ -1144,7 +1144,11 @@ def write_data_to_text_file( def subscribe( self, - callback: Callable[[Any, int, Any | None], None], + # ``Callable[..., None]`` rather than a three argument callable because + # ``callback_kwargs`` below is bound onto the callback with + # ``functools.partial``, so it may take further keyword arguments. This + # matches how ``_Subscriber`` types the same callback. + callback: Callable[..., None], min_wait: int = 0, min_count: int = 1, state: Any | None = None, @@ -1406,28 +1410,21 @@ def _finalize_res_dict_standalones( for param, value in result_dict.items(): if param.type == "text": if value.shape: - new_res: list[dict[str, VALUE]] = [ - {param.name: str(val)} for val in value - ] - res_list += new_res + res_list.extend({param.name: str(val)} for val in value) else: - new_res = [{param.name: str(value)}] - res_list += new_res + res_list.append({param.name: str(value)}) elif param.type == "numeric": if value.shape: - res_list += [{param.name: number} for number in value] + res_list.extend({param.name: number} for number in value) else: - new_res = [{param.name: float(value)}] - res_list += new_res + res_list.append({param.name: float(value)}) elif param.type == "complex": if value.shape: - res_list += [{param.name: number} for number in value] + res_list.extend({param.name: number} for number in value) else: - new_res = [{param.name: complex(value)}] - res_list += new_res + res_list.append({param.name: complex(value)}) else: - new_res = [{param.name: value}] - res_list += new_res + res_list.append({param.name: value}) return res_list diff --git a/src/qcodes/dataset/data_set_in_memory.py b/src/qcodes/dataset/data_set_in_memory.py index 241b789ba3bb..5fe228c189f2 100644 --- a/src/qcodes/dataset/data_set_in_memory.py +++ b/src/qcodes/dataset/data_set_in_memory.py @@ -595,6 +595,11 @@ def _snapshot_raw(self) -> str | None: """Snapshot of the run as a JSON-formatted string (or None).""" return self._snapshot_raw_data + @property + def snapshot_raw(self) -> str | None: + """Snapshot of the run as a JSON-formatted string (or None).""" + return self._snapshot_raw + def add_metadata(self, tag: str, metadata: Any) -> None: """ Adds metadata to the :class:`.DataSet`. diff --git a/src/qcodes/dataset/data_set_protocol.py b/src/qcodes/dataset/data_set_protocol.py index cd31082e8809..339e11ab5a7a 100644 --- a/src/qcodes/dataset/data_set_protocol.py +++ b/src/qcodes/dataset/data_set_protocol.py @@ -168,6 +168,9 @@ def add_snapshot(self, snapshot: str, overwrite: bool = False) -> None: ... @property def _snapshot_raw(self) -> str | None: ... + @property + def snapshot_raw(self) -> str | None: ... + def add_metadata(self, tag: str, metadata: Any) -> None: ... @property diff --git a/src/qcodes/dataset/descriptions/param_spec.py b/src/qcodes/dataset/descriptions/param_spec.py index 8a965080cca6..d455565b80e2 100644 --- a/src/qcodes/dataset/descriptions/param_spec.py +++ b/src/qcodes/dataset/descriptions/param_spec.py @@ -181,7 +181,7 @@ def base_version(self) -> _ParamSpecBase: ) @classmethod - def _from_dict(cls, ser: ParamSpecDict) -> ParamSpec: # type: ignore[override] + def _from_dict(cls, ser: ParamSpecDict) -> ParamSpec: # type: ignore[override] # ty: ignore[invalid-method-override] """ Create a ParamSpec instance of the current version from a dictionary representation of ParamSpec of some version diff --git a/src/qcodes/dataset/json_exporter.py b/src/qcodes/dataset/json_exporter.py index dcf4ac3fef19..6c60aa79e4d1 100644 --- a/src/qcodes/dataset/json_exporter.py +++ b/src/qcodes/dataset/json_exporter.py @@ -8,13 +8,17 @@ if TYPE_CHECKING: from collections.abc import Mapping -json_template_linear = { +# These are templates for a JSON document, so the values are deliberately +# heterogeneous and consumers index arbitrarily deep into them. Annotating the +# value type as ``Any`` matches how ``export_data_as_json_*`` below already +# types the state they are copied into. +json_template_linear: dict[str, Any] = { "type": "linear", "x": {"data": [], "name": "", "full_name": "", "is_setpoint": True, "unit": ""}, "y": {"data": [], "name": "", "full_name": "", "is_setpoint": False, "unit": ""}, } -json_template_heatmap = { +json_template_heatmap: dict[str, Any] = { "type": "heatmap", "x": {"data": [], "name": "", "full_name": "", "is_setpoint": True, "unit": ""}, "y": {"data": [], "name": "", "full_name": "", "is_setpoint": True, "unit": ""}, diff --git a/src/qcodes/dataset/legacy_import.py b/src/qcodes/dataset/legacy_import.py index babd3955209e..22cd030ce436 100644 --- a/src/qcodes/dataset/legacy_import.py +++ b/src/qcodes/dataset/legacy_import.py @@ -44,23 +44,45 @@ def setup_measurement( return meas +def _array_id(array: DataArray) -> str: + """ + Return the ``array_id`` of a legacy ``DataArray``. + + Args: + array: Legacy data array to read the id from. + + Raises: + ValueError: If the array has no ``array_id``. Parameters are registered + by name, so an array without an id cannot be stored. + + """ + array_id = array.array_id + if array_id is None: + raise ValueError(f"Cannot store an array without an array_id: {array!r}") + return array_id + + def store_array_to_database(datasaver: DataSaver, array: DataArray) -> int: assert array.shape is not None dims = len(array.shape) assert array.array_id is not None if dims == 2: - for index1, i in enumerate(array.set_arrays[0]): - for index2, j in enumerate(array.set_arrays[1][index1]): + setpoints_outer = array.set_arrays[0] + setpoints_inner = array.set_arrays[1] + outer_id = _array_id(setpoints_outer) + inner_id = _array_id(setpoints_inner) + for index1, i in enumerate(setpoints_outer): + for index2, j in enumerate(setpoints_inner[index1]): datasaver.add_result( - (array.set_arrays[0].array_id, i), - (array.set_arrays[1].array_id, j), + (outer_id, i), + (inner_id, j), (array.array_id, array[index1, index2]), ) elif dims == 1: - for index, i in enumerate(array.set_arrays[0]): - datasaver.add_result( - (array.set_arrays[0].array_id, i), (array.array_id, array[index]) - ) + setpoints = array.set_arrays[0] + setpoints_id = _array_id(setpoints) + for index, i in enumerate(setpoints): + datasaver.add_result((setpoints_id, i), (array.array_id, array[index])) else: raise NotImplementedError( "The exporter only currently handles 1 and 2 Dimensional data" @@ -73,23 +95,25 @@ def store_array_to_database_alt(meas: Measurement, array: DataArray) -> int: dims = len(array.shape) assert array.array_id is not None if dims == 2: - outer_data = np.empty( - array.shape[1] # pyright: ignore[reportGeneralTypeIssues] - ) + setpoints_outer = array.set_arrays[0] + setpoints_inner = array.set_arrays[1] + outer_id = _array_id(setpoints_outer) + inner_id = _array_id(setpoints_inner) + outer_data = np.empty(array.shape[1]) with meas.run() as datasaver: - for index1, i in enumerate(array.set_arrays[0]): + for index1, i in enumerate(setpoints_outer): outer_data[:] = i datasaver.add_result( - (array.set_arrays[0].array_id, outer_data), - (array.set_arrays[1].array_id, array.set_arrays[1][index1, :]), + (outer_id, outer_data), + (inner_id, setpoints_inner[index1, :]), (array.array_id, array[index1, :]), ) elif dims == 1: + setpoints = array.set_arrays[0] + setpoints_id = _array_id(setpoints) with meas.run() as datasaver: - for index, i in enumerate(array.set_arrays[0]): - datasaver.add_result( - (array.set_arrays[0].array_id, i), (array.array_id, array[index]) - ) + for index, i in enumerate(setpoints): + datasaver.add_result((setpoints_id, i), (array.array_id, array[index])) else: raise NotImplementedError( "The exporter only currently handles 1 and 2 Dimensional data" diff --git a/src/qcodes/dataset/plotting.py b/src/qcodes/dataset/plotting.py index 6145968e7822..11c5ee79c351 100644 --- a/src/qcodes/dataset/plotting.py +++ b/src/qcodes/dataset/plotting.py @@ -95,7 +95,10 @@ def heatmaphandler(**kwargs: Any) -> Any: def plot_dataset( dataset: DataSetProtocol, axes: Axes | Sequence[Axes] | None = None, - colorbars: Colorbar | Sequence[Colorbar] | Sequence[None] | None = None, + # ``Sequence[Colorbar | None]`` so that the list of colorbars returned by + # this function can be passed straight back in, which is how you plot into + # the same axes again. A ``Sequence[Colorbar]`` is also one of these. + colorbars: Colorbar | Sequence[Colorbar | None] | None = None, rescale_axes: bool = True, auto_color_scale: bool | None = None, cutoff_percentile: tuple[float, float] | float | None = None, @@ -417,7 +420,7 @@ def plot_and_save_image( def plot_by_id( run_id: int, axes: Axes | Sequence[Axes] | None = None, - colorbars: Colorbar | Sequence[Colorbar] | None = None, + colorbars: Colorbar | Sequence[Colorbar | None] | None = None, rescale_axes: bool = True, auto_color_scale: bool | None = None, cutoff_percentile: tuple[float, float] | float | None = None, diff --git a/src/qcodes/dataset/sqlite/database.py b/src/qcodes/dataset/sqlite/database.py index a0d17507babf..f3c9be83a0b3 100644 --- a/src/qcodes/dataset/sqlite/database.py +++ b/src/qcodes/dataset/sqlite/database.py @@ -105,7 +105,7 @@ def _convert_numeric(value: bytes) -> float | int | str: return numeric_int -def _adapt_float(fl: float) -> float | str: +def _adapt_float(fl: float | np.floating) -> float | str: # For a single value, math.isnan is 10 times faster than np.isnan # Overall, saving floats with numeric format is 2 times faster with math.isnan if math.isnan(fl): @@ -174,7 +174,10 @@ def connect( sqlite3.register_converter("numeric", _convert_numeric) - for numpy_float in (float, *numpy_floats): + # registered separately from the numpy floats below, so that the element + # type of the loop stays a numpy float rather than widening to object + sqlite3.register_adapter(float, _adapt_float) + for numpy_float in numpy_floats: sqlite3.register_adapter(numpy_float, _adapt_float) for complex_type in complex_types: diff --git a/src/qcodes/dataset/sqlite/db_overview.py b/src/qcodes/dataset/sqlite/db_overview.py index e596439f46aa..590a94fb18a9 100644 --- a/src/qcodes/dataset/sqlite/db_overview.py +++ b/src/qcodes/dataset/sqlite/db_overview.py @@ -253,7 +253,7 @@ def get_db_overview( # The keys of ``extra`` are only known at runtime (they are the # user-supplied ``extra_columns``), so they cannot be part of # the closed ``RunOverviewDict`` definition. - entry.update(extra) # type: ignore[typeddict-item] + entry.update(extra) # type: ignore[typeddict-item] # ty: ignore[invalid-argument-type] overview[run_id] = entry diff --git a/src/qcodes/extensions/infer.py b/src/qcodes/extensions/infer.py index 3abc0d3ff0bd..80c002857f5c 100644 --- a/src/qcodes/extensions/infer.py +++ b/src/qcodes/extensions/infer.py @@ -226,7 +226,10 @@ def get_chain_links_of_type[C: ParameterBase]( link_param_type: type[C] | tuple[type[C], ...], parameter: Parameter ) -> tuple[C, ...]: """Gets all parameters in a chain of linked parameters that match a given type""" - chain_links: list[C] = [ + # ty does not narrow the element type to C here: for a generic parameter + # class it widens the isinstance narrowing to a union with the unnarrowed + # type. The equivalent non generic code narrows correctly. + chain_links: list[C] = [ # ty: ignore[invalid-assignment] param for param in get_parameter_chain(parameter) if isinstance(param, link_param_type) diff --git a/src/qcodes/extensions/parameters/parameter_mixin_on_cache_change.py b/src/qcodes/extensions/parameters/parameter_mixin_on_cache_change.py index edd9ca9cbc88..fdeafd7c6701 100644 --- a/src/qcodes/extensions/parameters/parameter_mixin_on_cache_change.py +++ b/src/qcodes/extensions/parameters/parameter_mixin_on_cache_change.py @@ -143,7 +143,7 @@ def wrapped_cache_update( raw_value_new=raw_value_new, ) - parameter.cache._update_with = wrapped_cache_update # type: ignore[method-assign] + parameter.cache._update_with = wrapped_cache_update # type: ignore[method-assign] # ty: ignore[invalid-assignment] def _handle_on_cache_change( self, *, value_old: Any, value_new: Any, raw_value_old: Any, raw_value_new: Any diff --git a/src/qcodes/instrument/channel.py b/src/qcodes/instrument/channel.py index 1dbc25deabca..2c77792638e7 100644 --- a/src/qcodes/instrument/channel.py +++ b/src/qcodes/instrument/channel.py @@ -731,9 +731,10 @@ def __setitem__( # asserts added to work around https://github.com/python/mypy/issues/7858 if isinstance(index, int): assert isinstance(value, InstrumentModule) - self._channels[index] = value # type: ignore[assignment] - # mypy does not know that InstrumentModuleType is a TypeVar bound to - # InstrumentModule so complains here + # neither mypy nor ty knows that InstrumentModuleType is a TypeVar + # bound to InstrumentModule, so narrowing value with the isinstance + # above does not give them the element type of the list + self._channels[index] = value # type: ignore[assignment] # ty: ignore[invalid-assignment] else: assert not isinstance(value, InstrumentModule) self._channels[index] = value @@ -1213,7 +1214,9 @@ def __init__( chan_type: type[TAUTORELOADCHANNEL], chan_list: Sequence[TAUTORELOADCHANNEL] | None = None, snapshotable: bool = True, - multichan_paramclass: type = MultiChannelInstrumentParameter, + multichan_paramclass: type[MultiChannelInstrumentParameter] = ( + MultiChannelInstrumentParameter + ), **kwargs: Any, ) -> None: super().__init__( diff --git a/src/qcodes/instrument/instrument_base.py b/src/qcodes/instrument/instrument_base.py index 733bfa0ecb6c..b94e33a0bfa6 100644 --- a/src/qcodes/instrument/instrument_base.py +++ b/src/qcodes/instrument/instrument_base.py @@ -32,7 +32,13 @@ log = logging.getLogger(__name__) # Cannot convert to PEP 695: uses default= which requires PEP 696 (Python 3.13+). -TParameter = TypeVar("TParameter", bound="ParameterBase", default="Parameter") +# The default is `Parameter[Any, Any]` rather than a bare `Parameter`: when +# `add_parameter` is called without a `parameter_class` the returned parameter is +# bound to `self`, so spelling the default as `Parameter` (which expands to +# `Parameter[Any, InstrumentBase | None]`) would wrongly claim that the +# instrument is `InstrumentBase | None` and make the result unassignable to the +# `Parameter[SomeType, Self]` annotations that drivers use. +TParameter = TypeVar("TParameter", bound="ParameterBase", default="Parameter[Any, Any]") TSubmodule = TypeVar( "TSubmodule", bound="InstrumentModule | ChannelTuple", default="InstrumentModule" ) diff --git a/src/qcodes/instrument/ip_to_visa.py b/src/qcodes/instrument/ip_to_visa.py index 67d61acb421b..1183509a3f5e 100644 --- a/src/qcodes/instrument/ip_to_visa.py +++ b/src/qcodes/instrument/ip_to_visa.py @@ -24,7 +24,7 @@ # Such a driver is just a two-line class definition. -class IPToVisa(VisaInstrument, IPInstrument): # type: ignore[misc] +class IPToVisa(VisaInstrument, IPInstrument): # type: ignore[misc] # ty: ignore[invalid-method-override] """ Class to inject an VisaInstrument like behaviour in an IPInstrument that we'd like to use as a VISAInstrument with the diff --git a/src/qcodes/instrument_drivers/AlazarTech/ATS.py b/src/qcodes/instrument_drivers/AlazarTech/ATS.py index dd37c8244686..ea07debefd85 100644 --- a/src/qcodes/instrument_drivers/AlazarTech/ATS.py +++ b/src/qcodes/instrument_drivers/AlazarTech/ATS.py @@ -153,7 +153,7 @@ def __init__( self.buffer_list: list[Buffer] = [] - def get_idn(self) -> dict[str, str | int | None]: # type: ignore[override] + def get_idn(self) -> dict[str, str | int | None]: # type: ignore[override] # ty: ignore[invalid-method-override] # TODO return type is inconsistent with the super class. We should consider # if ints and floats are allowed as values in the dict """ diff --git a/src/qcodes/instrument_drivers/AlazarTech/dll_wrapper.py b/src/qcodes/instrument_drivers/AlazarTech/dll_wrapper.py index 425ab143f5e8..2432ae048452 100644 --- a/src/qcodes/instrument_drivers/AlazarTech/dll_wrapper.py +++ b/src/qcodes/instrument_drivers/AlazarTech/dll_wrapper.py @@ -64,17 +64,21 @@ def _check_error_code( if len(argrepr) > 100: argrepr = argrepr[:96] + "...]" + # ``errcheck`` is always handed a ctypes foreign function, which has a + # ``__name__``, but a plain ``Callable`` is not guaranteed to. + func_name = getattr(func, "__name__", repr(func)) + logger.error( f"Alazar API returned code {return_code} from function " - f"{func.__name__} with args {argrepr}" + f"{func_name} with args {argrepr}" ) if return_code not in ERROR_CODES: raise RuntimeError( - f"unknown error {return_code} from function {func.__name__} with args: {argrepr}" + f"unknown error {return_code} from function {func_name} with args: {argrepr}" ) raise RuntimeError( - f"error {return_code}: {ERROR_CODES[ReturnCode(return_code)]} from function {func.__name__} with args: {argrepr}" + f"error {return_code}: {ERROR_CODES[ReturnCode(return_code)]} from function {func_name} with args: {argrepr}" ) return arguments diff --git a/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py b/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py index b45b87504149..2929ce948c87 100644 --- a/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py +++ b/src/qcodes/instrument_drivers/Keithley/Keithley_7510.py @@ -367,7 +367,8 @@ def _get_data(self) -> DataArray7510: n_elements = len(elements) units = tuple(elements_units[element] for element in elements) - processed_data = dict.fromkeys(elements) + # every element is filled in by the loop below + processed_data: dict[str, npt.NDArray] = {} for i, (element, unit) in enumerate(zip(elements, units)): if unit == "str": processed_data[element] = np.array(all_data[i::n_elements]) @@ -384,12 +385,9 @@ def _get_data(self) -> DataArray7510: setpoint_units=((self.setpoints.unit,),) * n_elements, setpoint_names=((self.setpoints.label,),) * n_elements, ) - data._data = tuple( - tuple(processed_data[element]) # type: ignore[arg-type] - for element in elements - ) + data._data = tuple(tuple(processed_data[element]) for element in elements) for i in range(len(data.names)): - setattr(data, data.names[i], tuple(processed_data[data.names[i]])) # type: ignore[arg-type] + setattr(data, data.names[i], tuple(processed_data[data.names[i]])) return data def clear_buffer(self) -> None: diff --git a/src/qcodes/instrument_drivers/Keysight/Infiniium.py b/src/qcodes/instrument_drivers/Keysight/Infiniium.py index 8fdb6ee41adb..80ce22637bd1 100644 --- a/src/qcodes/instrument_drivers/Keysight/Infiniium.py +++ b/src/qcodes/instrument_drivers/Keysight/Infiniium.py @@ -3,7 +3,7 @@ from io import BytesIO from os.path import splitext from pathlib import Path -from typing import TYPE_CHECKING, Any, ClassVar, Literal +from typing import TYPE_CHECKING, Any, ClassVar, Literal, cast import numpy as np import numpy.typing as npt @@ -134,8 +134,7 @@ def setpoints(self) -> "Sequence[ParameterBase]": """ instrument = self.instrument if isinstance(instrument, KeysightInfiniiumChannel): - root_instrument: KeysightInfiniium - root_instrument = self.root_instrument # type: ignore[assignment] + root_instrument = cast("KeysightInfiniium", self.root_instrument) cache_setpoints = root_instrument.cache_setpoints() if not cache_setpoints: self.update_setpoints() @@ -201,7 +200,8 @@ def update_fft_setpoints(self) -> None: """ Update waveform parameters for an FFT. """ - instrument: KeysightInfiniiumFunction = self.instrument # type: ignore[assignment] + # only reached for a function parameter, see the caller in ``setpoints`` + instrument = cast("KeysightInfiniiumFunction", self.instrument) instrument.write(f":WAV:SOUR {self._channel}") preamble = instrument.ask(":WAV:PRE?").strip().split(",") self.update_setpoints(preamble) @@ -215,7 +215,7 @@ def get_raw(self) -> npt.NDArray: """ if self.instrument is None: raise RuntimeError("Cannot get data without instrument") - root_instr: KeysightInfiniium = self.root_instrument # type: ignore[assignment] + root_instr = cast("KeysightInfiniium", self.root_instrument) # Check if we can use cached trace parameters if not root_instr.cache_setpoints(): self.update_setpoints() @@ -234,13 +234,16 @@ def get_raw(self) -> npt.NDArray: root_instr.write(":WAV:DATA?") # Ignore first two bytes, which should be "#0" _ = root_instr.visa_handle.read_bytes(2) - data: npt.NDArray - data = root_instr.visa_handle.read_binary_values( # type: ignore[assignment] - "h", - container=np.ndarray, - header_fmt="empty", - expect_termination=True, - data_points=self._points, + # pyvisa types the return as a Sequence[float] regardless of ``container`` + data = cast( + "npt.NDArray", + root_instr.visa_handle.read_binary_values( + "h", + container=np.ndarray, + header_fmt="empty", + expect_termination=True, + data_points=self._points, + ), ) data = data.astype(np.float64) data = (data * self._yincrement) + self._yoffset @@ -1275,15 +1278,20 @@ def screenshot( ) try: with open(img_path, "wb") as f: - screen_bytes = self.visa_handle.query_binary_values( - f":DISPlay:DATA? {img_type.upper()[1:]}", # without . - # https://docs.python.org/3/library/struct.html#format-characters - datatype="B", # Capitcal B for unsigned byte - container=bytes, + # pyvisa types the return as a Sequence[float] regardless of + # ``container`` + screen_bytes = cast( + "bytes", + self.visa_handle.query_binary_values( + f":DISPlay:DATA? {img_type.upper()[1:]}", # without . + # https://docs.python.org/3/library/struct.html#format-characters + datatype="B", # Capitcal B for unsigned byte + container=bytes, + ), ) - f.write(screen_bytes) # type: ignore[arg-type] + f.write(screen_bytes) print(f"Screen image written to {img_path}") - return np.asarray(pil_open(BytesIO(screen_bytes))) # type: ignore[arg-type] + return np.asarray(pil_open(BytesIO(screen_bytes))) except Exception as e: self.log.error(f"Failed to save screenshot, Error occurred: \n{e}") return None diff --git a/src/qcodes/instrument_drivers/Keysight/keysight_34934a.py b/src/qcodes/instrument_drivers/Keysight/keysight_34934a.py index b206bf733474..a91d4f3d49b8 100644 --- a/src/qcodes/instrument_drivers/Keysight/keysight_34934a.py +++ b/src/qcodes/instrument_drivers/Keysight/keysight_34934a.py @@ -6,7 +6,7 @@ from .keysight_34980a_submodules import Keysight34980ASwitchMatrixSubModule if TYPE_CHECKING: - from collections.abc import Callable + from collections.abc import Callable, Collection from typing import Unpack from qcodes.instrument import ( @@ -105,7 +105,7 @@ def _set_relay_protection_mode(self, mode: str) -> None: self.write(f"SYSTem:MODule:ROW:PROTection {self.slot}, {mode}") def to_channel_list( - self, paths: list[tuple[int, int]], wiring_config: str | None = "" + self, paths: "Collection[tuple[int, int]]", wiring_config: str | None = "" ) -> str: """ Convert the (row, column) pair to a 4-digit channel number 'sxxx', where diff --git a/src/qcodes/instrument_drivers/Keysight/keysight_34980a.py b/src/qcodes/instrument_drivers/Keysight/keysight_34980a.py index 576b98882519..f93c08a149f2 100644 --- a/src/qcodes/instrument_drivers/Keysight/keysight_34980a.py +++ b/src/qcodes/instrument_drivers/Keysight/keysight_34980a.py @@ -70,7 +70,8 @@ def __init__( self._total_slot = 8 self._system_slots_info_dict: dict[int, dict[str, str]] | None = None - self.module = dict.fromkeys(self.system_slots_info.keys()) + # populated by scan_slots below, which puts an entry in for every slot + self.module: dict[int, Keysight34980ASwitchMatrixSubModule] = {} self.scan_slots() self.connect_message() @@ -132,7 +133,7 @@ def scan_slots(self) -> None: self.module[slot] = sub_module self.add_submodule(sub_module_name, sub_module) break - if self.module[slot] is None: + if slot not in self.module: sub_module_name = f"slot_{slot}_{model_string}_no_driver" sub_module_no_driver = Keysight34980ASwitchMatrixSubModule( self, sub_module_name, slot diff --git a/src/qcodes/instrument_drivers/Keysight/keysight_34980a_submodules.py b/src/qcodes/instrument_drivers/Keysight/keysight_34980a_submodules.py index 67c44060a613..ae9ad9b0d37a 100644 --- a/src/qcodes/instrument_drivers/Keysight/keysight_34980a_submodules.py +++ b/src/qcodes/instrument_drivers/Keysight/keysight_34980a_submodules.py @@ -3,6 +3,7 @@ from qcodes.instrument import InstrumentBaseKWArgs, InstrumentChannel if TYPE_CHECKING: + from collections.abc import Collection from typing import Unpack from .keysight_34980a import Keysight34980A @@ -43,7 +44,7 @@ def validate_value(self, row: int, column: int) -> None: raise NotImplementedError("Please subclass this") def to_channel_list( - self, paths: list[tuple[int, int]], wiring_config: str | None = None + self, paths: "Collection[tuple[int, int]]", wiring_config: str | None = None ) -> str: """ Convert the (row, column) pair to a 4-digit channel number 'sxxx', where @@ -125,7 +126,7 @@ def disconnect(self, row: int, column: int) -> None: channel = self.to_channel_list([(row, column)]) self.write(f"ROUT:OPEN {channel}") - def connect_paths(self, paths: list[tuple[int, int]]) -> None: + def connect_paths(self, paths: "Collection[tuple[int, int]]") -> None: """ To connect/close the specified channels. @@ -138,7 +139,7 @@ def connect_paths(self, paths: list[tuple[int, int]]) -> None: channel_list_str = self.to_channel_list(paths) self.write(f"ROUTe:CLOSe {channel_list_str}") - def disconnect_paths(self, paths: list[tuple[int, int]]) -> None: + def disconnect_paths(self, paths: "Collection[tuple[int, int]]") -> None: """ To disconnect/open the specified channels. @@ -151,7 +152,7 @@ def disconnect_paths(self, paths: list[tuple[int, int]]) -> None: channel_list_str = self.to_channel_list(paths) self.write(f"ROUTe:OPEN {channel_list_str}") - def are_closed(self, paths: list[tuple[int, int]]) -> list[bool]: + def are_closed(self, paths: "Collection[tuple[int, int]]") -> list[bool]: """ To check if a list of channels is closed/connected @@ -170,7 +171,7 @@ def are_closed(self, paths: list[tuple[int, int]]) -> list[bool]: messages = self.ask(f"ROUTe:CLOSe? {channel_list_str}") return [bool(int(message)) for message in messages.split(",")] - def are_open(self, paths: list[tuple[int, int]]) -> list[bool]: + def are_open(self, paths: "Collection[tuple[int, int]]") -> list[bool]: """ To check if a list of channels is open/disconnected diff --git a/src/qcodes/instrument_drivers/Keysight/keysight_b220x.py b/src/qcodes/instrument_drivers/Keysight/keysight_b220x.py index d6c4f9ee2c96..c8e01b519620 100644 --- a/src/qcodes/instrument_drivers/Keysight/keysight_b220x.py +++ b/src/qcodes/instrument_drivers/Keysight/keysight_b220x.py @@ -7,7 +7,7 @@ from qcodes.validators import Enum, Ints, Lists, MultiType if TYPE_CHECKING: - from collections.abc import Callable, Sequence + from collections.abc import Callable, Iterable from typing import Concatenate, Unpack from qcodes.parameters import Parameter @@ -251,12 +251,12 @@ def connect(self, input_ch: int, output_ch: int) -> None: self.write(f":CLOS (@{self._card:01d}{input_ch:02d}{output_ch:02d})") @post_execution_status_poll - def connect_paths(self, paths: "Sequence[tuple[int, int]]") -> None: + def connect_paths(self, paths: "Iterable[tuple[int, int]]") -> None: channel_list_str = self.to_channel_list(paths) self.write(f":CLOS {channel_list_str}") @post_execution_status_poll - def disconnect_paths(self, paths: "Sequence[tuple[int, int]]") -> None: + def disconnect_paths(self, paths: "Iterable[tuple[int, int]]") -> None: channel_list_str = self.to_channel_list(paths) self.write(f":OPEN {channel_list_str}") @@ -424,7 +424,7 @@ def parse_channel_list(channel_list: str) -> set[tuple[int, int]]: for match in re.finditer(pattern, channel_list) } - def to_channel_list(self, paths: "Sequence[tuple[int, int]]") -> str: + def to_channel_list(self, paths: "Iterable[tuple[int, int]]") -> str: chan = [f"{self._card:01d}{i:02d}{o:02d}" for i, o in paths] channel_list = f"(@{','.join(chan)})" return channel_list diff --git a/src/qcodes/instrument_drivers/Keysight/keysight_e4980a.py b/src/qcodes/instrument_drivers/Keysight/keysight_e4980a.py index 5b339d2e90bf..0c0e0f2107bd 100644 --- a/src/qcodes/instrument_drivers/Keysight/keysight_e4980a.py +++ b/src/qcodes/instrument_drivers/Keysight/keysight_e4980a.py @@ -50,6 +50,16 @@ class KeysightE4980AMeasurementPair(MultiParameter): value: tuple[float, float] = (0.0, 0.0) + if TYPE_CHECKING: + # The two measured values are exposed as attributes named after the + # ``names`` of the measurement function, so which attributes exist is + # only known at runtime. Declaring this for type checkers lets the + # documented usage, such as ``measurement.capacitance``, be written in + # typed code. It is not defined at runtime, so accessing an attribute + # that the current measurement function does not provide still raises + # the usual ``AttributeError``. + def __getattr__(self, name: str) -> float: ... + def __init__( self, name: str, names: "Sequence[str]", units: "Sequence[str]", **kwargs: Any ): diff --git a/src/qcodes/instrument_drivers/Lakeshore/lakeshore_base.py b/src/qcodes/instrument_drivers/Lakeshore/lakeshore_base.py index 1b8b9ff9d8e1..2ad9601ae4f8 100644 --- a/src/qcodes/instrument_drivers/Lakeshore/lakeshore_base.py +++ b/src/qcodes/instrument_drivers/Lakeshore/lakeshore_base.py @@ -690,9 +690,9 @@ class LakeshoreBase(VisaInstrument, Generic[ChanType_co]): # Define this in the model-specific class in case you want to use a # different class for sensor channels # type error. It's not clear to me why assigning a value that matches the - # default of the TypeVar is an error but both mypy and pyright - # flags it here. - CHANNEL_CLASS: type[ChanType_co] = LakeshoreBaseSensorChannel # type: ignore[assignment] + # default of the TypeVar is an error but mypy, pyright and ty all + # flag it here. + CHANNEL_CLASS: type[ChanType_co] = LakeshoreBaseSensorChannel # type: ignore[assignment] # ty: ignore[invalid-assignment] # This dict has channel name in the driver as keys, and channel "name" that # is used in instrument commands as values. For example, if channel called diff --git a/src/qcodes/instrument_drivers/QuantumDesign/DynaCoolPPMS/private/server.py b/src/qcodes/instrument_drivers/QuantumDesign/DynaCoolPPMS/private/server.py index 0494c21f161b..d08f587de900 100644 --- a/src/qcodes/instrument_drivers/QuantumDesign/DynaCoolPPMS/private/server.py +++ b/src/qcodes/instrument_drivers/QuantumDesign/DynaCoolPPMS/private/server.py @@ -31,7 +31,7 @@ def run_server() -> None: # Dictionary to keep track of sockets and addresses. # Keys are sockets and values are addresses. # Add server socket to the dictionary first. - socket_dict = {server_socket: (ADDRESS, PORT)} + socket_dict: dict[socket.socket, tuple[str, int]] = {server_socket: (ADDRESS, PORT)} print(f"Server started on port {PORT}.") print("Press ESC to exit.") diff --git a/src/qcodes/instrument_drivers/tektronix/AWG5014.py b/src/qcodes/instrument_drivers/tektronix/AWG5014.py index c0084abea479..604a7ade9f64 100644 --- a/src/qcodes/instrument_drivers/tektronix/AWG5014.py +++ b/src/qcodes/instrument_drivers/tektronix/AWG5014.py @@ -605,7 +605,7 @@ def __init__( r"^ch(?P[1-4])_(?:(?Pm[12])_)?(?P.+)$" ) - def __getattr__(self, name: str) -> Any: + def __getattr__(self, key: str) -> Any: """ Provide backwards-compatible access to the old flat parameter names like ``ch1_amp``, ``ch1_m1_high``, etc. @@ -613,7 +613,7 @@ def __getattr__(self, name: str) -> Any: These now live on channel / marker submodules but are still reachable via the old names with a deprecation warning. """ - m = self._LEGACY_CHANNEL_RE.match(name) + m = self._LEGACY_CHANNEL_RE.match(key) if m is not None: ch_num = int(m.group("ch")) marker = m.group("marker") @@ -629,7 +629,7 @@ def __getattr__(self, name: str) -> Any: if hasattr(mrk, new_param): new_name = f"ch{ch_num}.{marker}.{new_param}" warnings.warn( - f"Accessing '{name}' is deprecated. " + f"Accessing '{key}' is deprecated. " f"Use '{new_name}' instead.", category=QCoDeSDeprecationWarning, stacklevel=2, @@ -638,12 +638,12 @@ def __getattr__(self, name: str) -> Any: elif hasattr(ch, param): new_name = f"ch{ch_num}.{param}" warnings.warn( - f"Accessing '{name}' is deprecated. Use '{new_name}' instead.", + f"Accessing '{key}' is deprecated. Use '{new_name}' instead.", category=QCoDeSDeprecationWarning, stacklevel=2, ) return getattr(ch, param) - return super().__getattr__(name) + return super().__getattr__(key) # Convenience parser def newlinestripper(self, string: str) -> str: diff --git a/src/qcodes/instrument_drivers/tektronix/AWGFileParser.py b/src/qcodes/instrument_drivers/tektronix/AWGFileParser.py index 9c7ed36b4c0e..da64e7919eb6 100644 --- a/src/qcodes/instrument_drivers/tektronix/AWGFileParser.py +++ b/src/qcodes/instrument_drivers/tektronix/AWGFileParser.py @@ -295,14 +295,18 @@ "WAIT_VALUE": {1: "First", 2: "Last"}, } +# The tuple returned by ``_parser3``, and therefore by ``parse_awg_file``. It +# deliberately matches the call signature of +# ``TektronixAWG5014.make_send_and_load_awg_file``, so that the output of the +# parser can be passed straight back in. _parser3_output = tuple[ - list[list[dict[Any, Any]]], - list[list[dict[Any, Any]]], - list[list[dict[Any, Any]]], - list[str | int], - list[str | int], - list[str | int], - list[str | int], + list[list[npt.NDArray]], + list[list[npt.NDArray]], + list[list[npt.NDArray]], + list[int], + list[int], + list[int], + list[int], list[int], ] diff --git a/src/qcodes/parameters/array_parameter.py b/src/qcodes/parameters/array_parameter.py index d3565f83f023..5fb1df55f58d 100644 --- a/src/qcodes/parameters/array_parameter.py +++ b/src/qcodes/parameters/array_parameter.py @@ -142,7 +142,8 @@ def __init__( kwargs.setdefault("snapshot_value", False) super().__init__( name, - **kwargs, + # see the note on ParameterBaseKWArgs + **kwargs, # ty: ignore[invalid-argument-type] ) if self.settable: diff --git a/src/qcodes/parameters/combined_parameter.py b/src/qcodes/parameters/combined_parameter.py index 4a6adab034b0..9c6ead6b2928 100644 --- a/src/qcodes/parameters/combined_parameter.py +++ b/src/qcodes/parameters/combined_parameter.py @@ -3,6 +3,7 @@ import collections import logging from copy import copy +from dataclasses import dataclass from typing import TYPE_CHECKING, Any import numpy as np @@ -21,6 +22,30 @@ _LOG = logging.getLogger(__name__) +@dataclass +class _CombinedParameterInfo: + """ + The subset of the ``Parameter`` api that :class:`CombinedParameter` fakes. + + This exists because :class:`CombinedParameter` does not inherit from + :class:`.Parameter` or :class:`.ParameterBase`, yet is expected to carry the + identifying metadata of one so that it can be snapshotted like one. + """ + + name: str + full_name: str + label: str | None + unit: str | None + + def __call__(self) -> None: + """ + Do nothing. + + This used to be a lambda, so external code may be calling it. Calling it + has always returned ``None``. + """ + + def combine( *parameters: Parameter, name: str, @@ -77,10 +102,6 @@ def __init__( aggregator: Callable[..., Any] | None = None, ) -> None: super().__init__() - # TODO(giulioungaretti)temporary hack - # starthack - # this is a dummy parameter - # that mimicks the api that a normal parameter has if not name.isidentifier(): raise ValueError( f"Parameter name must be a valid identifier " @@ -89,13 +110,6 @@ def __init__( f"must not contain spaces or special characters" ) - self.parameter = lambda: None - # mypy will complain that a callable does not have these attributes - # but you can still create them here. - self.parameter.full_name = name # type: ignore[attr-defined] - self.parameter.name = name # type: ignore[attr-defined] - self.parameter.label = label # type: ignore[attr-defined] - if units is not None: _LOG.warning( f"`units` is deprecated for the " @@ -103,9 +117,19 @@ def __init__( ) if unit is None: unit = units - self.parameter.unit = unit # type: ignore[attr-defined] - self.setpoints: list[Any] = [] + + # TODO(giulioungaretti)temporary hack + # starthack + # this is a dummy parameter + # that mimicks the api that a normal parameter has. + # CombinedParameter does not inherit from Parameter or ParameterBase, + # so it has to fake the parts of their api that it is expected to + # provide. + self.parameter = _CombinedParameterInfo( + name=name, full_name=name, label=label, unit=unit + ) # endhack + self.setpoints: list[Any] = [] self.parameters = parameters self.sets = [parameter.set for parameter in self.parameters] self.dimensionality = len(self.sets) @@ -215,9 +239,9 @@ def snapshot_base( meta_data: dict[str, Any] = collections.OrderedDict() meta_data["__class__"] = full_class(self) param = self.parameter - meta_data["unit"] = param.unit # type: ignore[attr-defined] - meta_data["label"] = param.label # type: ignore[attr-defined] - meta_data["full_name"] = param.full_name # type: ignore[attr-defined] + meta_data["unit"] = param.unit + meta_data["label"] = param.label + meta_data["full_name"] = param.full_name meta_data["aggregator"] = repr(getattr(self, "f", None)) update = normalize_snapshot_update(update) for parameter in self.parameters: diff --git a/src/qcodes/parameters/command.py b/src/qcodes/parameters/command.py index e2da0b14d014..0561dab1dabe 100644 --- a/src/qcodes/parameters/command.py +++ b/src/qcodes/parameters/command.py @@ -124,7 +124,10 @@ def __init__( elif is_function(cmd, arg_count): assert cmd is not None self._cmd = cmd - exec_mapping = { + cmd_exec_mapping: dict[ + tuple[bool | Literal["multi"], bool], + Callable[..., Output | ParsedOutput], + ] = { # (parse_input, parse_output) (False, False): cmd, (False, True): self.call_cmd_parsed_out, (True, False): self.call_cmd_parsed_in, @@ -132,7 +135,7 @@ def __init__( ("multi", False): self.call_cmd_parsed_in2, ("multi", True): self.call_cmd_parsed_in2_out, } - self.exec_function = exec_mapping[(parse_input, parse_output)] + self.exec_function = cmd_exec_mapping[(parse_input, parse_output)] elif cmd is None: if no_cmd_function is not None: diff --git a/src/qcodes/parameters/delegate_parameter.py b/src/qcodes/parameters/delegate_parameter.py index b948b8c568d4..b63736f19288 100644 --- a/src/qcodes/parameters/delegate_parameter.py +++ b/src/qcodes/parameters/delegate_parameter.py @@ -210,7 +210,8 @@ def __init__( initial_cache_value = kwargs.pop("initial_cache_value", None) self.source = source - super().__init__(name, **kwargs) + # see the note on ParameterBaseKWArgs + super().__init__(name, **kwargs) # ty: ignore[invalid-argument-type] self.label = kwargs.get("label", None) self.unit = kwargs.get("unit", None) diff --git a/src/qcodes/parameters/multi_parameter.py b/src/qcodes/parameters/multi_parameter.py index 0f230b693343..80385ff6304b 100644 --- a/src/qcodes/parameters/multi_parameter.py +++ b/src/qcodes/parameters/multi_parameter.py @@ -153,7 +153,8 @@ def __init__( kwargs.setdefault("snapshot_value", False) super().__init__( name, - **kwargs, + # see the note on ParameterBaseKWArgs + **kwargs, # ty: ignore[invalid-argument-type] ) self._meta_attrs.extend( diff --git a/src/qcodes/parameters/parameter.py b/src/qcodes/parameters/parameter.py index 47626681be8c..74feae11f0fe 100644 --- a/src/qcodes/parameters/parameter.py +++ b/src/qcodes/parameters/parameter.py @@ -10,6 +10,8 @@ from typing_extensions import TypedDict +from qcodes.utils import qcodes_abstractmethod + from .command import Command from .parameter_base import ( InstrumentTypeVar_co, @@ -286,6 +288,12 @@ class Parameter( """ + _get_raw_impl: Callable[[], ParamRawDataType] | None = None + """Implementation of ``get_raw`` generated from ``get_cmd``, if any.""" + + _set_raw_impl: Callable[[ParamRawDataType], None] | None = None + """Implementation of ``set_raw`` generated from ``set_cmd``, if any.""" + def __init__( self, name: str, @@ -354,7 +362,8 @@ def _set_manual_parameter( super().__init__( name=name, - **kwargs, + # see the note on ParameterBaseKWArgs + **kwargs, # ty: ignore[invalid-argument-type] ) no_instrument_get = not self._implements_get_raw and ( @@ -382,9 +391,9 @@ def _set_manual_parameter( " get_raw is an error." ) elif not self._implements_get_raw and get_cmd is not False: + get_raw_impl: Callable[[], ParamRawDataType] if get_cmd is None: - # ignore typeerror since mypy does not allow setting a method dynamically - self.get_raw = MethodType(_get_manual_parameter, self) # type: ignore[method-assign] + get_raw_impl = MethodType(_get_manual_parameter, self) else: if isinstance(get_cmd, str) and instrument is None: raise TypeError( @@ -396,14 +405,14 @@ def _set_manual_parameter( exec_str_ask = getattr(instrument, "ask", None) if instrument else None # TODO get_raw should also be a method here. This should probably be done by wrapping # it with MethodType like above - # ignore typeerror since mypy does not allow setting a method dynamically - self.get_raw = Command( # type: ignore[method-assign] + get_raw_impl = Command( arg_count=0, cmd=get_cmd, exec_str=exec_str_ask, ) + self._get_raw_impl = get_raw_impl self._gettable = True - self.get = self._wrap_get(self.get_raw) + self.get = self._wrap_get(get_raw_impl) if self._implements_set_raw and set_cmd not in (None, False): raise TypeError( @@ -412,9 +421,9 @@ def _set_manual_parameter( " set_raw is an error." ) elif not self._implements_set_raw and set_cmd is not False: + set_raw_impl: Callable[[ParamRawDataType], None] if set_cmd is None: - # ignore typeerror since mypy does not allow setting a method dynamically - self.set_raw = MethodType(_set_manual_parameter, self) # type: ignore[method-assign] + set_raw_impl = MethodType(_set_manual_parameter, self) else: if isinstance(set_cmd, str) and instrument is None: raise TypeError( @@ -426,14 +435,14 @@ def _set_manual_parameter( exec_str_write = ( getattr(instrument, "write", None) if instrument else None ) - # TODO get_raw should also be a method here. This should probably be done by wrapping + # TODO set_raw should also be a method here. This should probably be done by wrapping # it with MethodType like above - # ignore typeerror since mypy does not allow setting a method dynamically - self.set_raw = Command( # type: ignore[assignment] + set_raw_impl = Command( arg_count=1, cmd=set_cmd, exec_str=exec_str_write ) + self._set_raw_impl = set_raw_impl self._settable = True - self.set = self._wrap_set(self.set_raw) + self.set = self._wrap_set(set_raw_impl) self._meta_attrs.extend(["label", "unit", "vals"]) @@ -459,6 +468,31 @@ def _set_manual_parameter( self._docstring = docstring self.__doc__ = self._build__doc__() + @qcodes_abstractmethod + def get_raw(self) -> ParamRawDataType: + """ + Call the ``get_raw`` implementation generated from ``get_cmd``. + + This method stays marked as abstract so that + :attr:`~ParameterBase._implements_get_raw` keeps reporting ``False`` + for :class:`Parameter` itself: a subclass is still expected to either + override ``get_raw`` or supply a ``get_cmd``. + """ + if self._get_raw_impl is None: + raise NotImplementedError + return self._get_raw_impl() + + @qcodes_abstractmethod + def set_raw(self, value: ParamRawDataType) -> None: + """ + Call the ``set_raw`` implementation generated from ``set_cmd``. + + See :meth:`get_raw` for why this method stays marked as abstract. + """ + if self._set_raw_impl is None: + raise NotImplementedError + self._set_raw_impl(value) + def _build__doc__(self) -> str: if len(self.validators) == 0: validator_docstrings = ["* `vals` None"] @@ -521,7 +555,7 @@ def increment(self, value: ParameterDataTypeVar) -> None: """ # this method only works with parameters that support addition # however we don't currently enforce that via typing - self.set(self.get() + value) # type: ignore[operator] + self.set(self.get() + value) # type: ignore[operator] # ty: ignore[unsupported-operator] def sweep( self, diff --git a/src/qcodes/parameters/parameter_base.py b/src/qcodes/parameters/parameter_base.py index 70216cd0ef76..210a8c1cfc3b 100644 --- a/src/qcodes/parameters/parameter_base.py +++ b/src/qcodes/parameters/parameter_base.py @@ -147,6 +147,16 @@ class ParameterBaseKWArgs( ``**kwargs: Unpack[ParameterBaseKWArgs]`` as input and forward this to the super class to ensure that it can accept all the arguments defined here. + + Note that forwarding the kwargs on requires a + ``ty: ignore[invalid-argument-type]``. When a generic TypedDict declares a + PEP 696 default for a type parameter, 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``. Expanding + with ``**`` is one of those, and reports the rather misleading + ``must be a mapping type``. ``InstrumentTypeVar_co`` defaults to + ``InstrumentBase | None`` and so triggers this; ``ParameterDataTypeVar`` + defaults to ``Any``, which happens to be the one default ty accepts. """ instrument: NotRequired[InstrumentTypeVar_co] @@ -257,6 +267,62 @@ class ParameterBaseKWArgs( """ +# The four helpers below convert between a parameter value and its raw +# counterpart. They are deliberately duck typed: they assume that the caller +# does not set ``scale``/``offset`` unless the data type is numeric, and either +# check for an iterable up front or fall back on catching ``TypeError``. +# Taking and returning ``Any`` keeps that boundary explicit, and keeps the +# generic ``ParameterDataTypeVar`` out of the arithmetic. + + +def _scale_raw_value(raw_value: Any, scale: float | Iterable[float]) -> Any: + """Multiply a value by ``scale`` on the way to the instrument.""" + if isinstance(scale, collections.abc.Iterable): + # Scale contains multiple elements, one for each value + return tuple(val * sub_scale for val, sub_scale in zip(raw_value, scale)) + # Use single scale for all values + return raw_value * scale + + +def _offset_raw_value(raw_value: Any, offset: float | Iterable[float]) -> Any: + """Add ``offset`` to a value on the way to the instrument.""" + if isinstance(offset, collections.abc.Iterable): + # offset contains multiple elements, one for each value + return tuple(val + sub_offset for val, sub_offset in zip(raw_value, offset)) + # Use single offset for all values + return raw_value + offset + + +def _unoffset_value(value: Any, offset: float | Iterable[float]) -> Any: + """Subtract ``offset`` from a value coming back from the instrument.""" + try: + return value - offset + except TypeError: + if isinstance(offset, collections.abc.Iterable): + # offset contains multiple elements, one for each value + return tuple(val - sub_offset for val, sub_offset in zip(value, offset)) + elif isinstance(value, collections.abc.Iterable): + # Use single offset for all values + return tuple(val - offset for val in value) + else: + raise + + +def _unscale_value(value: Any, scale: float | Iterable[float]) -> Any: + """Divide a value coming back from the instrument by ``scale``.""" + try: + return value / scale + except TypeError: + if isinstance(scale, collections.abc.Iterable): + # Scale contains multiple elements, one for each value + return tuple(val / sub_scale for val, sub_scale in zip(value, scale)) + elif isinstance(value, collections.abc.Iterable): + # Use single scale for all values + return tuple(val / scale for val in value) + else: + raise + + class ParameterBase( MetadatableWithName, Generic[ParameterDataTypeVar, InstrumentTypeVar_co] ): @@ -363,9 +429,10 @@ def __init__( self, name: str, *, - # mypy seems to be confused here. The bound and default for InstrumentTypeVar_co - # contains None but mypy will not allow None as a default as of v 1.19.0 - instrument: InstrumentTypeVar_co = None, # type: ignore[assignment] + # The bound and default for InstrumentTypeVar_co contain None, but + # neither mypy (as of v1.19.0) nor ty accept None as the default for a + # parameter annotated with the type variable itself. + instrument: InstrumentTypeVar_co = None, # type: ignore[assignment] # ty: ignore[invalid-parameter-default] snapshot_get: bool = True, metadata: Mapping[Any, Any] | None = None, step: float | None = None, @@ -814,25 +881,11 @@ def _from_value_to_raw_value(self, value: ParameterDataTypeVar) -> ParamRawDataT # transverse transformation in reverse order as compared to # getter: apply scale first if self.scale is not None: - if isinstance(self.scale, collections.abc.Iterable): - # Scale contains multiple elements, one for each value - raw_value = tuple( - val * scale for val, scale in zip(raw_value, self.scale) - ) - else: - # Use single scale for all values - raw_value = raw_value * self.scale + raw_value = _scale_raw_value(raw_value, self.scale) # apply offset next if self.offset is not None: - if isinstance(self.offset, collections.abc.Iterable): - # offset contains multiple elements, one for each value - raw_value = tuple( - val + offset for val, offset in zip(raw_value, self.offset) - ) - else: - # Use single offset for all values - raw_value = raw_value + self.offset + raw_value = _offset_raw_value(raw_value, self.offset) # parser last if self.set_parser is not None: @@ -843,6 +896,9 @@ def _from_value_to_raw_value(self, value: ParameterDataTypeVar) -> ParamRawDataT def _from_raw_value_to_value( self, raw_value: ParamRawDataType ) -> ParameterDataTypeVar: + # ``value`` keeps the parameter's data type as its declared type; the + # offset and scale transformations below rely on duck typing and are + # therefore delegated to the helpers at the top of this module. value: ParameterDataTypeVar if self.get_parser is not None: @@ -850,42 +906,13 @@ def _from_raw_value_to_value( else: value = raw_value - # the code below is not very type safe but relies on duck typing / try except - # and assumes the user does not set scale/offset unless the datatype is numeric - # this should probably be rewritten but for now we ignore type errors # apply offset first (native scale) - if self.offset is not None and value is not None: - # offset values - try: - value = value - self.offset # type: ignore[operator,assignment] - except TypeError: - if isinstance(self.offset, collections.abc.Iterable): - # offset contains multiple elements, one for each value - value = tuple( # type: ignore[assignment] - val - offset - for val, offset in zip(value, self.offset) # type: ignore[call-overload] - ) - elif isinstance(value, collections.abc.Iterable): - # Use single offset for all values - value = tuple(val - self.offset for val in value) # type: ignore[assignment] - else: - raise + value = _unoffset_value(value, self.offset) # scale second if self.scale is not None and value is not None: - # Scale values - try: - value = value / self.scale # type: ignore[assignment,operator] - except TypeError: - if isinstance(self.scale, collections.abc.Iterable): - # Scale contains multiple elements, one for each value - value = tuple(val / scale for val, scale in zip(value, self.scale)) # type: ignore[call-overload,assignment] - elif isinstance(value, collections.abc.Iterable): - # Use single scale for all values - value = tuple(val / self.scale for val in value) # type: ignore[assignment] - else: - raise + value = _unscale_value(value, self.scale) if self.inverse_val_mapping is not None: if value in self.inverse_val_mapping: @@ -896,7 +923,7 @@ def _from_raw_value_to_value( except (ValueError, KeyError): raise KeyError(f"'{value}' not in val_mapping") - return value # pyright: ignore[reportReturnType] + return value def _wrap_get( self, get_function: Callable[..., ParamRawDataType] @@ -946,14 +973,16 @@ def set_wrapper(value: ParameterDataTypeVar, **kwargs: Any) -> None: # In some cases intermediate sweep values must be used. # Unless `self.step` is defined, get_sweep_values will return # a list containing only `value`. - steps = self.get_ramp_values(value, step=self.step) # type: ignore[arg-type] + # The steps are deliberately untyped: ``get_ramp_values`` works + # in terms of numbers rather than the parameter's data type. + steps: Sequence[Any] = self.get_ramp_values(value, step=self.step) # type: ignore[arg-type] # ty: ignore[invalid-argument-type] for val_step in steps: # even if the final value is valid we may be generating # steps that are not so validate them too - self.validate(val_step) # type: ignore[arg-type] + self.validate(val_step) - raw_val_step = self._from_value_to_raw_value(val_step) # type: ignore[arg-type] + raw_val_step = self._from_value_to_raw_value(val_step) # Check if delay between set operations is required t_elapsed = time.perf_counter() - self._t_last_set @@ -976,9 +1005,9 @@ def set_wrapper(value: ParameterDataTypeVar, **kwargs: Any) -> None: # Sleep until total time is larger than self.post_delay time.sleep(self.post_delay - t_elapsed) - self.cache._update_with(value=val_step, raw_value=raw_val_step) # type: ignore[arg-type] + self.cache._update_with(value=val_step, raw_value=raw_val_step) - self._call_on_set_callback(val_step) # type: ignore[arg-type] + self._call_on_set_callback(val_step) except Exception as e: e.args = (*e.args, f"setting {self} to {value}") diff --git a/src/qcodes/parameters/parameter_with_setpoints.py b/src/qcodes/parameters/parameter_with_setpoints.py index 9653be69281f..812b36f5a6ce 100644 --- a/src/qcodes/parameters/parameter_with_setpoints.py +++ b/src/qcodes/parameters/parameter_with_setpoints.py @@ -74,7 +74,8 @@ def __init__( super().__init__( name=name, - **kwargs, + # see the note on ParameterBaseKWArgs + **kwargs, # ty: ignore[invalid-argument-type] ) if setpoints is None: self.setpoints = [] diff --git a/src/qcodes/plotting/matplotlib_helpers.py b/src/qcodes/plotting/matplotlib_helpers.py index c86ade62ad69..fb6ea81cfba4 100644 --- a/src/qcodes/plotting/matplotlib_helpers.py +++ b/src/qcodes/plotting/matplotlib_helpers.py @@ -49,7 +49,7 @@ def _set_colorbar_extend( "min": slice(1, None), "max": slice(0, -1), } - colorbar._inside = _slice_dict[extend] # type: ignore[attr-defined] + colorbar._inside = _slice_dict[extend] # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] def apply_color_scale_limits( diff --git a/src/qcodes/utils/abstractmethod.py b/src/qcodes/utils/abstractmethod.py index 26d0ac3611f1..ed6db0ecbf4e 100644 --- a/src/qcodes/utils/abstractmethod.py +++ b/src/qcodes/utils/abstractmethod.py @@ -16,7 +16,7 @@ def qcodes_abstractmethod[**input, output]( instantiated and we will use this property to detect if the method is abstract and should be overwritten. """ - funcobj.__qcodes_is_abstract_method__ = True # type: ignore[attr-defined] + funcobj.__qcodes_is_abstract_method__ = True # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] return funcobj diff --git a/src/qcodes/utils/types.py b/src/qcodes/utils/types.py index 01fe50e3a2eb..dc82f1692674 100644 --- a/src/qcodes/utils/types.py +++ b/src/qcodes/utils/types.py @@ -44,7 +44,7 @@ Default integer types. The size may be platform dependent. """ -numpy_ints: tuple[type, ...] = ( +numpy_ints: tuple[type[np.integer], ...] = ( numpy_concrete_ints + numpy_c_ints + numpy_non_concrete_ints_instantiable ) """ @@ -61,7 +61,7 @@ Floating point types that matches C types. """ -numpy_floats: tuple[type, ...] = numpy_concrete_floats + numpy_c_floats +numpy_floats: tuple[type[np.floating], ...] = numpy_concrete_floats + numpy_c_floats """ All numpy float types """ diff --git a/src/qcodes/validators/validators.py b/src/qcodes/validators/validators.py index bcda83550e19..24d5f38be7be 100644 --- a/src/qcodes/validators/validators.py +++ b/src/qcodes/validators/validators.py @@ -987,12 +987,12 @@ def shape_unevaluated(self) -> shape_tuple_type: def shape(self) -> tuple[int, ...] | None: if self._shape is None: return None - shape_array = [] + shape_array: list[int] = [] for s in self._shape: - if callable(s): - shape_array.append(s()) - else: + if isinstance(s, int): shape_array.append(s) + else: + shape_array.append(s()) shape = tuple(shape_array) return shape diff --git a/suppression-codes-mypy-and-ty.md b/suppression-codes-mypy-and-ty.md new file mode 100644 index 000000000000..8479b7d269c7 --- /dev/null +++ b/suppression-codes-mypy-and-ty.md @@ -0,0 +1,211 @@ +# Combining mypy and ty suppression codes + +## Summary + +The [ty suppression docs](https://docs.astral.sh/ty/suppression/) document putting +a ty rule into a mypy `type: ignore` comment by prefixing it with `ty:`: + +```python +sum_three_numbers("one", 5, 2) # type: ignore[arg-type, ty:invalid-argument-type] +``` + +ty honours this. **mypy does not ignore the `ty:` prefixed code**, and reports it +as an unused suppression when `warn_unused_ignores` is enabled, which qcodes +enables in `pyproject.toml`. So the combined form cannot be used here. + +qcodes therefore uses two comments on the same line: + +```python +f("one") # type: ignore[arg-type] # ty: ignore[invalid-argument-type] +``` + +That is the only form of the three below that all three checkers accept. + +## Results + +| form | ty 0.0.74 | mypy 2.3.1 with `warn_unused_ignores` | mypy 2.3.1 without it | pyright 1.1.411 | +| --- | --- | --- | --- | --- | +| `# type: ignore[arg-type, ty:invalid-argument-type]` | suppressed | `Unused "type: ignore[ty:invalid-argument-type]" comment` | clean | suppressed | +| `# type: ignore[arg-type]` + `# ty: ignore[invalid-argument-type]` | suppressed | clean | clean | suppressed | +| `# type: ignore[ty:invalid-argument-type]` | suppressed | unused, and `arg-type` not covered | `arg-type` not covered | suppressed | + +Note that the `arg-type` half of the combined form *is* honoured by mypy. It is +only the `ty:` prefixed code that mypy does not recognise, and therefore reports +as unused. + +## How pyright fits in + +pyright has its own suppression comment and also honours mypy's, which is why it +accepts all three forms above. + +| comment | pyright | +| --- | --- | +| `# type: ignore` | suppressed | +| `# type: ignore[arg-type]` | suppressed | +| `# type: ignore[arg-type, ty:invalid-argument-type]` | suppressed | +| `# pyright: ignore` | suppressed | +| `# pyright: ignore[reportArgumentType]` | suppressed | +| `# pyright: ignore[reportGeneralTypeIssues]` | **not** suppressed, wrong rule | +| `# ty: ignore[invalid-argument-type]` | **not** suppressed | + +Two things follow from this. + +**`# type: ignore` is a blanket suppression for pyright.** pyright does not parse +the codes in it, so `# type: ignore[arg-type]` silences *every* pyright rule on +that line, not just the argument type one. A consequence that came up repeatedly +during the ty migration: removing a mypy suppression can surface a pyright error +on the same line that was never visible before. `# pyright: ignore[rule]` is the +precise form, and unlike `# type: ignore` it only suppresses the rules listed. + +**A ty only suppression does not silence pyright.** `# ty: ignore[...]` is just a +comment as far as pyright is concerned. That is what makes the two comment form +safe: the mypy half keeps pyright quiet as a side effect, and the ty half is +inert for both of the others. + +## Unused suppression detection + +The three checkers differ in whether they tell you a suppression has gone stale. + +| checker | setting | default | reports unused | +| --- | --- | --- | --- | +| mypy | `warn_unused_ignores` | off | enabled in `pyproject.toml` | +| ty | `unused-ignore-comment` | on | yes, for `ty: ignore` directives | +| pyright | `reportUnnecessaryTypeIgnoreComment` | off | not enabled, see below | + +With the pyright setting enabled it reports all of these: + +```python +def g(a: int) -> None: ... + + +g(1) # type: ignore +g(1) # pyright: ignore +g(1) # pyright: ignore[reportArgumentType] +``` + +``` +Unnecessary "# type: ignore" comment +Unnecessary "# type: ignore" comment +Unnecessary "# pyright: ignore" rule: "reportArgumentType" +``` + +**We cannot enable it while we also run mypy.** Because pyright treats +`# type: ignore` as a blanket suppression of *its own* rules, it calls the +comment unnecessary whenever pyright itself has nothing to report on the line, +with no knowledge of whether mypy needed it. Every mypy only suppression in the +code base would be reported as unnecessary. For example: + +```python +from typing import Any + + +class A: + def m(self) -> None: ... + + +def make(a: A, replacement: Any) -> None: + # mypy reports method-assign here, pyright has no equivalent check + a.m = replacement # type: ignore[method-assign] +``` + +mypy needs that suppression: removing it gives +`error: Cannot assign to a method [method-assign]`. pyright with +`reportUnnecessaryTypeIgnoreComment` enabled reports the very same line as +`Unnecessary "# type: ignore" comment`. + +So mypy's `warn_unused_ignores` and ty's `unused-ignore-comment` are the two +stale suppression checks we can actually rely on. + +## Test case + +```python +def f(a: int) -> None: ... + + +# 1. combined form from the ty docs +f("one") # type: ignore[arg-type, ty:invalid-argument-type] + +# 2. the two comment form used in qcodes +f("one") # type: ignore[arg-type] # ty: ignore[invalid-argument-type] + +# 3. combined form, ty rule only +f("one") # type: ignore[ty:invalid-argument-type] + +# 4. control, expected to be reported by every checker +f("one") +``` + +And for the pyright specific forms: + +```python +def h(a: int) -> None: ... + + +# 5. pyright: ignore, blanket +h("one") # pyright: ignore + +# 6. pyright: ignore with the matching rule +h("one") # pyright: ignore[reportArgumentType] + +# 7. pyright: ignore with a non matching rule +h("one") # pyright: ignore[reportGeneralTypeIssues] + +# 8. ty: ignore only +h("one") # ty: ignore[invalid-argument-type] +``` + +Run from the repository root so that the mypy configuration in `pyproject.toml` +is picked up: + +``` +uv run ty check --output-format concise +uv run --extra test mypy +uv run --extra test mypy --no-warn-unused-ignores +uv run pyright +``` + +Expected results: + +| block | ty | mypy | pyright | +| --- | --- | --- | --- | +| first, cases 1 to 4 | 4 | 1, 3, 4 and two unused directives | 4 | +| second, cases 5 to 8 | 5, 6, 7 | 5, 6, 7, 8 | 7, 8 | + +The second block deliberately exercises comments that only one checker +understands, so most cases are reported by the other two. That is the point: it +shows that `pyright: ignore` is inert for mypy and ty, and that `ty: ignore` is +inert for mypy and pyright. + +Any deviation from this table tells you that one of the checkers has changed how +it reads these comments. + +## Why we keep `warn_unused_ignores` + +Dropping `warn_unused_ignores` would make the combined form work, but that +setting is worth more than the shorter comments. As shown above it is, together +with ty's `unused-ignore-comment`, one of only two stale suppression checks +available to us. During the ty migration it caught: + +- the `issuperset` suppression becoming redundant once + [astral-sh/ty#4303](https://github.com/astral-sh/ty/issues/4303) was fixed in + ty 0.0.74 +- the two suppressions in the Keithley 7510 buffer becoming unnecessary once the + data dictionary was annotated +- several suppressions in `ParameterBase` becoming unnecessary once the duck + typed conversions were moved behind helpers + +## Suggested upstream change + +mypy could ignore codes carrying a `:` prefix in `type: ignore` comments, +rather than treating them as mypy codes that turned out to be unused. That would +make the form documented by ty usable in projects that run both checkers with +`warn_unused_ignores` enabled, and would generalise to any other checker that +wants to share the comment. + +Failing that, the ty documentation could note that the combined form conflicts +with mypy's `warn_unused_ignores`, and suggest the two comment form for projects +that run both. + +## Versions + +Measured with ty 0.0.74, mypy 2.3.1 and pyright 1.1.411. diff --git a/tests/dataset/test_snapshot.py b/tests/dataset/test_snapshot.py index b723033574e0..1a72ec0c7e1d 100644 --- a/tests/dataset/test_snapshot.py +++ b/tests/dataset/test_snapshot.py @@ -68,9 +68,7 @@ def test_station_snapshot_during_measurement( assert expected_snapshot == snapshot_from_dataset # 2. Test `snapshot_raw` property - # this is not part of the DatasetProtocol interface - # but we test it anyway - assert json_snapshot_from_dataset == data_saver.dataset.snapshot_raw # type: ignore[attr-defined] + assert json_snapshot_from_dataset == data_saver.dataset.snapshot_raw # 3. Test `snapshot` property diff --git a/tests/test_instrument.py b/tests/test_instrument.py index b2ba6dec987b..3b77fed4569c 100644 --- a/tests/test_instrument.py +++ b/tests/test_instrument.py @@ -212,8 +212,10 @@ def test_attr_access(testdummy: DummyInstrument) -> None: def test_parameter_property(testdummy: DummyInstrument) -> None: # since this is added dynamically we cannot know the type statically assert_type(testdummy.dac1, Any) - # this is an assigned attribute so we know it statically - assert_type(testdummy.fixed_parameter, Parameter) + # this is an assigned attribute so we know it statically. Without an + # explicit ``parameter_class`` the data and instrument types of the + # returned parameter are unknown, hence ``Parameter[Any, Any]``. + assert_type(testdummy.fixed_parameter, Parameter[Any, Any]) assert testdummy.fixed_parameter.get() == 5 testdummy.fixed_parameter.set(10) diff --git a/ty-issue-1-typeddict-self-bound.md b/ty-issue-1-typeddict-self-bound.md new file mode 100644 index 000000000000..70ea4ed48587 --- /dev/null +++ b/ty-issue-1-typeddict-self-bound.md @@ -0,0 +1,195 @@ +# ty issue draft 1 + +**Title** + +> Generic `TypedDict` with a type parameter default: `Self`-bound methods and `**`-unpacking rejected for every non-default specialization + +**Labels to suggest:** `bug`, `generics`, `typeddict`, `constraint-solver` + +--- + +### Summary + +When a generic `TypedDict` declares a default for its type parameter, ty computes +the upper bound of the synthesized `Self` type variable as the *default* +specialization rather than the generic one. Every other specialization is then +rejected by any method that binds `Self`. + +```python +from typing import TypedDict + + +class Movie[T = int](TypedDict): + extra: T + + +def f(m: Movie[str]) -> None: + m.keys() +``` + +``` +error[invalid-argument-type]: Argument to bound method `TypedDictFallback.keys` is incorrect + --> repro.py:7:5 + | +7 | m.keys() + | ^^^^^^^^ Argument type `Movie[str]` does not satisfy upper bound `Movie[int]` of type variable `Self` +``` + +Removing the default (`class Movie[T](TypedDict)`) makes the error go away +without any other change, so the default is what introduces the bound. + +Note that `Movie[str]` here is an ordinary concrete specialization. No type +variable is unsolved at the call site, and nothing is being inferred. + +### Which specializations are affected + +Only the declared default is accepted: + +| annotation | result | +| --- | --- | +| `Movie[int]` (the default) | ok | +| `Movie` (bare, default applies) | ok | +| `Movie[str]` | error | +| `Movie[T]` for an enclosing type variable `T` | error | + +### Which members are affected + +Members whose signature binds `Self`: + +| member | result | +| --- | --- | +| `keys()` | error | +| `values()` | error | +| `items()` | error | +| `copy()` | error | +| `**` unpacking | error | +| `get()` | ok | +| `setdefault()` | ok | +| `pop()` | ok | +| `update()` | ok | + +Assignability is unaffected, which is consistent with the problem being the +`Self` bound rather than the type itself: + +```python +from typing import Mapping, TypedDict + + +class Movie[T = int](TypedDict): + extra: T + + +def f(m: Movie[str]) -> None: + ok: Mapping[str, object] = m # no error +``` + +### The `**` unpacking symptom + +`**`-unpacking reports a different and rather misleading message, which is how I +originally ran into this: + +```python +from typing import TypedDict + + +class Movie[T = int](TypedDict): + extra: T + + +def f(m: Movie[str]) -> None: + dict(**m) +``` + +``` +error[invalid-argument-type]: Argument expression after ** must be a mapping type + --> repro.py:7:12 + | +7 | dict(**m) + | ^ Found `Movie[str]` +``` + +A `TypedDict` is always a `Mapping[str, object]`, so this message points away +from the real cause. + +### Not specific to `TypedDict` syntax or version + +The legacy spelling behaves identically: + +```python +from typing import Generic, TypedDict, TypeVar + +T = TypeVar("T", default=int) + + +class Movie(TypedDict, Generic[T]): + extra: T + + +def f(m: Movie[str]) -> None: + m.copy() +``` + +A plain generic class with a type parameter default is **not** affected, so this +looks specific to the synthesized `TypedDictFallback` `Self`: + +```python +class WithDefault[T = int]: + def m(self) -> None: ... + + +def f[T](a: WithDefault[T]) -> None: + a.m() # no error +``` + +Any non-`Any` default triggers it. `Any` is the only default that is accepted, +which is probably why this has gone unnoticed: + +| type parameter | result | +| --- | --- | +| `class Movie[T](TypedDict)` | ok | +| `class Movie[T = Any](TypedDict)` | ok | +| `class Movie[T = int](TypedDict)` | error | +| `class Movie[T = None](TypedDict)` | error | +| `class Movie[T = int \| None](TypedDict)` | error | +| `class Movie[T = object](TypedDict)` | error | +| `class Movie[T: int \| None = int \| None](TypedDict)` | error | + +Reproduced on 0.0.72, 0.0.73 and 0.0.74. Checked with `--python-version 3.13` so +that the PEP 696 syntax is not itself reported as an error. mypy 2.3.1 and +pyright both accept all of the above. + +I searched existing issues for `"must be a mapping type"`, `TypedDict Unpack +default`, `"generic TypedDict"`, `"PEP 696"` and `Unpack kwargs` and did not +find a preexisting issue. #4255 is the closest but is about a union alias in a +stub leaking an unspecialized type variable. + +The error shape is reminiscent of #4303, which is also an upper bound on a type +variable being applied too strictly, though that one is about a `bound=` on a +class scoped type variable rather than a `default=` on `Self`. + +### Relation to the feature overview + +The type system feature overview in #1889 lists all of the following as +implemented: + +- Generics: `TypeVar` defaults (PEP 696) +- `TypedDict`: Inheritance, generic `TypedDict`s +- `TypedDict`: Structural assignability and equivalence +- `TypedDict`: Methods (`get`, `pop`, `setdefault`, `keys`, `values`, `copy`) + +This report sits at the intersection of those, so following the guidance at the +top of #1889 for features marked completed, it seemed worth reporting rather +than upvoting a tracking issue. + +It is worth stressing that this is **not** about `Unpack` for `**kwargs` typing, +which #1889 tracks separately in #1746. The lead repro contains no `Unpack` and +no `**` at all, just `Movie[str].keys()`. The `**` message is only how I +happened to notice it. + +Structural assignability also still works (`Mapping[str, object] = m` is +accepted), so this looks narrowly scoped to the upper bound computed for the +synthesized `Self`. + +### Version + +0.0.74 diff --git a/ty-issue-2-typevar-default-context.md b/ty-issue-2-typevar-default-context.md new file mode 100644 index 000000000000..62eec02d3d0c --- /dev/null +++ b/ty-issue-2-typevar-default-context.md @@ -0,0 +1,142 @@ +# ty issue draft 2 + +**Title** + +> Function scoped `TypeVar` default takes precedence over the declared type context, where an unsolved type variable would be accepted + +**Labels to suggest:** `bidirectional inference`, `constraint-solver`, `generics` + +--- + +### Summary + +When a function scoped type variable appears only in the return type and is not +constrained by any argument, ty leaves it unsolved as `Unknown`, which is +gradually compatible with whatever the result is assigned to. If that same type +variable declares a PEP 696 default, ty substitutes the default instead, which +is concrete and then conflicts with the declared type. + +```python +class Box[T]: + pass + + +def make[T = int](cls: type[T] | None = None) -> Box[T]: + raise NotImplementedError + + +def caller() -> None: + a: Box[str] = make() +``` + +``` +error[invalid-assignment]: Object of type `Box[int]` is not assignable to `Box[str]` + --> repro.py:8:19 + | +8 | a: Box[str] = make() + | ^^^^^^ +``` + +Removing the default makes ty accept it: + +```python +class Box[T]: + pass + + +def make[T](cls: type[T] | None = None) -> Box[T]: + raise NotImplementedError + + +def caller() -> None: + a: Box[str] = make() # ty: ok +``` + +`reveal_type` shows what is actually happening. The declared type is never used +to solve `T` in either case; the difference is only what fills the unsolved slot: + +| declaration | `reveal_type(make())` | `a: Box[str] = make()` | +| --- | --- | --- | +| `def make[T](...) -> Box[T]` | `Box[Unknown]` | accepted | +| `def make[T = int](...) -> Box[T]` | `Box[int]` | error | + +So adding a default is strictly worse than having no default at all, at every +call site that annotates its target. mypy 2.3.1 and pyright accept both forms. + +### The type context is available + +This is not a case of ty lacking the necessary context. Using the example from +#3933, the declared type of the assignment target clearly does reach the +constraint solver, since it widens the argument: + +```python +class Parent: ... + + +class Child(Parent): ... + + +def head[T](x: list[T]) -> T: + return x[0] + + +x: Parent = head(reveal_type([Child()])) # revealed: list[Parent] +``` + +I reproduced that on 0.0.74. So in `a: Box[str] = make()` the constraint +`Box[T] <: Box[str]` is available, but the default is applied in preference to +it. + +### Why this matters + +This pattern is common in factory functions, where the default exists to give a +sensible type to an unannotated call while still allowing the caller to ask for +something more specific (illustrative, from our codebase): + +```python +p = instrument.add_parameter("name") # want the default +q: Parameter[float, Self] = instrument.add_parameter("x") # want this instead +``` + +With ty's current behaviour the default wins in both cases, so the second form +is unusable and every annotated call site becomes an error. In our codebase this +produced 34 errors across instrument drivers from a single type variable +declaration. We ended up widening the default to a fully gradual type to work +around it, which loses the information the default was there to provide. + +### Relation to #3933 and the feature overview + +This looks like it may fall under #3933, constraint-set-aware bidirectional +inference. That issue is written in terms of constraints flowing into *argument* +inference, and all of its examples involve arguments that get eagerly +specialized or wrongly widened. The case here has no arguments at all, so the +symptom is different, but the underlying gap looks similar: the outer constraint +is not being unified with the specialization of the call. + +If the second approach in #3933 is taken, propagating constraints during +bidirectional inference rather than eagerly specializing, then `Box[T] <: +Box[str]` should presumably solve `T` to `str` before any default is considered, +which would fix this too. Filing separately in case that is not the intent, and +because the interaction with PEP 696 defaults is not mentioned there. + +The type system feature overview in #1889 lists "`TypeVar` defaults (PEP 696)" +as implemented under Generics. That section also has an open sub-item, "Solve +type variables in all cases" (#623), which may be the more appropriate home if +this is considered a solver limitation rather than a deliberate choice about +defaults. + +### Note on the spec + +I could not find wording in PEP 696 or the typing spec that settles whether the +declared type context should take precedence over a type variable default, so +this may be intentional. If it is, it would be helpful to say so explicitly, +since the natural reading of "the default is used when the type variable cannot +be solved" is that a solution derived from the type context counts as solving +it. The current behaviour also has the surprising property that adding a default +makes a call site fail that would otherwise have been accepted. + +Reproduced on 0.0.72, 0.0.73 and 0.0.74, checked with `--python-version 3.13`. + +### Version + +0.0.74