Change return type of root_instrument to Instrument by jenshnielsen · Pull Request #7924 · microsoft/Qcodes

Jens Hedegaard Nielsen (jenshnielsen) · 2026-03-12T15:31:08Z

The point of root_instrument is to get to the instrument.

Breaking change notice

codecov · 2026-03-13T09:27:12Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.26%. Comparing base (7428872) to head (9fa7995).

Additional details and impacted files

@@ Coverage Diff @@## main #7924 +/- ##
=======================================
Coverage 70.26% 70.26% =======================================
Files 333 333 Lines 32169 32169 =======================================
Hits 22604 22604 Misses 9565 9565

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates root_instrument typing/usage to better represent the “top-level instrument” concept and adjusts tests accordingly.

Changes:

Changes ParameterBase.root_instrument return type to Instrument | None (with casting) and adjusts monitor typing usage.
Updates InstrumentBase.root_instrument typing and adds/updates overrides in several drivers/channels.
Strengthens driver tests by asserting mocked root_instrument behavior/types.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/drivers/keysight_b1500/b1500_driver_tests/test_sampling_measurement.py	Adds assertion that `root_instrument.ask` is mocked in tests.
tests/drivers/keysight_b1500/b1500_driver_tests/test_b1520a_cmu.py	Adds assertions that `root_instrument` is a `MagicMock` before using `.ask`.
tests/drivers/keysight_b1500/b1500_driver_tests/test_b1517a_smu.py	Adds assertion that `root_instrument` is a `MagicMock` before verifying `.write` calls.
src/qcodes/parameters/parameter_base.py	Changes `root_instrument` type to `Instrument
src/qcodes/monitor/monitor.py	Updates local typing for `baseinst` when using `parameter.root_instrument`.
src/qcodes/instrument_drivers/tektronix/DPO7200xx.py	Overrides `root_instrument` return type with `type: ignore[override]` and runtime assert.
src/qcodes/instrument_drivers/Keysight/N52xx.py	Same as above for Keysight PNA driver.
src/qcodes/instrument_drivers/Keysight/KtMAwg.py	Same as above for Keysight AWG driver.
src/qcodes/instrument_drivers/Keysight/Infiniium.py	Same as above for Keysight Infiniium driver.
src/qcodes/instrument/instrument_base.py	Changes `root_instrument` return type annotation to `Self`.
src/qcodes/instrument/channel.py	Overrides `root_instrument` return type to `Instrument` (via `cast`) with expanded comment.

Copilot · 2026-04-14T09:00:33Z


 @property
- def root_instrument(self) -> InstrumentBase:
+ def root_instrument(self) -> Self:


The PR title/description says the return type of root_instrument is being changed to Instrument, but InstrumentBase.root_instrument is now annotated as returning Self. Self implies the return value is the same concrete type as self, which is not generally true for modules/channels where root_instrument typically returns the top-level Instrument (a different class than the module/channel). Consider adjusting the base annotation to a type that matches the actual contract (e.g., InstrumentBase or InstrumentBase + a narrower override on Instrument/channels), or update the PR description/title to reflect that this is a Self-typing change rather than an Instrument return-type change.

Copilot · 2026-04-14T09:00:34Z

+ # the root instrument is the top level parent of this module, we need to
+ # go up the parent hierarchy until we find an object that returns itself as the parent, this should be the root instrument. We also
+ # this is required to be an Instrument.
+ # Once 3.13 is the minimum supported version
+ # consider replacing with a generic parameter with a default
+ # value of Instrument.
+ return cast("Instrument", self._parent.root_instrument)



The comment describes walking the parent hierarchy to locate the root, but the implementation simply returns self._parent.root_instrument with a cast. This makes the comment misleading and also relies on cast (type-only) rather than a runtime check. Consider either (a) simplifying/updating the comment to match the implementation and invariants, and/or (b) using a runtime isinstance(..., Instrument) assertion (similar to the driver overrides in this PR) to enforce the contract that the channel’s root is an Instrument.
Suggested change
# the root instrument is the top level parent of this module, we need to
# go up the parent hierarchy until we find an object that returns itself as the parent, this should be the root instrument. We also
# this is required to be an Instrument.
# Once 3.13 is the minimum supported version
# consider replacing with a generic parameter with a default
# value of Instrument.
returncast("Instrument", self._parent.root_instrument)
# Delegate root resolution to the parent and enforce that the
# resolved root instrument is an Instrument at runtime.
# Once 3.13 is the minimum supported version
# consider replacing with a generic parameter with a default
# value of Instrument.
from .instrumentimportInstrument
root_instrument=self._parent.root_instrument
assertisinstance(root_instrument, Instrument)
returnroot_instrument

Copilot · 2026-04-14T09:00:34Z

- def root_instrument(self) -> "TektronixDPO7000xx":
+ def root_instrument(self) -> "TektronixDPO7000xx": # type: ignore[override]
+ # ideally this should be a generic type parameter but
+ # for now we override it here. This requies a mypy ignore


Corrected spelling of 'requies' to 'requires'.
Suggested change
# for now we override it here. This requies a mypy ignore
# for now we override it here. This requires a mypy ignore

Copilot · 2026-04-14T09:00:34Z

- def root_instrument(self) -> "KeysightPNABase":
+ def root_instrument(self) -> "KeysightPNABase": # type: ignore[override]
+ # ideally this should be a generic type parameter but
+ # for now we override it here. This requies a mypy ignore


Corrected spelling of 'requies' to 'requires'.
Suggested change
# for now we override it here. This requies a mypy ignore
# for now we override it here. This requires a mypy ignore

Copilot · 2026-04-14T09:00:35Z

- def root_instrument(self) -> "KeysightM9336A":
+ def root_instrument(self) -> "KeysightM9336A": # type: ignore[override]
+ # ideally this should be a generic type parameter but
+ # for now we override it here. This requies a mypy ignore


Corrected spelling of 'requies' to 'requires'.
Suggested change
# for now we override it here. This requies a mypy ignore
# for now we override it here. This requires a mypy ignore

Copilot · 2026-04-14T09:00:35Z

- def root_instrument(self) -> "KeysightInfiniium":
+ def root_instrument(self) -> "KeysightInfiniium": # type: ignore[override]
+ # ideally this should be a generic type parameter but
+ # for now we override it here. This requies a mypy ignore


Corrected spelling of 'requies' to 'requires'.
Suggested change
# for now we override it here. This requies a mypy ignore
# for now we override it here. This requires a mypy ignore

Jens Hedegaard Nielsen (jenshnielsen) commented Mar 13, 2026
View reviewed changes

Comment threadsrc/qcodes/instrument/delegate/delegate_instrument.py

Jens Hedegaard Nielsen (jenshnielsen)force-pushed the jenshnielsen/root_instrument_type branch 2 times, most recently from 269ad6e to 9fa2192Compare April 7, 2026 19:41

Jens Hedegaard Nielsen (jenshnielsen) added 3 commits April 10, 2026 07:26

More specific return type for root_instrument
1e09c49

Parameter.root_instrument is Instrument
7fdecd0

Fix mypy type checking
9fa7995

Jens Hedegaard Nielsen (jenshnielsen)force-pushed the jenshnielsen/root_instrument_type branch from 9fa2192 to 9fa7995Compare April 10, 2026 05:34

Jens Hedegaard Nielsen (jenshnielsen) requested a review from Copilot April 14, 2026 08:52

CopilotAI reviewed Apr 14, 2026
View reviewed changes

Copilotstarted reviewing on behalf of Jens Hedegaard Nielsen (jenshnielsen)April 14, 2026 09:12 View session

Navigation Menu

Uh oh!

Change return type of root_instrument to Instrument - #7924

Change return type of root_instrument to Instrument#7924
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 3 commits into
microsoft:mainfrom
jenshnielsen:jenshnielsen/root_instrument_type

Jens Hedegaard Nielsen (jenshnielsen) commented Mar 12, 2026•
edited
Loading

Uh oh!

Uh oh!

codecovBot commented Mar 13, 2026•
edited
Loading

Uh oh!

CopilotAI left a comment

Uh oh!

CopilotAIApr 14, 2026

Uh oh!

CopilotAIApr 14, 2026

Uh oh!

CopilotAIApr 14, 2026

Uh oh!

CopilotAIApr 14, 2026

Uh oh!

CopilotAIApr 14, 2026

Uh oh!

CopilotAIApr 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	# for now we override it here. This requies a mypy ignore
	# for now we override it here. This requires a mypy ignore

Uh oh!

Conversation

Jens Hedegaard Nielsen (jenshnielsen) commented Mar 12, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

codecovBot commented Mar 13, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

CopilotAIApr 14, 2026

Choose a reason for hiding this comment

Uh oh!

CopilotAIApr 14, 2026

Choose a reason for hiding this comment

Uh oh!

CopilotAIApr 14, 2026

Choose a reason for hiding this comment

Uh oh!

CopilotAIApr 14, 2026

Choose a reason for hiding this comment

Uh oh!

CopilotAIApr 14, 2026

Choose a reason for hiding this comment

Uh oh!

CopilotAIApr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Jens Hedegaard Nielsen (jenshnielsen) commented Mar 12, 2026•
edited
Loading

codecovBot commented Mar 13, 2026•
edited
Loading