fix: notebook errata pass -- chap2 through app.ipynb all clean - #45
Open
petercorke wants to merge 14 commits into
Open
fix: notebook errata pass -- chap2 through app.ipynb all clean#45petercorke wants to merge 14 commits into
petercorke wants to merge 14 commits into
Conversation
These were never actually tracked in git -- they only "worked" because they sat as untracked files in local working directories, invisible under the old blanket `*test*` .gitignore rule removed in #40. A clean checkout doesn't have them, which breaks: - rvctool --test's bdsim check (imports vloop_test directly) - figures/code/chapter9/fig9_13.py, fig9_14.py (import ploop_test) - notebooks/scripts/chap9.py (%run -m ploop_test -H)
RVC3/examples/imu_data.py had been reconstructed from ex_tumble.py
(a different, incompatible script) rather than the canonical source
already tracked at figures/code/chapter3/imu_data.py, leaving it with
a tumble() function/single-namedtuple shape that doesn't match what
chap3.ipynb actually calls (IMU() -> (true, imu) two-namedtuple pair).
Copied the correct implementation over and fixed its own stale
__main__ self-test block, which still referenced the old tumble()
name and field layout.
Also fixes an unrelated pre-existing syntax error in
figures/code/chapter3/fig3_17.py ("from rvcprint from RVC3.tools
import rvcprint", present since the file's original commit).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
bdsim's VehiclePlot block dropped the boolean trail= option in favour of path= (a linestyle string/dict), matching driveconfig.py and drivepoint.py which already use path="b:". braitenberg.py was still passing the old trail=True, which errored with "unexpected keyword arguments ['trail']". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The AttributeError (spatialmath.base.plot_poly no longer exists, renamed to plot_polygon) is fixed upstream: MVTB's esttheta() was rewritten in petercorke/machinevision-toolbox-python#82 to use spatialmath.Polygon2 directly, already merged to MVTB main. What's left is structural, not a bug: esttheta() calls plt.ginput() to let the user click a region on the displayed image. Under the headless Agg backend used for automated testing there's no display to click on, so ginput() returns zero points and Polygon2() gets an empty array. Same category as the neighbouring markdown-flagged ginput() cell and the swift/PointCloud entries already in this file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…frame limit
The %run -m visodom cell crashed on cv2.findFundamentalMat's raw
cv2.error (fixed upstream, machinevisiontoolbox-python#101), which
masked two further bugs only reachable once that run completes:
- visodom.py's loop reused the variable names left/right, which
%run -m leaks into the notebook's own namespace -- clobbering the
FileArchive objects the notebook created earlier (left = FileArchive
("bridge-l.zip", ...) at cell 207) with the loop's last Image
object. Renamed the loop variables to left_img/right_img throughout.
- ts = np.loadtxt(left.open("timestamps.dat")) never reliably worked:
timestamps.dat isn't a member of bridge-l.zip, it's a separate file
in the mvtb-data package (mvtbdata/images/timestamps.dat).
left.open() only appeared to succeed when that filename happened to
exist in cwd (FileArchive.open() falls back to a plain local read).
Switched to mvtb_path_to_datafile("images", "timestamps.dat").
- Processing all 251 frames takes several minutes, so added an
optional frame-count CLI arg to visodom.py (all frames if omitted)
and the notebook now passes 50.
Documented both notebook-visible changes in errata.md (the cv2.error
fix needed no notebook change, so it's not listed there per the
file's own scope).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
FileArchive.open(name) reads directly from the archive by exact name,
bypassing the filter used for iteration entirely -- no cwd fallback,
contrary to what the previous commit's errata entry claimed.
left.open("timestamps.dat") works correctly once left is the real,
un-clobbered FileArchive object, which it now is now that visodom.py
no longer reuses the left/right variable names. The namespace
clobbering was the entire bug; the book's original line needed no
change. Reverting the cell to the as-printed code and dropping the
incorrect errata entry.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
bdsim now supports BDSIM_NO_GRAPHICS, an unconditional override that disables graphics/animation/movies regardless of what a script's own BDSim(...) constructor call requests -- unlike the BDSIM envariable (lowest priority, loses to explicit constructor kwargs like animation=True and raises "cannot enable animation but disable graphics" instead of silently downgrading). Confirmed this resolves the real slow-render cases from earlier in this branch's history (chap5's braitenberg.py, chap7's jointspace.py, chap9's zerotorque.py/computed-torque-main.py) without the BDSIM=graphics=0 regression that broke scripts passing animation=True explicitly (previously reverted in this same branch). Does not fix chap8's RRMC2.py hang, which reproduces identically with or without this set -- that one was never about render time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…xtures' into fix/notebook-errata
bdsim's run() used to store each watch=[...] entry as its own out.y0, out.y1, ... array. A refactor replaced this with a single combined out.y (np.column_stack of every watched signal) plus out.ynames. feedforward-main.py only watches one signal, so out.y is exactly what out.y0 used to be -- confirmed same shape (254, 6). Found the same old .y0/.y1/... pattern in 10 figures/code/chapter* scripts, none of which are exercised by any notebook (not in scope for this notebook-errata pass) -- flagged separately for a follow-up sweep. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
….x -> clock0.X The AttributeError: y2 is fixed upstream: bdsim's watch=True SCOPE registration silently no-op'd when graphics was disabled, fixed in petercorke/bdsim#75 (promoted watch to GraphicsBlock.__init__) plus a follow-up deprecation shim for the old per-index out.y0/out.y1/... attribute access, so out.y2 still works today with a DeprecationWarning instead of a hard failure. What needed an actual notebook change: out.clock0.x -> out.clock0.X, bdsim's per-clock logged state array uses the uppercase field name. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…name These .bd files (JSON block-diagram definitions loaded by bdload()) still used bdsim's old FUNCTION block parameter names args/kwargs, renamed to fargs/fkwargs. bdload() passes every "parameters" entry straight through as a constructor kwarg, so the stale names hit Function.__init__()'s **blockargs catch-all and errored with "unexpected keyword arguments ['args']" / "['kwargs']". IBVS-holonomic.bd: 1 FUNCTION block (args). IBVS-nonholonomic.bd: 5 FUNCTION blocks (args), 3 of which also had kwargs. Left FKINE blocks' own args= parameter alone in the other .bd files (IBVS-arm.bd, RRMC2.bd, jointspace.bd) -- that's a different, still-valid parameter on a different block type. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same class of stale .bd parameter as the holonomic/nonholonomic fix: this file had maxw/minw, but MultiRotorMixer.__init__() (RTB's blocks/uav.py) accepts wmax/wmin. bdload() passes "parameters" entries straight through as constructor kwargs, so the stale names hit Block.__init__()'s **blockargs catch-all and errored with "unexpected keyword arguments ['maxw', 'minw']". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… commands
RTB moved these from standalone scripts to roboticstoolbox.demo.*
modules exposed as installed console commands ([project.scripts]
entry points), so %run -m <name> can no longer find them as bare
top-level modules ('<name>' is not a valid modulename on sys.path).
Invoke them as the installed commands directly instead (!<name>).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…, drop cell numbers from output - Makefile: `make test` runs the fast suite (plain pytest), `make testall` runs the full suite including notebooks/examples (pytest --runall). Kept the fast/slow split -- BDSIM_NO_GRAPHICS only removes bdsim's own render-time slowness, most of --runall's time is unrelated (kernel/cell overhead across 16 notebooks, vision and deep-learning chapters, visodom's bundle adjustment loop). - notebook_runner.run_notebook(): moved the MPLBACKEND/ BDSIM_NO_GRAPHICS os.environ.setdefault() calls here from main(), since tests/test_notebooks.py (the actual pytest --runall path) calls run_notebook() directly and was bypassing main() entirely -- neither var was being set under `make testall`, only when running notebook_runner.py as a CLI script directly. - Dropped cell[N] from both the pytest failure detail and the written report -- not meaningful to a reader, Jupyter doesn't number cells. Report now leads with a concise per-notebook pass/fail summary instead of requiring a scroll through cell-by-cell detail to see which notebooks are clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Aug 18, 2026
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
Went through all 16 chapter notebooks +
app.ipynberror by error against current RTB/MVTB/SMTB/bdsim releases. Every notebook now runs clean end to end.imu_data.pyrestored from the wrong source,IMU()API now matches what the notebook actually callsbraitenberg.py'sVEHICLEPLOT(trail=...)kwarg no longer exists, usepath=vloop_test/ploop_test.py/.bdfixtures),feedforward-main.py'sout.y0→out.y(bdsim API change)esttheta()'sginput()cell (structurally can't run headless -- needs a real display to click on)visodom.pywas clobbering the notebook's ownleft/rightvariables via%run -inamespace sharing, plus a frame-count limit so the bridge-dataset loop doesn't take several minutesout.clock0.x→out.clock0.X(bdsim field rename);out.y2fixed upstream (bdsim#75).bdfiles had staleFUNCTION/MultiRotorMixerparameter names (args/kwargs→fargs/fkwargs,maxw/minw→wmax/wmin)tripleangledemo/twistdemo/eigdemoare now installed CLI commands in RTB, not standalone scripts --%run -m X→!XBDSIM_NO_GRAPHICS=1wired into both the CLI runner and thepytest --runallpath; dropped cell numbers from failure output/reports in favour of a concise per-notebook pass/fail summaryerrata.mdupdated throughout with what changed and whySeveral fixes landed upstream rather than here -- see
errata.md's scope note. Related upstream PRs: machinevision-toolbox-python#82 (merged) and #101, bdsim#75, robotics-toolbox-python#613.Test plan
python tests/notebook_runner.py-- all 16 chapter notebooks +app.ipynbclean (0 errors)make testall(pytest --runall) passes