Skip to content

Validate that Intrinsics width/height are positive#60

Merged
bamdadd merged 1 commit into
bamdadd:mainfrom
dchaudhari7177:fix/validate-intrinsics-size
Jul 22, 2026
Merged

Validate that Intrinsics width/height are positive#60
bamdadd merged 1 commit into
bamdadd:mainfrom
dchaudhari7177:fix/validate-intrinsics-size

Conversation

@dchaudhari7177

Copy link
Copy Markdown
Contributor

Closes #59.

from_fov guards width > 0 / height > 0, but an Intrinsics built directly (or via from_focal) with a zero/negative size was accepted — silently breaking matrix(), in_image bounds, and every downstream projection.

Change: a field_validator on width/height rejects non-positive values with a clear ValueError, so the invariant holds however the model is constructed (mirrors the existing shape validators on Camera). All current construction paths (from_focal, from_fov, drifted) already pass positive sizes, so only the direct path tightens.

Acceptance criteria:

  • Intrinsics(..., width=0, ...) and negative width/height raise ValueError. ✅
  • A valid Intrinsics (e.g. from from_focal(100, 640, 480)) still constructs unchanged. ✅
  • Test asserts both the raise (parametrized) and the happy path. ✅

ruff check/format and mypy clean on the changed files; pytest tests/test_cameras.py green. (Pre-existing mypy type-arg warnings in smoke.py are untouched by this PR.)

from_fov guards width/height > 0, but Intrinsics built directly (or via
from_focal) accepted a zero/negative size, which silently breaks matrix(),
in_image bounds and every downstream projection.

Add a field_validator on width/height that rejects non-positive values with
a clear ValueError, so the invariant holds no matter how the model is built
(mirrors the existing shape validators on Camera). All current construction
paths already pass positive sizes, so only the direct path tightens.

Closes bamdadd#59
@bamdadd
bamdadd merged commit 87d2476 into bamdadd:main Jul 22, 2026
1 check passed
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.

Validate that Intrinsics width/height are positive

3 participants