feat: comprehensive cleanup, 92-test suite, and full roadmap implementation#1
feat: comprehensive cleanup, 92-test suite, and full roadmap implementation#1JGSphaela wants to merge 19 commits into
Conversation
Housekeeping: - Update .gitignore to block CSV/data files, .claude/, .DS_Store - Remove 8MB of tracked measurement CSV data files from git - Create pyproject.toml with all dependencies (pyvisa, PySide6, pandas, etc.) - Remove empty unused src/config/ package Bug fixes (7 critical bugs): - Fix set_output_format sending 'FMT 11,None' when mode=None - Fix division by zero in two_way_sweep when sweep2_step=1 - Fix read_response() returning None (now raises DeviceNotConnectedError) - Fix setCornerWidget overwriting language label in main_window.py - Remove dead variable in set_auto_zero - Fix z-axis label mismatch in plot3d2.py (Drain_I -> Gate_I) - Fix sweep_delay falsy checks failing when value=0 (now uses is not None) Code quality: - Create custom exception hierarchy (PyVARError, GPIBError, DeviceNotConnectedError, InstrumentError, VoltageLimitError, MeasurementAbortedError) - Replace all print() with logging throughout - Fix pd.concat performance anti-pattern in advance_sweep loops - Fix voltage_measurement_range docstring (said 'current' instead of 'voltage') - Add proper docstrings to all key files File organization: - Rename src/tests/ -> src/measurement/ (measurement orchestration, not unit tests) - Rename debug_tests/ -> examples/ (usage examples, not automated tests) - Fix misleading file comments across renamed files Test suite (92 tests): - tests/test_gpib_communication.py: GPIBCommunication with mocked PyVISA - tests/test_gpib_command_b1500.py: B1500 command building + regression tests - tests/test_gpib_command_model335.py: Model335 temperature queries - tests/test_data_process.py: B1500 response parsing - tests/test_measurement_sweep.py: Sweep logic, progress callbacks, abort mechanism - tests/test_config.py: MeasurementConfig serialization and validation - tests/test_profiles.py: Profile save/load round-trips - tests/test_export.py: CSV export with metadata headers Roadmap implementation: - Safety interlocks: configurable voltage limit, checks both start+stop voltages - Progress callback: callback(current, total, elapsed) on both sweep methods - Abort mechanism: threading.Event flag with MeasurementAbortedError + partial data - Measurement config dataclasses: SweepChannelConfig + MeasurementConfig with to_dict/from_dict serialization and to_advance_sweep_kwargs() - GUI wiring: SweepWidget.get_values/set_values, SweepWindow.get_config/set_config, MeasurementWorker(QThread) for background execution, full start/abort/progress flow - CSV export with metadata: export_with_metadata() writes # comment headers - Measurement profiles: save_profile/load_profile/list_profiles as JSON - Temperature integration: query_kelvin_float, set_setpoint, query_heater_output, TemperatureWorker(QThread) for background polling, temperature_callback on sweeps - Docs: full README rewrite, docs/architecture.md, project roadmap
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f32a2f00b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Create data/ directory before writing backup/error CSVs (P1) - Reject zero-step sweep configs when voltages are set (P2) - Rebuild sweep widgets fully when loading shorter profiles (P2) - Fix console script import path and add setuptools package find config (P2)
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6de863542
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Reject blank/zero step counts in validation (step must be > 0) - Check abort flag inside inner sweep2 loop in three_way_sweep - Use separate aborted_with_data signal instead of finished for partial aborts
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f1fb26f15
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Include loop number in HTR? query so both outputs can be sampled - Reject duplicate SweepChannelConfig.channel values in validation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e65c21dd09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Use try/except import so both 'python src/main.py' and 'pyvar3' work - Set MAX_SWEEP_CHANNELS to 3 to match validation limit
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09d5565845
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nels - main.py: insert repo root into sys.path so src.* imports work from direct execution - config.py: reject configs with fewer than 2 sweep channels - sweep_window.py: clear add_sweep_button reference after deleteLater()
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f5c5ea284
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…te stale buttons - Tag partial abort results with sweep3 voltage and temperature columns - Store loaded profile's output_file/metadata in get_config() - Delete old Add Channel button in _rebuild_sweep_widgets before creating new one
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac9dcc2078
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
on_aborted_with_data now saves partial DataFrames to the loaded profile's output_file, matching on_finished behavior.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3312e6273d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
on_finished and on_aborted_with_data now open a QFileDialog when output_file is None, ensuring measurement results are never silently lost.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e0b03836f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…iting - on_finished/on_aborted_with_data read config from worker, not live UI - os.makedirs before to_csv prevents OSError on fresh checkouts
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf9ce9efb8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Empty I Comp field now defaults to 0.1A instead of omitting compliance - Renamed MeasurementWorker.finished to result_ready to avoid shadowing QThread's built-in finished signal
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b09f7222bf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
After MeasurementAbortedError, call reset_channel() on the B1500 to send DZ and disable all SMUs, preventing devices from being left biased at the last sweep point.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 211d5a710a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- on_finished and on_aborted_with_data now use export_with_metadata to preserve sweep config and metadata in saved CSV files - Added pyqt optional dependency group for legacy PyQt6 GUI
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2800a59d0e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Apply the same SMU reset_channel() cleanup from the abort handler to the PyVARError and generic Exception handlers, so a failed measurement never leaves the device under test biased.
JGSphaela
force-pushed
the
claude/hardcore-wiles-7cee23
branch
from
5d00248 to
a310939
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a310939afa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b4f9fc8b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ce8b5cde9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f3bb02679
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e32d74d153
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def create_plot(csv_path: str) -> str: | ||
| """Create the Plotly HTML for a measurement CSV file.""" | ||
| fig = plot_interactive_currents(csv_path) |
There was a problem hiding this comment.
When the PyQt GUI's Show 3D Plot opens a CSV produced by the new GUI/export path, the columns are the raw instrument names emitted by DataProcess/AdvanceTest (for example A_I, A_V, B_V, C_V), not the legacy Gate_V/Sub_V/Drain_V names required by plot_interactive_currents defaults. This unparameterized call therefore raises the missing-columns ValueError for PyVAR3's own measurement files; derive/pass a mapping from the CSV/config or accept raw channel columns.
Useful? React with 👍 / 👎.
|
|
||
| from __future__ import annotations | ||
|
|
||
| import tomllib |
There was a problem hiding this comment.
The project still declares Python 3.10 support in pyproject.toml, but tomllib is only in the standard library starting with Python 3.11. In a supported 3.10 dev environment, this new test fails during collection before any tests run, so use a tomli fallback/dev dependency or raise the supported Python version.
Useful? React with 👍 / 👎.
1 participant
What this PR does
A comprehensive overhaul of PyVAR3 — fixing bugs, adding tests, reorganizing files, and implementing the entire roadmap of features.
Changes
Housekeeping
.gitignorenow blocks CSV/data files,.claude/,.DS_Storepyproject.tomlwith all dependenciessrc/config/packageBug Fixes (7 critical)
set_output_format(mode=None)sent invalid GPIB commandread_response()returned None — callers crashed with AttributeErrorsetCornerWidgetcalled twice — language label overwrittenset_auto_zerosweep_delayfalsy checks failed when value=0Code Quality
File Organization
src/tests/renamed tosrc/measurement/(measurement orchestration, not unit tests)debug_tests/renamed toexamples/(usage examples)Test Suite (92 tests, all passing)
Roadmap Implementation
Safety Interlocks
Progress Callback + Abort
Measurement Configuration
GUI Wiring
CSV Export with Metadata
Measurement Profiles
Temperature Integration
Documentation
Testing
Run:
pytest tests/ -v— 92 tests, all passing