Skip to content

Reject unsupported ndarray ranks in isvector - #239

Open
MokiMeow wants to merge 1 commit into
rai-opensource:masterfrom
MokiMeow:fix-isvector-array-rank
Open

MokiMeow wants to merge 1 commit into
rai-opensource:masterfrom
MokiMeow:fix-isvector-array-rank

Conversation

@MokiMeow

@MokiMeow MokiMeow commented Oct 9, 2026 •

Copy link
Copy Markdown

isvector() accepts higher-rank arrays and crashes on zero-dimensional arrays. Require two dimensions before checking row/column shapes, matching its documented contract and preventing homogeneous conversions from silently flattening unsupported inputs. Restore positive controls hidden by a duplicate test name.

Thirteen regressions fail before the fix. Full suites pass on Windows Python 3.10/3.12/3.14 and Linux Python 3.13: 351 passed, three skipped, 18 subtests passed. Changed-file pre-commit checks and package builds pass.

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@petercorke

Copy link
Copy Markdown
Collaborator

Thank you for your interest in SMTB, and for the careful fix. isvector accepting arrays of three or more dimensions, and raising IndexError on a 0-D array, were real bugs, and the extra len(s) == 2 check is the minimal, correct change. I also appreciate you spotting that a duplicated test_isvector name was silently hiding the first test.

Your e2h/h2e regression test prompted a small follow-up: #240 adds direct tests for those two functions, documents their shape contract (columns are points), and makes h2e raise on input with fewer than two rows. It is independent of this PR.

I've approved this. Merging is up to the RAI maintainers, so it may take a little while. Thanks again!

@petercorke

Copy link
Copy Markdown
Collaborator

A quick note on the failing sphinx / sphinx check: this isn't caused by your change, so please don't worry about it. All of the unit-test jobs and codecov pass.

The docs build itself completes. It fails on its final step, which tries to push the built docs to gh-pages. For pull requests from a fork, GitHub gives the workflow a read-only token, so that push is always rejected with a 403. It would happen to any contributor's PR right now.

The fix is already open as #222, which stops the docs job running on pull requests at all. It's waiting on review by the RAI maintainers. Once that merges, the check will go away, and refreshing this PR (for example with "Update branch") will pick up the fix. There's nothing you need to change on your side.

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.

3 participants