fix(instrument): silence get_idn warning on virtual instruments#8038
fix(instrument): silence get_idn warning on virtual instruments#8038officialasishkumar wants to merge 1 commit intomicrosoft:mainfrom
Conversation
``Instrument.get_idn`` calls ``self.ask("*IDN?")`` which in turn invokes
``ask_raw``. The base ``Instrument`` class leaves ``ask_raw`` as a stub that
raises ``NotImplementedError``; virtual instruments that do not override it
therefore always produced a warning on every call to ``get_idn``. This is
noisy in practice, for example when a virtual instrument is registered in a
station and the station queries its IDN.
Treat ``NotImplementedError`` specifically as the expected signal that the
instrument is virtual and return the default ``None`` dict without a warning.
All other exceptions are still surfaced via ``self.log.warning`` so genuine
communication failures remain visible.
Adds focused regression tests covering both the silenced virtual-instrument
path and the still-noisy unexpected-error path.
Closes microsoft#4611
Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR updates Instrument.get_idn to treat NotImplementedError (raised by the base Instrument.ask_raw stub) as an expected condition for virtual/in-memory instruments, avoiding spurious warnings while keeping warnings for genuine unexpected failures.
Changes:
- Handle
NotImplementedErrorinInstrument.get_idnwithout logging a warning and return the defaultNone-valued IDN dict. - Add regression tests ensuring virtual instruments don’t warn, while other exceptions still do.
- Add a Towncrier newsfragment documenting the behavioral change.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/qcodes/instrument/instrument.py |
Narrows exception handling in get_idn to silence warnings only for NotImplementedError. |
tests/test_instrument.py |
Adds regression coverage for the new get_idn warning behavior. |
docs/changes/newsfragments/4611.improved |
Documents the change in user-visible behavior for virtual instruments. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8038 +/- ##
=======================================
Coverage 70.63% 70.63%
=======================================
Files 333 333
Lines 32330 32332 +2
=======================================
+ Hits 22837 22839 +2
Misses 9493 9493 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # in case parts at the end are missing, fill in None | ||
| if len(idparts) < 4: | ||
| idparts += [None] * (4 - len(idparts)) | ||
| except NotImplementedError: |
There was a problem hiding this comment.
Rather than doing this I think my preference would be to add a VirtualInstrument base class. This should probably be a sibling class to Instrument e.g. inherit from InstrumentBase but not from instrument. This method can supply an empty idn dict
Summary
Closes #4611.
Instrument.get_idncallsself.ask("*IDN?")which delegates toask_raw. The baseInstrumentclass leavesask_rawas a stub that raisesNotImplementedError, so any virtual instrument that does not override it — e.g. a purely in-memory model — always produced a warning on every call toget_idn. That regularly appears when a virtual instrument is added to aStationand the station queries its IDN during snapshotting.This PR narrows the
exceptblock so thatNotImplementedErroris treated as the expected signal of a virtual instrument and returns the defaultNone-valued dict silently. All other exceptions still go throughself.log.warningwithexc_info=True, so genuine communication failures remain visible.src/qcodes/instrument/instrument.py: split the broadexcept ExceptionsoNotImplementedErroris handled without a warning.tests/test_instrument.py: two regression tests — one confirms no warning is emitted for a virtual instrument withoutask_raw, the other confirms that unexpected errors (e.g.RuntimeErrorfrom a brokenask_raw) still produce a warning.Test plan
pytest tests/test_instrument.py— 34 passed locally, including the two new tests.pytest tests/test_instrument.py -k get_idn— 3 passed.