fix: assert hygiene -- unreachable assert, two caller-facing checks - #100
Open
petercorke wants to merge 1 commit into
Open
fix: assert hygiene -- unreachable assert, two caller-facing checks#100petercorke wants to merge 1 commit into
petercorke wants to merge 1 commit into
Conversation
Found while surveying src/'s ~48 Bandit B101 (assert_used) findings; most
are legitimate (type-narrowing after a prior check, or optional-dependency
guards immediately following an ImportError raise) and are staying as-is.
Three were real issues:
- Sources.py (PointCloud publish support): `assert o3d is not None` sat
*inside* the `if not _open3d_available: raise ImportError(...)` block,
after the raise -- unreachable dead code, apparently a mis-indented
copy of the same pattern used correctly at 7 other call sites in this
file. Dedented to match.
- BundleAdjust.py: `add_projection()`'s `assert len(uv) == 2` validates a
caller-supplied argument on a public method, not an internal invariant --
converted to `if len(uv) != 2: raise ValueError(...)` so it survives
`python -O` instead of silently vanishing.
- Camera.py: `nu`/`nv`/`width`/`height` each asserted
`self._imagesize is not None, "imagesize not set"` -- a real "you called
this before configuring the camera" caller mistake (the informative
message was the tell), not an internal invariant. Converted to
`if self._imagesize is None: raise ValueError(...)`, matching the
existing `raise ValueError("imagesize or rho properties not set")`
convention already used elsewhere in this file.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 10 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
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.
Summary
Found while surveying
src/'s ~48 Bandit B101 ("assert used") findings (see #99, which excludestests/from that check but leavessrc/'s real findings active). Most are legitimate -- type-narrowing right after a prior check, or optional-dependency guards immediately following anImportErrorraise -- and are staying as-is. Three were real issues:Sources.py(PointCloud publish support):assert o3d is not Nonesat inside theif not _open3d_available: raise ImportError(...)block, after theraise-- unreachable dead code. Apparently a mis-indented copy of the same pattern used correctly at 7 other call sites in this file. Dedented to match them.BundleAdjust.py:add_projection()'sassert len(uv) == 2, "uv must be a 2-vector"validates a caller-supplied argument on a public method, not an internal invariant -- converted toif len(uv) != 2: raise ValueError(...)so it survivespython -Oinstead of silently vanishing.Camera.py:nu/nv/width/heighteach assertedself._imagesize is not None, "imagesize not set"-- a real "you called this before configuring the camera" caller mistake (the informative message was the tell), not an internal invariant. Converted toif self._imagesize is None: raise ValueError(...), matching the existingraise ValueError("imagesize or rho properties not set")convention already used elsewhere in this file (Camera.py:1900).Test plan
🤖 Generated with Claude Code