Conversation
…ty access Convert PlotParameters dict-returning properties in artist.py to @cached_property with MappingProxyType for immutable, cached access. Wrap DetectorResult.detectors and .observables in tuple() to prevent mutation of internal lists. Add comment to ArchSpec.paths explaining why it remains mutable (populated incrementally after construction). Closes #284 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors several Python APIs to return cached, read-only views of collection-like properties (via @cached_property, MappingProxyType, and tuple()), reducing repeated allocations and discouraging accidental mutation of internal state.
Changes:
- Converted
PlotParametersdict-returning properties to@cached_propertyreturningMappingProxyType. - Updated
DetectorResult.detectors/DetectorResult.observablesto return tuples instead of the underlying mutable lists. - Adjusted tests and added documentation note clarifying
ArchSpec.pathsis intentionally mutable.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
python/bloqade/lanes/visualize/artist.py |
Cache and wrap plot-argument dicts in MappingProxyType to avoid reallocation and mutation. |
python/bloqade/lanes/device.py |
Return tuples for detector/observable shot collections to reduce external mutation surface. |
python/bloqade/lanes/layout/arch.py |
Document that paths is intentionally mutable and incrementally populated. |
python/tests/visualize/test_artist.py |
Update tests to expect MappingProxyType for plot-argument properties. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
weinbe58
commented
Mar 20, 2026
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
…284) - Add from __future__ import annotations to artist.py for Python 3.10 compatibility with MappingProxyType[str, Any] annotations - Make PlotParameters frozen=True to keep cached_property coherent - Deep-copy inner lists in DetectorResult: return tuple(tuple(shot)) instead of tuple(self._list) to prevent mutation of inner lists - Make ArchSpec.paths read-only with MappingProxyType — refactor impls.py to merge path dicts before construction instead of mutating after - Add caching identity check to test_plot_parameters_properties Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #284. Applies
@cached_property+MappingProxyType/tuple()patterns to mutable collection properties across the codebase.Changes
python/bloqade/lanes/visualize/artist.pyPlotParametersproperties (atom_plot_args,gate_spot_args,slm_plot_args,aod_line_args,aod_marker_args,atom_label_args) converted from@propertyto@cached_propertyreturningMappingProxyType[str, Any]python/bloqade/lanes/device.pyDetectorResult.detectorsandDetectorResult.observablesnow returntuple()instead of exposing mutable internal lists directlypython/bloqade/lanes/layout/arch.pypathsis intentionally mutable (populated incrementally after construction)python/tests/visualize/test_artist.pyMappingProxyTypeinstead ofdictTest plan
Related
🤖 Generated with Claude Code