Skip to content

fix(geom3d): accept array-like volume bounds - #274

Open
Doribelove wants to merge 2 commits into
rai-opensource:masterfrom
Doribelove:fix/line-volume-arraylike-bounds
Open

Doribelove wants to merge 2 commits into
rai-opensource:masterfrom
Doribelove:fix/line-volume-arraylike-bounds

Conversation

@Doribelove

@Doribelove Doribelove commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Line3.intersect_volume() documents six array-like bounds, but passing a Python list or tuple raises AttributeError at bounds.reshape(...). Row and column NumPy vectors also fail later because the face loop indexes the unflattened bounds.

from spatialmath import Line3

line = Line3.PointDir([0, 1, 2], [1, 0, 0])
line.intersect_volume([-2, 3, -1, 4, 0, 5]).p
# expected columns: [-2, 1, 2] and [3, 1, 2]
# before: AttributeError: 'list' object has no attribute 'reshape'

Normalize the bounds once with base.getvector(bounds, 6), as Line3.plot() already does. This accepts lists, tuples, flat arrays, and row/column vectors, and validates their length before the face loop. Correct the bounds type field and the reshape comment. The intersection algorithm and return structure are unchanged.

Add 21 regression cases for axis-aligned and oblique intersections, a line outside the volume, all five input formats, and invalid lengths. Check intersection coordinates, point-array shape and line parameters.

Validation against upstream 41e0a4d:

  • New regressions on the original runtime: 16 failed, 5 passed. All 21 pass with the fix.
  • Geometry tests on Python 3.12.14 and 3.10.12: 63 passed on each.
  • Full suite on Python 3.12.14: 431 passed, 3 skipped, with 55 subtests passed and 23 warnings.
  • Repository pre-commit hooks, including Black 23.10.0, and git diff --check pass.

Update after upstream 1.1.19

Merged current master (25b19b3) to resolve the overlapping additions in tests/test_geom3d.py. Both the line-intersection regressions merged in #273 and all 21 volume-bounds cases from this PR are retained. The diff against current master remains the bounds normalization and its tests.

Validation of the updated branch:

  • Full suite on Python 3.12.14 with the revised source installed: 587 passed, 3 skipped, with 99 subtests passed.
  • Geometry tests on Python 3.10.12: 95 passed.
  • Repository pre-commit hooks and git diff --check: passed.

Signed-off-by: 李永祺 <doribelove@gmail.com>
Signed-off-by: 李永祺 <doribelove@gmail.com>
@Doribelove

Copy link
Copy Markdown
Contributor Author

Merged current master (25b19b3) to resolve the overlapping test additions after #273 was merged. Both sets of geometry regressions are retained; the diff against master remains the bounds normalization and its 21 regression cases.

Validation: 587 passed / 3 skipped on Python 3.12.14, 95 geometry tests passed on Python 3.10.12, and the repository pre-commit hooks plus git diff --check pass.

@petercorke

Copy link
Copy Markdown
Collaborator

Thank you for your interest in SMTB, and for another careful fix! Routing the bounds through getvector(bounds, 6) matches what Line3.plot() already does, and the regression cases for list, tuple, array, row and column inputs and for invalid lengths are thorough.

1.1.19 is now released, and this PR currently has merge conflicts with master. Could you rebase onto the current master and resolve them? Once that is done I'm happy to approve. I can only approve; we rely on RAI to merge and publish.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants