Add well-named snapshot update modes, at measurement start do only-invalid snapshot, a few driver fixes of snapshotting - #8354
Conversation
Add string update values `All`/`Only_invalid`/`Never` to `snapshot`/`snapshot_base` (from `Metadatable` down), keeping the legacy `True`/`None`/`False` as aliases. `Only_invalid` snapshots only refresh parameters whose cache is invalid via `cache.get(get_if_invalid=True)`. The station snapshot taken by `Measurement` now uses `Only_invalid`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa3f57b5-ddc1-45ce-b920-3f8efed6851c
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa3f57b5-ddc1-45ce-b920-3f8efed6851c
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8354 +/- ##
==========================================
- Coverage 71.12% 71.09% -0.04%
==========================================
Files 305 305
Lines 32010 31944 -66
==========================================
- Hits 22768 22711 -57
+ Misses 9242 9233 -9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…precate legacy Address review feedback: - normalize_snapshot_update now returns the canonical SnapshotUpdate literal (All/Only_invalid/Never); all internal code branches on those values. - Export normalize_snapshot_update and SnapshotUpdate from qcodes.metadatable. - Default of update for snapshot/snapshot_base/print_readable_snapshot is now 'Only_invalid' across the Metadatable hierarchy. - Deprecate the legacy bool/None update values via typing_extensions.deprecated overloads on Metadatable.snapshot (static only, no runtime warning); internal code and docs use only the string values (except the Metadatable note). - Measurement registered-parameter snapshots pinned to 'Never' to preserve prior behavior after the default change. - Update docstrings, the 'Working with snapshots' notebook and driver examples; add newsfragments (.new and .breaking). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa3f57b5-ddc1-45ce-b920-3f8efed6851c
|
Updated based on review plus two follow-ups:
Validation: |
There was a problem hiding this comment.
🟡 Not ready to approve
Some driver and helper snapshot implementations currently don’t fully honor the new "Only_invalid"/"Never" semantics (leading to unnecessary instrument queries and ineffective update="Never" in at least one case).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR formalizes the snapshot(..., update=...) behavior across the Metadatable hierarchy by introducing a canonical string enum ("All", "Only_invalid", "Never") and switching the default snapshot behavior to refresh only invalid caches.
Changes:
- Introduces
SnapshotUpdate+normalize_snapshot_updateand updatessnapshot/snapshot_base/print_readable_snapshotsignatures and docstrings to use the new modes (with deprecated bool/None aliases). - Updates measurement snapshotting semantics: station snapshots use
"Only_invalid", while registered-parameter snapshots are pinned to"Never". - Refreshes tests, driver examples, notebooks, and newsfragments to use the new string values and documents the behavior change.
File summaries
| File | Description |
|---|---|
| tests/test_snapshot.py | Updates snapshot tests to use update="Never"/"All" instead of bools. |
| tests/test_instrument.py | Updates instrument snapshot test to use update="All". |
| tests/parameter/test_snapshot.py | Expands/updates parameter snapshot behavior tests for the new update modes and normalization helper. |
| tests/parameter/test_parameter_cache.py | Updates cache invalidation test to use update="Only_invalid" (and updates accompanying comment). |
| tests/parameter/test_parameter_basics.py | Updates parameter basics snapshot tests to use update="All". |
| tests/parameter/test_multi_parameter.py | Updates multi-parameter snapshot tests to use update="All". |
| tests/parameter/test_delegate_parameter.py | Updates delegate parameter snapshot test to use update="All". |
| tests/parameter/test_array_parameter.py | Updates array parameter snapshot tests to use update="All". |
| tests/parameter/conftest.py | Updates update fixture to parametrize over the new string modes. |
| tests/drivers/test_ami430_visa.py | Updates driver snapshot tests to use update="All". |
| tests/drivers/keysight_b1500/b1500_driver_tests/test_b1500.py | Updates driver snapshot test to use update="All". |
| tests/dataset/test_snapshot.py | Adds a regression test ensuring Measurement station snapshot refreshes only invalid caches. |
| src/qcodes/station.py | Changes station snapshot default to "Only_invalid" and normalizes update mode. |
| src/qcodes/parameters/sweep_values.py | Normalizes update mode before snapshotting nested parameter. |
| src/qcodes/parameters/parameter_base.py | Implements "All"/"Only_invalid"/"Never" semantics for parameter snapshotting via cache validity. |
| src/qcodes/parameters/delegate_parameter.py | Normalizes update mode in delegate parameter snapshotting. |
| src/qcodes/parameters/combined_parameter.py | Updates signature/docs for new update modes. |
| src/qcodes/metadatable/metadatable_base.py | Defines SnapshotUpdate, adds normalize_snapshot_update, and updates Metadatable.snapshot overloads/default. |
| src/qcodes/metadatable/init.py | Exports SnapshotUpdate and normalize_snapshot_update. |
| src/qcodes/instrument/visa.py | Updates snapshot update typing/docs to new modes. |
| src/qcodes/instrument/ip.py | Updates snapshot update typing/docs to new modes. |
| src/qcodes/instrument/instrument_base.py | Normalizes update mode and updates snapshot/print_readable_snapshot defaults and docs. |
| src/qcodes/instrument/channel.py | Normalizes update mode and updates snapshot/print_readable_snapshot defaults and docs. |
| src/qcodes/instrument_drivers/stanford_research/SR86x.py | Updates snapshot update typing/default to "Only_invalid". |
| src/qcodes/instrument_drivers/QuantumDesign/DynaCoolPPMS/DynaCool.py | Updates a forced refresh call to update="All". |
| src/qcodes/instrument_drivers/QDev/QDac_channels.py | Updates snapshot defaults/typing and adapts driver-specific snapshot cache logic to string modes. |
| src/qcodes/instrument_drivers/mock_instruments/init.py | Updates mock driver snapshot default typing. |
| src/qcodes/instrument_drivers/Keysight/private/Keysight_344xxA_submodules.py | Updates forced refresh call to update="All". |
| src/qcodes/instrument_drivers/Keysight/keysightb1500/KeysightB1517A.py | Updates snapshot update typing/default to "Only_invalid". |
| src/qcodes/instrument_drivers/Keithley/_Keithley_2600.py | Updates snapshot update typing/default and forced refresh calls to update="All". |
| src/qcodes/instrument_drivers/american_magnetics/AMI430_visa.py | Normalizes update mode and switches update is True checks to update == "All". |
| src/qcodes/instrument_drivers/AimTTi/_AimTTi_PL_P.py | Updates forced refresh call to update="All". |
| src/qcodes/dataset/measurements.py | Uses station.snapshot(update="Only_invalid") and pins registered-parameter snapshots to update="Never". |
| docs/examples/driver_examples/Qcodes example with Tektronix AWG70002A.ipynb | Updates example snapshot call to update="All". |
| docs/examples/driver_examples/Qcodes example with Rohde Schwarz ZNB.ipynb | Updates example snapshot call to update="All". |
| docs/examples/driver_examples/Qcodes example with Rohde Schwarz SGS100A.ipynb | Updates example snapshot call to update="All". |
| docs/examples/driver_examples/Qcodes example with Rohde Schwarz RTO 1000 series Oscilloscope.ipynb | Updates example snapshot call to update="All". |
| docs/examples/driver_examples/Qcodes example with Oxford Mercury iPS.ipynb | Updates example snapshot call to update="All". |
| docs/examples/driver_examples/Qcodes example with Keysight 344xxA.ipynb | Updates example snapshot call to update="All". |
| docs/examples/driver_examples/Qcodes example with HP8753D.ipynb | Updates example snapshot call to update="All". |
| docs/examples/driver_examples/Qcodes example with DynaCool PPMS.ipynb | Updates example snapshot call to update="All". |
| docs/examples/driver_examples/QCoDeS example with CopperMountain_M5065.ipynb | Updates example snapshot call to update="All". |
| docs/examples/DataSet/Working with snapshots.ipynb | Documents the new update modes and measurement snapshot behavior. |
| docs/changes/newsfragments/8354.new | Announces the new update mode enum and normalization helper. |
| docs/changes/newsfragments/8354.breaking | Documents the default change to "Only_invalid" and how to restore old behavior. |
Review details
Comments suppressed due to low confidence (1)
src/qcodes/instrument_drivers/QDev/QDac_channels.py:350
- Instrument-level
snapshot_basealways calls_update_cacheforupdate="Only_invalid"(update != "Never"), which can make the new default snapshot mode slow even when all channel caches are already valid. Consider only calling_update_cachefor"All", or for"Only_invalid"when any cached channel values (e.g. v/vrange/irange) are invalid.
update = normalize_snapshot_update(update)
update_currents = self._update_currents and update == "All"
if update != "Never":
self._update_cache(readcurrents=update_currents)
self._get_status_performed = True
- Files reviewed: 45/45 changed files
- Comments generated: 3
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
jenshnielsen
left a comment
There was a problem hiding this comment.
Please have a look at the open copilot questions. Otherwise looks good.
…meter forwarding - QDac snapshot: only perform the bulk status read (_update_cache) for update='All', or for 'Only_invalid' when a channel cache that would be reported is actually invalid, instead of on every non-'Never' snapshot. Clarify the confusing comment (parent._update -> self.parent._update_currents). - CombinedParameter.snapshot_base now forwards the normalized update value to the underlying parameters' snapshots (previously it ignored update). Add a test. - Fix 'os' -> 'so' typo in a test comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa3f57b5-ddc1-45ce-b920-3f8efed6851c
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa3f57b5-ddc1-45ce-b920-3f8efed6851c
Reworks the
updateargument ofsnapshot/snapshot_base(across the wholeMetadatablehierarchy) into an enumeration and refreshes only invalid caches bydefault.
"All"/"Only_invalid"/"Never". LegacyTrue/None/Falsestill work as deprecated aliases (static-only via@deprecated, noruntime warning).
"Only_invalid"forsnapshot/snapshot_base/print_readable_snapshot(was effectively"Never"), so.snapshot()nowrefreshes parameters with an invalid cache. Pass
update="Never"for the oldbehavior. See
8354.breaking."Only_invalid"; registered-parametersnapshots pinned to
"Never".qcodes.metadatable.normalize_snapshot_update+SnapshotUpdatetype.the enum. Newsfragments:
8354.new,8354.breaking.Validation:
pyright(src+tests) andmypy -p qcodesclean,pre-commitpasses,full suite 3135 passed / 268 skipped.