Repository navigation
Conversation
Bare except: pass means any exception during the importlib.metadata lookup gets silently swallowed (not just PackageNotFoundError for "not installed"), and __version__ never gets set at all in that case -- spatialmath.__version__ raises AttributeError instead of giving something to print. Narrow the except to PackageNotFoundError and fall back to an explicit "unknown" so the attribute always exists.
trlog's general-case branch divides by sin(theta), computed from trace(R) via acos. The only existing test used R = np.eye(3) exactly, never a numerically near-identity matrix as produced by real computation -- the case #63 (unmerged) reported hitting a divide-by-zero on. Confirmed against the pre-a9fc08a code (rework code to be more robust to nearly identity rotation matrix) that this exact input raised FloatingPointError there; current code's crisp `st == 0` guard handles it cleanly.
…one file spatialmath/base/types.py dispatched on sys.version_info across _types_35.py, _types_39.py and _types_311.py, from when Python 3.5-3.10 support coexisted. pyproject.toml has required Python >=3.10 since #186, so _types_35.py could never be reached, and _types_39/_types_311 differed only in where Self is imported from. Collapse to a single file with a two-line version check; no change to the names or aliases it exports.
np.linalg.det/norm and np.eye carry large dispatch overhead relative to the actual work for 2x2/3x3 matrices, dominating SE3/SO3 constructor cost when check=True. Replace with explicit cofactor- expansion determinants and a squared Frobenius residual for the 2x2 and 3x3 cases (falling back to the generic path otherwise), and replace the bottom-row `all(... == np.array(...))` checks in ishom/ishom2 with direct scalar comparisons. ~2x faster isR, ~30-40% faster SE3(T, check=True) construction. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both functions built small, fixed-size (3x3/4x4/6x6) results out of generic NumPy helpers (np.cross/unitvec/np.stack in trnorm, np.block in tr2adjoint) whose dispatch overhead - built for arbitrary shapes and broadcasting - dominates cost at this size. Same pattern as the isR/ishom speedup in #213. Replaced with explicit scalar arithmetic (trnorm) and direct pre-allocated slice-assignment (tr2adjoint), dtype preserved via np.zeros(..., dtype=T.dtype) so tr2adjoint's documented SymPy support is unaffected. ~12x faster trnorm, ~2.7x faster tr2adjoint standalone. End to end: SE3 @ SE3 (which normalizes via trnorm) drops from ~30us to ~4us. Verified bit-for-bit numeric equivalence against the prior implementation over 200 random SO(3)/SE(3) trials, plus symbolic (SymPy dtype=object) equivalence for tr2adjoint. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both used np.dot/np.cross on 3-vectors, whose generic dispatch overhead - built for arbitrary shapes/broadcasting - dominates cost at this size, same pattern as isR/trnorm/tr2adjoint (#213, #214). qqmul: replaced with explicit scalar Hamilton-product arithmetic, bit-identical to the prior result (verified to ~1 ULP over 2000 random trials). qvmul: replaced the q * pure(v) * conj(q) sandwich (two full Hamilton products, each wasting work on a zero scalar part, via qqmul/qpure/ qconj with their own getvector re-validation) with the closed-form rotation identity v' = v + 2s(w x v) + 2 w x (w x v) for q = (s, w). This is a different, well-known equivalent formula rather than a re-expression of the same one, so it is not bit-identical - verified to ~1e-14 absolute over 2000 random trials (v scaled to magnitude ~10), well within floating-point noise. qqmul ~10x faster (15.0us -> 1.4us isolated), qvmul ~24x faster (33.4us -> 1.4us isolated). End to end: Q1 * v drops from ~33us to ~4us (~9x), Q1 * Q2 from ~19us to ~10us (~2x - qqmul itself is no longer the bottleneck there; the remainder is UnitQuaternion construction overhead, in particular qunit()'s np.linalg.norm/np.r_ calls, which is a separate, un-addressed candidate for a future fix in the same spirit). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…/rodrigues trexp/trlog don't have one dominant wasteful generic call like the isR/trnorm/tr2adjoint/qqmul/qvmul fixes did (#213/#214/#215) - they're deep call chains (trexp -> isskewa -> vexa -> iszerovec -> unittwist_norm -> rodrigues -> skew -> rt2tr -> ishom -> isR) where cost is spread thin across many small layers. Two targeted, verified wins pulled out of that chain: - isskewa (validity check trexp runs on se(3) input) had the same bug ishom had pre-#213: np.linalg.norm on a small fixed matrix, plus an all(S[-1,:] == 0) array-allocation-and-compare for the bottom row. Fixed the same way, ~1.9x faster standalone. - np.eye(3) was rebuilt from scratch on every call in rodrigues (1x), trexp's V-matrix construction (1x), and trlog (2x), despite only ever being used as a read-only operand in an addition/multiplication that produces a new array. Replaced with a module-level constant _EYE3, used only at call sites where it's provably never mutated or returned directly (an aliasing hazard if it were) - the four zero-motion/zero-rotation early-return `np.eye(N)` calls in rodrigues/trexp are untouched, left as fresh arrays, since they hand the object directly to the caller. isskew (the plain, non-augmented so(n) check) was also tried with the same explicit-arithmetic treatment as isskewa, but did NOT show a reliable win under min-of-repeats benchmarking - the only cost it avoids is np.linalg.norm on an already-cheap `S + S.T`, not enough margin to reliably beat by hand-unrolling. Reverted to the original implementation rather than keep an unproven "fix". Net effect, measured old-vs-new in the same process (min of 9 repeats each, to suppress system jitter after an earlier round of misleading single-run numbers): isskewa ~1.9x, rodrigues ~1.13x, trexp ~1.05x, trlog ~1.10x. Real but modest - see the PR description's "further speedup opportunities" section for what a bigger win here would actually require. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sphinx-pyrunblock is Peter's own package (published to PyPI, hard fork of the original WhyNotHugo/sphinx-autorun), reachable at the same content today via git+https://github.com/petercorke/sphinx-autorun.git (an old, pre-rename URL that still redirects). Installing the PyPI wheel directly is ~2-4x faster in CI (no git clone/VCS resolution) and makes pyproject.toml's declared dependency honest -- previously "sphinx-autorun" there would, in isolation, resolve to the unrelated upstream PyPI package of that name rather than Peter's fork. No functional change: both paths resolve to the same sphinx-pyrunblock 1.1.0 release. Verified with a real sphinx-build against a clean venv.
sphinx-codeautolink's own parsing pass over code blocks is stricter than plain docutils about this pre-existing 3-space/4-space mismatch between the :linenos: option and the code content, surfacing a new 'unexpected indent' warning that was previously silently tolerated.
…he root cause they shared Found via the sphinx-pyrunblock rollout (PR #221): several `.. runblock::` docstring examples were silently baking broken tracebacks into the published docs instead of demonstrating real output. ## Twist2.unit() / Twist3.unit() Branches were swapped (comments didn't match what the code under them did), `self.w` had a `S.w` typo (undefined name), and the "prismatic" branch passed a wrong-shape zero argument for Twist2 (3-element list where a scalar is required). Fixed the branch logic, and use `abs()` instead of `smb.norm()` for Twist2's scalar `w`. ## Twist2.pole The docstring's own example called `S.pole()`, but `pole` is a `@property`, not a method -- `TypeError: 'numpy.ndarray' object is not callable`. (Twist3.pole's own example was already correct; only its prose incorrectly said `X.pole()` too.) ## DualQuaternion: a shared root cause across three methods Fixing `DualQuaternion.SE3()`'s docstring (called `d.T` instead of the actual `d.SE3()` method) surfaced a real bug: `UnitQuaternion.conj()` silently re-canonicalized its result's sign (forces scalar part >= 0), which is correct for constructing a `UnitQuaternion` from arbitrary data (q and -q are the same rotation) but wrong for conjugation, an algebraic operation that must satisfy q*conj(q) == 1 for downstream algebra to be correct. Fixed `Quaternion.conj()` itself (spatialmath/quaternion.py) rather than routing around it at each call site -- this is what "conjugate" should mean regardless of caller. No existing test asserted the old (re-canonicalizing) behaviour, only the return type. This one fix resolved three separate, previously-untested DualQuaternion bugs at once: - `SE3()`: silently returned a transform with negated translation for any rotation with negative quaternion scalar part. - `norm()`: crashed outright (`math domain error`) for the same reason -- also added a small floating-point clamp, since the norm-squared terms are mathematically non-negative but rounding can leave e.g. -1e-17 instead of exactly 0. - Vector transformation (`dq * v`): found while re-verifying `norm()`'s fix -- unrelated to the conj() sign bug, this affected *every* case regardless of sign. The textbook q*P*conj(q) sandwich product's translation terms cancel to exactly zero under this class's own dual-part embedding convention (`__init__` builds `dual = 0.5*Pure(t)*real`, translation on the left) -- silently applying only the rotation and dropping translation entirely. Since `SE3()` already correctly extracts (R, t) from this same embedding, reused it instead of hand-deriving a second, convention-specific sandwich formula. ## Test coverage All of the above had zero prior test coverage (existing DualQuaternion tests used SE3.Rx(pi/4) -- no translation, positive quaternion scalar part -- which can't exercise any of these bugs). Added regression tests for conj()'s sign behaviour (including a mixed-sign multi-valued case), both branches of Twist2/Twist3.unit(), Twist2/Twist3.pole, and DualQuaternion's SE3()/norm()/vector-transform with both a negative-quaternion-scalar case and a general case. Full suite: 334 passed (up from 326), 0 regressions. Real sphinx-build verification (isolated venv, before/after): RUNBLOCK-ERROR count 6 -> 0, warning count unchanged at 14 (confirmed stable across repeat builds of both branches, ruling out build-to-build nondeterminism).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…oilerplate Reworks the ad hoc, partly-commented-out timing script (which had been manually re-enabled with a couple of tweaks, uncommitted, sitting in the working tree across several perf PRs this session) into something that's actually a useful reference: - Split into separate tables, one per category (SO(3)/SE(3) base functions, SE3 class, quaternion base functions, UnitQuaternion class, twist/exponential-map base functions, Twist3 class, NumPy baseline), printed with a plain header between each. Base-level (module functions on raw arrays) and class-level (constructors/ operators/properties on instances) are now consistently separated throughout, rather than mixed together - the old Twist3 section in particular had base functions (skew, trlog, trexp, rodrigues, ...) interleaved with class-level operations. - Fixed a corrupted docstring header left over from a bad find/replace on upstream (`# -*- coding", t)` / `@author", t)`), unrelated to any of this but noticed while rewriting the file. - Fixed mislabelled/ambiguous rows: `inner()` -> `np.inner(s, s).sum()`, bare `cross()` -> `base.cross(a, b)` (paired explicitly against `np.cross(a, b)`), and rows comparing decomposed-(R,t) vs full-4x4 composition that had identical labels for different code paths. - Added coverage that didn't exist before: SE3 <-> RPY, SE3 <-> Euler, SE3 -> UnitQuaternion, UnitQuaternion <-> rotation matrix, UnitQuaternion -> SE3, and SE3 @ SE3 (normalized compose) alongside SE3 * SE3. - Collapsed the `timeit.timeit(...); result(...)` two-line-per-test boilerplate into a single `timeit(stmt, label, setup)` call (module import aliased to `_timeit` to free up the name). Replaced the single `timeit.timeit(..., number=N)` run per test with `min(timeit.repeat(..., number=N, repeat=REPEATS))` - a single long run doesn't distinguish real cost from a GC pause or OS scheduling hiccup, min-of-repeats does. N dropped from 100_000 to 10_000 (an arbitrary starting point either way) since the repeats now do the work of suppressing noise that a bigger N was being used for. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The script is a dev tool, not library API; move it out of the installed package so it no longer ships in the wheel. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CPU, OS, Python, numpy and spatialmath versions and the timing settings, so a pasted table is self-describing. Modeled on RTB's rne_speed.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both trplot() and trplot2() ended with a bare **kwargs that was never read anywhere in their bodies, so a typo'd keyword (e.g. framelabel= instead of frame=) was silently accepted and ignored instead of raising -- it drew the frame with no error and no label at all. Removing the dead sink lets Python's own unexpected-keyword-argument check do its job. While rewriting the two places that used to lean on this catch-all: - trplot's anaglyph branch only forwarded a small fixed subset of parameters to each eye's recursive call, silently dropping textcolor, labels, originsize, origincolor, axislabel, axissubscript, width and projection; it also never returned ax and never actually called plt.show(block=...), so anaglyph mode ignored block entirely. - trplot's "T is an iterable of transforms" branch forwarded every parameter except axissubscript, and also never returned ax. - tranimate/tranimate2 were splatting the same unfiltered kwargs dict into Animate()/Animate2()'s constructor, the drawing call, and run()'s animation-control call, relying on each one's own dead **kwargs sink to drop what it didn't need. Once trplot/trplot2 validate strictly, run()-only parameters (movie, repeat, interval, nframes, wait) needed to be split off before the drawing call. Also fixes a long-standing typo in test_pose2d.py::test_graphics (T0=T2, which was never a real parameter -- animate()'s documented one is start=) that this same silent-swallow bug had been quietly masking. Adds tests/base/test_animate.py: constructing a FuncAnimation under a non-interactive backend doesn't actually run its per-frame update() callback (confirmed directly -- Animate2's frame counter never advances past its initial value), so existing animate() tests only proved construction didn't raise. These tests force every frame through the real callback via FuncAnimation.save() (PillowWriter, no external ffmpeg dependency) and assert trinterp/trinterp2 actually ran across a real s: 0->1 sweep. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Normalize line directions and moments for parallel and intersection checks. Construct common perpendiculars from the nearest point and unit direction cross product, with regression coverage across Plucker scalings. Assisted-by: OpenAI Codex (GPT-6 Sol) Signed-off-by: 李永祺 <doribelove@gmail.com>
Keep the near-parallel tolerance test focused on direction vectors by constructing both lines through the origin. This avoids an unrelated Plucker orthogonality check failure on NumPy 2. Assisted-by: OpenAI Codex (GPT-6 Sol) Signed-off-by: 李永祺 <doribelove@gmail.com>
e2h/h2e had one assertion each (N-vector only), with the matrix path and h2e scaling only exercised indirectly via the pose tests. Add tests for vector input types, matrix input, per-column scaling, round trips, bad types, and pin the current (1,N)-is-a-matrix interpretation. Docstrings: e2h's :seealso: pointed at itself (now h2e); document the (1,N) interpretation in both. No behavior change. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
h2e of a (1,N) array, a length-1 vector or a scalar returned an empty (0,N) array, which is never useful (a homogeneous point has at least two elements). Raise ValueError instead. Docstrings for e2h and h2e now state the contract up front: columns are points, rows never are, with explicit shapes, the (1,N) consequence and :raises: entries. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Refactor test_animate_nonidentity_start to improve axis geometry validation and streamline assertions.
Added support for stacked numpy arrays in baseposelist.py.
… error Added support for SE3 constructor with (N, 4, 4) NumPy array.
Add unit test for SE3 constructor with stacked array input.
Add a test to ensure scipy is not loaded when importing spatialmath.
…t unset on failure
…type shim into one file
…trlog/rodrigues
…tead of git+https
…aternion (and their shared root cause)
…s, document contract
…ses (SE3, SO3, SE2, SO2)
taughz
added this pull request to stack #247
October 11, 2026 16:04
This was referenced Oct 11, 2026
Draft
This branch has not been deployed
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.
Do not merge this PR. It is a hub for integration testing the open PR backlog against Core (internal RAI software). The branch stages the PRs listed below, one
--no-ffmerge commit per PR in merge order, so they can be tested together. Once Core tests pass, the individual PRs are approved and merged tomasterin this order (ticking them off here), and this hub is closed.The branch is rebuilt (force-pushed) whenever a contained PR changes or lands on
master. Do not usegh stack sync,gh stack rebase, orgh stack mergeon this stack: rebasing flattens the per-PR merge commits, and merging would land the hub itself.Stack: #245 (high priority) -> this PR (backlog). This PR's diff only shows the backlog PRs; it will be retargeted to
masteronce #245's PRs have all merged.Core tested: no
Contained PRs (merge order)
Roughly oldest first, with related PRs kept adjacent. Simulated merges show no textual conflicts in this order.
Not included yet
Waiting on a rebase onto
master(they conflict with #193, #196, #207); they will be appended here once rebased:Deferred (decision pending):
Proposed for closure: