Restructure examples: flatten ported/, drop example_ prefix, parametrize tests#686
Merged
Restructure examples: flatten ported/, drop example_ prefix, parametrize tests#686
Conversation
…ize tests - Move 29 examples from examples/ported/example_*.py to examples/*.py - Drop redundant example_ prefix (files are already in examples/) - Move 22 old subdirectory examples to examples/legacy/ - Delete 4 superseded demo scripts - Rewrite test_example_parity.py: 15 copy-paste classes → table-driven parametrized tests (550 → 290 lines, 35 pass / 2 skip, zero noise skips) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #686 +/- ##
==========================================
- Coverage 74.46% 74.40% -0.07%
==========================================
Files 56 56
Lines 8024 8024
Branches 1569 1569
==========================================
- Hits 5975 5970 -5
- Misses 1432 1436 +4
- Partials 617 618 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Add jupytext.toml and generate-notebooks.yml: auto-generates .ipynb from examples/*.py on push to master, enabling Colab links - Fix run-examples.yml: exclude legacy/, trigger on PRs touching examples - Delete test_example.yml (disabled, superseded by run-examples.yml) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Update all doc/README links from examples/X/ to examples/legacy/X/ (fixes link-check CI failure) - Separate p_thresh and p_final_thresh in test registry: TVSP examples keep machine-precision (5e-13) for test_p, looser (5e-11) for test_p_final (addresses Greptile P2 review finding) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- na_filtering_part_{1,2,3}: cast f_max to float (was array from k_max)
- tvsp_steering_linear_array: infer p_final grid shape instead of
hardcoding 128x128 (PML stripping changes the size)
- ivp_loading_external_image: fix _REPO_ROOT after move from
examples/ported/ to examples/ (one fewer parent dir)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
kgrid.k_max is a multi-element array in 1D grids; np.max() reduces it to a scalar before float() conversion for axvline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Owner
Author
|
@greptile-app |
- Add # %% jupytext cell markers to all 29 examples (imports, setup, run, __main__) so generated notebooks have useful multi-cell structure - Pin jupytext version, add git pull --rebase before push to prevent non-fast-forward race condition (Greptile P1) - Update release plan: v0.6.2 done items, v0.6.3 feature list, simplification targets, real-world validation step Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Owner
Author
|
@greptile-app |
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.
Greptile Summary
This PR cleanly achieves its stated goal: flatten
examples/ported/example_*.py→examples/*.py, move legacy subdirectory examples toexamples/legacy/, and refactor 15 per-class parity test classes into a single parametrizedTestStandardExamples(550 → 290 lines, same coverage). All per-example MATLAB parity thresholds are faithfully preserved.Key changes:
setup() / run() / __main__pattern with# %%jupytext cell markers_EXAMPLESregistry drives table-driven parity checks with per-examplep_threshandpf_thresh_reorder_fullgridand_grid_shape_from_refhelpers generalize F→C reorder logic to N dimensionsgenerate-notebooks.ymlauto-commits jupytext notebooks directly to master (potential branch-protection conflict)TestStandardExamples.scenarioruns the expensive simulation before_load_refcan skipConfidence Score: 5/5
Safe to merge; all findings are non-blocking P2 style and best-practice suggestions with no correctness impact
The structural refactoring is clean and well-executed. Per-example thresholds are correctly preserved. The three flagged issues are all P2: the direct-push-to-master workflow pattern is a process concern, the simulation-before-skip ordering is a performance nit, and the missing MPLBACKEND is a minor robustness gap. None affect physics correctness or test validity.
.github/workflows/generate-notebooks.yml (direct push to master) and tests/test_example_parity.py (simulation ordering in scenario fixture)
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD PR[Pull Request touches\nexamples/*.py] --> RUN[run-examples.yml\ntriggers] PUSH[Push to master\ntouches examples/*.py] --> NB[generate-notebooks.yml\ntriggers] RUN --> DISC[discover-examples job\nfind -maxdepth 1 *.py] DISC --> MAT[Matrix: one job\nper example file] MAT --> EXEC[python example.py\nruns __main__ block] NB --> JTEXT[jupytext --to notebook\nfor each examples/*.py] JTEXT --> COMMIT[git pull --rebase +\ngit push HEAD:master\n⚠️ direct push to master] PARITY[pytest -m matlab_parity\ntest_example_parity.py] --> MOD[importlib.import_module\nexamples.name] MOD --> SIM[_run_with_binary_sensor\nfull k-Wave simulation] SIM --> REF[_load_ref → skip if\n.mat not found\n⚠️ simulation already ran] REF --> ASSERT[_assert_close\np + p_final vs MATLAB]Reviews (3): Last reviewed commit: "Add # %% cell markers, update release pl..." | Re-trigger Greptile