Clean up what the last audit turned over: two switches, six stale commands, and four crowded figures - #94
Merged
Merged
Conversation
PR #80 made "does running this open a window" a single decision: core.eval.show_figures_if_requested reads IPIN_SHOW_FIGURES and is off by default. Two examples kept a --no-show flag from before that, gating the new helper: if not args.no_show: show_figures_if_requested() else: plt.close("all") Which means the reader has two switches for one behaviour, and the flag only does anything in the case where they set the environment variable and then asked for the window to be suppressed anyway. That is the exact shape #80 removed everywhere else, left behind in the two files it did not touch. The flag is gone from ch2_coords.example_attitude_visualization and ch7_slam.example_scan_matching_visualization, along with the branch, the two tests that passed it, and the ch7 README line that documented it. Also fixed on the way: ch2's module docstring told the reader to run python ch2_coords/example_attitude_visualization.py which is the script form -- the one that fails on a clone that has not been pip-installed. tests/docs/test_documented_commands_use_module_form.py reads .md and .ipynb, not .py docstrings, and since every example now passes description=__doc__ to argparse, that line is printed by --help. A reader-facing command in a place no guard looks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit fixed one script-form command in a docstring and said ch2
was the only place. It was not: `git grep` found **six more**, all in Chapter 3.
ch3_estimators/example_comparison.py
ch3_estimators/example_ekf_range_bearing.py (2)
ch3_estimators/example_iekf_range_bearing.py
ch3_estimators/example_kalman_1d.py
ch3_estimators/example_least_squares.py
`python ch3_estimators/example_least_squares.py` is the invocation that fails
with ModuleNotFoundError on a clone that has not been pip-installed. That is
the whole reason the repository standardised on `python -m`, and these six
survived the sweep because tests/docs/test_documented_commands_use_module_form
read `.md` and `.ipynb` only.
**A `.py` file was reasonable to exclude before #80 and is not now.** Every
example passes `description=__doc__` to argparse, so a module docstring is
printed verbatim by `--help` -- it is reader documentation whatever extension
it lives under. The guard reads `.py` too.
Three files quote the script form deliberately, as the failing case, and are
exempt with that stated: CLAUDE.md, this guard, and
tests/test_examples_import_this_checkout.py, which exists because of the same
trap.
Mutation-tested: restoring one script-form line to a Chapter 3 docstring names
the file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The four figures Chapter 2 started showing last week are the right pictures and were hard to look at. Matplotlib's default 3-D axes wrap a unit triad -- a few centimetres across on the page -- in three filled grey panes and a full grid, which between them take most of the area and none of the attention. On the gimbal-lock figure, eight panels of grey box surrounded eight small triads. There are no numbers on these axes to read off. Every panel says "this arrow moved, that one did not", so the scaffolding is turned down until the arrows are the subject: panes unfilled, their edges and the axis lines at 0.85 and 0.75 grey, grid alpha 0.15, tick marks removed as well as tick labels. One helper, `_style_3d`, styles every panel in the file, so all four figures change together -- which is the argument for it having been a helper. Opened all four PNGs before committing, as CLAUDE.md requires: the frame chain, the Euler convention, the transpose trap and gimbal lock. The transpose figure is the one that gains most, because the whole lesson is that the red x' points somewhere different in the two panels, and that is now the first thing you see. Chapter 8's lc_tc_comparison is deliberately left alone. Nine panels is a lot, but it sits a third of the way down the chapter as the detailed comparison rather than as an opener, and the panels earn their place there. Co-Authored-By: Claude Opus 5 <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.
Follow-up to #93. Three problems that surfaced while doing that work, each
fixed and each guarded where a guard applies.
1. Two mechanisms decided whether a window opens
#80 made that one decision:
core.eval.show_figures_if_requestedreadsIPIN_SHOW_FIGURESand is off by default. Two examples kept a--no-showflagfrom before it, gating the new helper:
So the reader had two switches for one behaviour, and the flag only did
anything if they set the environment variable and then asked for the window
to be suppressed anyway. Exactly the shape #80 removed everywhere else, left in
the two files it did not touch. Gone from
ch2_coords.example_attitude_visualizationandch7_slam.example_scan_matching_visualization, with the branch, the two teststhat passed it, and the ch7 README line.
2. A module docstring is reader documentation now — six commands had not caught up
Fixing ch2's docstring, I said it was the only one.
git grepsaid otherwise:six more, all in Chapter 3.
python ch3_estimators/example_least_squares.pyis the invocation that failswith
ModuleNotFoundErroron a clone that has not been pip-installed — thewhole reason the repo standardised on
python -m. They survived becausetest_documented_commands_use_module_formread.mdand.ipynbonly.Excluding
.pywas reasonable before #80 and is not now. Every examplepasses
description=__doc__to argparse, so a module docstring is printedverbatim by
--help. The guard reads.pytoo; three files that quote thescript form as the failing case are exempt with that stated. Mutation-tested.
3. Chapter 2's figures were 85% scaffolding
The four figures #93 surfaced are good, and matplotlib's default 3-D axes
surround each unit triad with three filled grey panes and a full grid that take
most of the area and none of the attention. There are no numbers to read off
these axes — the panels say "this arrow moved, that one did not".
Panes unfilled, edges and axis lines at 0.75–0.85 grey, grid at alpha 0.15,
tick marks removed. One shared
_style_3dhelper, so all four improve at once,and the triads are now what you see. Each was regenerated and opened before
being committed.
ch8's nine-panellc_tc_comparisonwas considered and deliberately left: itappears a third of the way down, not on the first screen, and it is the
chapter's core comparison rather than an attention-grabber. Redesigning it is a
different change with a much less clear payoff.
Verification
Full suite running locally; every README transcript and every committed figure
gate must hold, and the twelve regenerated Chapter 2 files are the only figure
bytes that move.
🤖 Generated with Claude Code