Trim raytracing_app.py: dead code, element-drawer dispatch, results data spec - #502
Merged
Conversation
- Drop unused `time` and `tkinter.DoubleVar` imports - Drop the commented-out `add_aperture_button` widget block - Drop the commented-out aperture seed record (it referenced fields that aren't in the table schema) - Drop the unreachable `add_aperture_button` branch in click_table_buttons (the button widget doesn't exist; the elif would have AttributeError'd if reached) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
create_optical_path had four near-identical branches (Lens, Aperture, ThickLens, DielectricSlab) that each redrew the aperture top/bottom marks inline. Pull the marks into _draw_aperture_marks, give each element type its own _draw_* helper, and dispatch through a small dict in create_optical_path. ThickLens and DielectricSlab share _draw_thick_element parameterised by the body shape (Oval vs Rectangle). Adding a new element type now means: write one helper, add one entry. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The 13 metrics displayed in the results table each had ~10 lines of inline data_source.append_record() calls with their own has-stop / no-stop branch. Pull them into RESULT_ROWS — a class-level list of (label, getter) tuples — and let calculate_imaging_path_results just walk the spec. Each getter is a self-contained lambda returning the final string, including the 'Inexistent' / 'Infinite [no FS]' fallback for the case where the metric doesn't apply. Adding a metric is now one line. Verified all 15 rows produce the expected output for paths with and without aperture/field stops. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
create_window_widgets used to be ~260 lines of widget construction, property bindings, and observer registration in one method. Split it into a table-of-contents top-level method that calls six helpers: _build_element_table, _build_results_table, _build_controls_panel, _build_canvas, _wire_bindings, _register_observers. Other tidying done in the same pass: - Drop the dead `optics_basis = ...` local that was never used. - Promote `radio_principal` to `self.radio_principal` so the bindings helper can find it. - Collapse the five repeated `is_disabled` bindings into a `for` loop, and the seven `add_observer` calls into another. 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
1 participant
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
Cuts
raytracing_app.pyfrom 1176 → 994 lines (-182) across three reviewable commits, no behavior change.time/tkinter.DoubleVarimports, commented-outadd_aperture_buttonwidget block, commented-out aperture seed record (referenced fields not in the schema), unreachableadd_aperture_buttonbranch inclick_table_buttons.create_optical_pathhad four near-identicalif/elif type(element) is Xbranches that each redrew the aperture top/bottom marks inline. Pull marks into_draw_aperture_marks, give each element type its own_draw_*helper, dispatch through a small dict. ThickLens and DielectricSlab share_draw_thick_elementparameterised by body shape (Oval vs Rectangle). Adding a new element type now means one entry + one helper.calculate_imaging_path_resultshad 13 metrics each written as ~10 lines of paired has-stop / no-stopappend_recordcalls. Pull them into class-levelRESULT_ROWS = [(label, lambda p: ...), ...]and let the method walk the spec. Each lambda is self-contained, including its 'Inexistent' / 'Infinite [no FS]' fallback. Verified all 15 rows render correctly against paths with and without aperture/field stops.Not done in this PR (separate readability decision, no line savings):
create_window_widgetsinto per-section build helpers.Test plan
python -m raytracing -a, confirm default Lens scene renders identically.🤖 Generated with Claude Code