Repository navigation
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Logger identity collisions and mutable scope resolution can break the promised isolation and hierarchy.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (5)
Use module-qualified identities for per-driver logger isolation · New Store scope at root initialization for all descendants · New Avoid ambient logging levels in class logger test · New Clarify propagate=False and separate VISA logger behavior · New Document permanent logging-registry growth caveat · New
What changed in this PR
Adds opt-in per-instrument logger hierarchies while preserving shared logging by default.
Changes:
- Adds configurable logger scope and hierarchical names.
- Applies scoped naming to VISA loggers.
- Adds tests, documentation, and a newsfragment.
| File | Description |
|---|---|
instrument_base.py |
Defines logger scopes and scoped-name generation. |
visa.py |
Applies scoped VISA logging. |
ip_to_visa.py |
Applies scoped logging to simulated VISA instruments. |
test_logger.py |
Tests scope, inheritance, filtering, and VISA behavior. |
logging_example.ipynb |
Documents scoped logger usage. |
8523.new |
Announces the feature. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
One or more custom setup steps configured for this repository failed during this Copilot code review run: Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review. Note You can configure setup steps for Copilot code review separately from Copilot cloud agent with a |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8523 +/- ##
==========================================
+ Coverage 72.00% 72.32% +0.32%
==========================================
Files 305 307 +2
Lines 32019 32335 +316
==========================================
+ Hits 23055 23387 +332
+ Misses 8964 8948 -16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The permanent logging registry growth under the "instrument" scope was only described in the PR discussion, not in the user facing documentation. Add it to the caveats of the logging example notebook, together with the mitigation that instruments with stable names reuse their existing logger. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2a787d42-150d-48cb-8bc7-47add23e17ad
|
ymampaey please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|



Closes #8522
Summary
Every instrument currently shares one
logging.Loggerobject becauseInstrumentBase.__init__uses the module nameqcodes.instrument.instrument_base. VISA communication similarly uses theshared
qcodes.instrument.instrument_base.com.visalogger.This PR lets a driver opt in to module-qualified scoped loggers:
QCoDeS only selects the logger name. It does not set levels, attach handlers,
or change
propagate; those remain application and driver policy.The default behavior is unchanged. Drivers that do not opt in continue to
use:
How logger names are selected
InstrumentBase._logger_namereceives the existing shared logger name as arequired fallback:
Regular instrument logging calls
_logger_name(__name__). VISA logging calls_logger_name(VISA_LOGGER, "com", "visa").For an unscoped driver,
_logger_nameimmediately returns__name__orVISA_LOGGER, preserving the existing names. For a scoped driver, thatfallback is ignored and the name is assembled from:
com.visa.name_parts.The scope is resolved and stored once when the root instrument is initialized.
All channels and later-added submodules read the stored value from
root_instrument, preventing a hierarchy from splitting if the class attributechanges later.
Naming examples
QCoDeS driver
Custom driver
For
MyDriverdefined inmy_lab.drivers.my_driver:A driver defined directly in a notebook starts with
__main__:VISA driver
For
AMIModel430and an instrument namedmag_x:The class logger is an ancestor of both the regular instrument branch and the
com.visabranch. Setting the class logger level therefore covers both drivermessages and wire traffic:
Design notes
Why module plus qualified class name. A class name alone is not unique.
Drivers with the same class name in different modules now receive distinct
logger trees.
Why the class comes from
root_instrument. Usingtype(self)would put achannel under its channel class rather than its instrument class. Using the
root class keeps channels below their instrument logger.
Why
name_partsrather thanfull_name. Joining individual name partswith
.makes a channel logger a real descendant of its instrument logger.Why VISA uses
com.visa. It keeps driver messages and wire trafficseparately configurable while allowing the module-qualified driver class logger
to control both branches.
Documented caveats
...AMIModel430.mag_xdoes not reach...AMIModel430.com.visa.mag_x; set the class level or configure both.combecomes an ancestor of thecom.visabranch.qcodes.instrument.instrument_basehierarchy andtherefore no longer inherit levels configured there.
qcodes, for exampleqcodes_contrib_driversor__main__.<locals>in__qualname__.%(name)soutput field.Changes
src/qcodes/instrument/instrument_base.pysrc/qcodes/instrument/visa.py,ip_to_visa.pycom.visabranchtests/test_logger.pydocs/examples/logging/logging_example.ipynbdocs/changes/newsfragments/8523.newValidation
.venv\Scripts\python.exe -m pytest tests\test_logger.py— 33 passed..venv\Scripts\python.exe -m pytest tests --reruns 1— 3255 passed,268 skipped.
.venv\Scripts\pyright.exe --pythonpath .venv\Scripts\python.exe— 0 errors,0 warnings.
pre-commit run --all— all hooks passed.docs/examples/logging/logging_example.ipynb— executed successfully end toend.