Repository navigation
test+fix: direct tests for e2h/h2e, h2e rejects <2 rows, document contract - #240
Open
petercorke wants to merge 2 commits into
Open
petercorke wants to merge 2 commits into
petercorke wants to merge 2 commits into
Conversation
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>
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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>
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.
e2handh2ehad a single assertion each (N-vector input only). The matrix path andh2e's per-column scaling were only exercised indirectly through the pose tests, and the shape contract for 2D input was undocumented.Tests (
tests/base/test_transformsNd.py)h2escaling by each column's own last elementh2e(e2h(x))ande2h(h2e(x))round tripsValueErrorfor bad types(1,N)array is a matrix of N 1-vectors, not a row vector. This documents current behavior rather than endorsing it, so any change to it is deliberate.Behavior change (small)
h2enow raisesValueErrorfor input with fewer than two rows (a(1,N)array, a length-1 vector, or a scalar). Previously it returned an empty(0,N)array, which is never useful: a homogeneous point has at least two elements. Checked callers in MVTB (Camera.py,ImageReshape.py); both pass 3-row arrays.e2his unchanged.Docstrings (
spatialmath/base/transformsNd.py):raises:entries added;e2h's:seealso:pointed at itself, nowh2e.Rank-0 and rank-3 rejection is covered by #239 (
isvector) and is deliberately not duplicated here; this PR is independent of it.Full suite: 355 passed, 3 skipped.
🤖 Generated with Claude Code