Skip to content

fix(instrument): silence get_idn warning on virtual instruments#8038

Open
officialasishkumar wants to merge 1 commit intomicrosoft:mainfrom
officialasishkumar:virtual_instrument_get_idn
Open

fix(instrument): silence get_idn warning on virtual instruments#8038
officialasishkumar wants to merge 1 commit intomicrosoft:mainfrom
officialasishkumar:virtual_instrument_get_idn

Conversation

@officialasishkumar
Copy link
Copy Markdown

Summary

Closes #4611.

Instrument.get_idn calls self.ask("*IDN?") which delegates to ask_raw. The base Instrument class leaves ask_raw as a stub that raises NotImplementedError, so any virtual instrument that does not override it — e.g. a purely in-memory model — always produced a warning on every call to get_idn. That regularly appears when a virtual instrument is added to a Station and the station queries its IDN during snapshotting.

This PR narrows the except block so that NotImplementedError is treated as the expected signal of a virtual instrument and returns the default None-valued dict silently. All other exceptions still go through self.log.warning with exc_info=True, so genuine communication failures remain visible.

  • src/qcodes/instrument/instrument.py: split the broad except Exception so NotImplementedError is handled without a warning.
  • tests/test_instrument.py: two regression tests — one confirms no warning is emitted for a virtual instrument without ask_raw, the other confirms that unexpected errors (e.g. RuntimeError from a broken ask_raw) still produce a warning.
  • Added a newsfragment.

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.

``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>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 NotImplementedError in Instrument.get_idn without logging a warning and return the default None-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
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.63%. Comparing base (36967bf) to head (ddaa400).

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.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

get_idn should not throw for virtual instruments

2 participants