Repository navigation
fix: OpenCV 5 compatibility fixes (aruco, HoughLinesP, BRISK/AKAZE, MSER) - #61
Merged
Merged
Conversation
OpenCV 5's FONT_HERSHEY_* glyphs render via a different embedded font than OpenCV 4, so an exact rendered-pixel-count assertion isn't stable across versions -- affects both the default '+' marker (itself drawn via cv2.putText) and explicit text= labels. Assert shape (some pixels drawn, roughly centered) instead of an exact count. Verified passing under real OpenCV 4.13.0. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
OpenCV 5 dropped BRISK_create/AKAZE_create from the main cv2 namespace; they now live under cv2.xfeatures2d (verified empirically: opencv-contrib-python 5.0.0.93 vs 4.13.0.92, no OPENCV_ENABLE_NONFREE build flag needed for these, unlike SURF). A prior code review flagged the BRISK relocation as a "likely copy-paste slip" -- that's disproven: cv2.BRISK_create genuinely does not exist in OpenCV 5. Resolve by attribute existence (cv2 first, then cv2.xfeatures2d) rather than a version-string check, so this keeps working unchanged if a future OpenCV release moves things back or a build has both. SIFT/ORB are unaffected, unchanged. Added test_brisk/test_akaze -- there was no prior coverage at all for either detector. Verified the regression tests actually catch the bug: reverting the fix under a real OpenCV 5.0.0.93 install makes both fail with AttributeError: module 'cv2' has no attribute 'BRISK_create', confirming these aren't tests that would pass regardless. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
OpenCV 5's HoughLinesP returns a true (N,4) array; OpenCV 4 returns (N,1,4) (the old std::vector<T>-wrapping convention). lines_p() unconditionally did lines[:, 0, :], which raises IndexError: too many indices for array on OpenCV 5. Guards on ndim/shape before indexing, rather than np.squeeze(): once the shape is already fully confirmed, a direct index is simpler than squeeze's extra validation, and squeeze has its own failure modes here anyway -- axis-less squeeze would incorrectly collapse the N=1 case (OpenCV 4's (1,1,4) loses its row dimension, becoming (4,) instead of (1,4)), and a fixed squeeze(axis=1) crashes outright on OpenCV 5's (N,4) output since axis 1 has size 4, not 1. Checks ndim before shape[1] so an unexpected 1D/0D array short-circuits safely instead of raising on the shape index. Added a real test_lines_p (previously a TODO stub with no assertions at all -- draws a line, exercises the actual detector). Verified the test catches the regression: reverting the fix under a real OpenCV 5.0.0.93 install fails with the exact IndexError described above. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two singleton-dim breaks in ImageFiducials.py, both verified empirically (opencv-contrib-python 5.0.0.93 vs 4.13.0.92): - ArucoDetector.detectMarkers()'s ids array is (N,1) on OpenCV 4 but a flat (N,) on OpenCV 5. Code doing id[0] to extract each tag's scalar ID broke on OpenCV 5 (id is already 0-d there) -- fixed by flattening ids once right after detectMarkers() in both fiducial() and FiducialCollection.estimatePose(), then using the scalar directly. - Board.matchImagePoints() returns (M,1,3)/(M,1,2) on OpenCV 4 but true (M,3)/(M,2) on OpenCV 5 -- not previously identified. Found by running the existing test_matchImagePoints under a real OpenCV 5 install (it already asserts the documented (M,1,3)/(M,1,2) shape). Normalized the return value back to that documented shape so the method's contract is stable across both versions. Also spotted, logged separately (not fixed here, unrelated to opencv5 -- pre-existing on both versions): FiducialCollection.estimatePose's ID-filtering step zips an already-filtered cornerss against the original unfiltered ids, silently misaligning when any detected marker isn't part of this board. See #52. Full tests/test_image_fiducials.py passes under real OpenCV 4.13. Under real OpenCV 5.0.0.93, 5/7 pass; the remaining 2 (test_draw, test_estimatePose) fail on cv2.aruco.estimatePoseSingleMarkers being fully removed, which is Step 3c's fix, not this one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CentralCamera.images2C (checkerboard calibration) had zero test coverage at all -- only referenced from an example notebook. Verified empirically whether OpenCV 5's findChessboardCorners/cornerSubPix returning (N,2) instead of OpenCV 4's (N,1,2) breaks this pipeline: it does not. cv2.calibrateCamera and cv2.drawChessboardCorners both accept either shape natively, and the raw corner arrays aren't part of images2C's public return value (C, distortion, list of CalibrationFrame namedtuples) -- so no source change was needed here, unlike the other Step 3b fixes. Added a real regression test instead: synthetic checkerboard views at three different scales/positions, exercising the full find-corners -> refine -> calibrate pipeline. Passes under both real OpenCV 4.13.0 and 5.0.0.93, confirming the pipeline is genuinely shape-agnostic rather than untested-and-hoping. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cv2.aruco.estimatePoseSingleMarkers is completely removed in OpenCV 5 (not moved/renamed -- hasattr(cv2.aruco, "estimatePoseSingleMarkers") is False in a real 5.0.0 install). Replaced with OpenCV's own documented replacement for planar square markers: per-marker cv2.solvePnP with SOLVEPNP_IPPE_SQUARE, using the same object-point convention (clockwise from top-left, Z out of the marker) the removed function used internally. This works unchanged on OpenCV 4 too, so no version branching is needed -- it's a straight replacement, not a compat shim. Verified beyond shape compatibility: reprojecting the estimated pose's object points back through the camera model reproduces the detected corners exactly (0.0 px RMSE) on a synthetic fronto-parallel marker scene, and the recovered distance matches the expected geometry. Full tests/test_image_fiducials.py (7 tests, including the two that were previously failing under OpenCV 5 -- test_draw, test_estimatePose) now passes under both real OpenCV 4.13.0 and 5.0.0.93. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…epancy
Resolves the discrepancy flagged in tech-debt.md's original opencv5
incident writeup ("MSER indexing return shape changed") that an
earlier synthetic-image probe this session had disproven -- the probe
just didn't happen to hit the empty-results case.
Found by running the full test suite under a real OpenCV 5.0.0.93
install: test_mser_indexing crashed with TypeError: tuple indices must
be integers or slices, not tuple. Root cause: when detectRegions()
finds no regions, OpenCV 5 returns bboxes as an empty tuple () instead
of an empty ndarray(0,4) -- the unconditional bboxes[:, 2:] = ...
slice assignment in MSERFeature.__init__ only works on an ndarray.
Fixed by normalizing to np.zeros((0, 4)) first.
Separately, and not fixable by this shape-compat change: verified
empirically that OpenCV 5's MSER finds fewer/zero regions than OpenCV
4 for identical default parameters and identical grayscale input
(tests/data/shark2.png: 2 regions on 4.13.0, 0 on 5.0.0.93) -- a real
upstream algorithm-behavior difference. Logged as its own issue (#54)
rather than silently worked around. test_mser_indexing now skips (with
a clear reason and issue link) specifically when this exact zero-result
case occurs, rather than weakening the assertion for OpenCV 4 too or
gating on version alone -- it self-heals if the upstream behavior is
ever fixed.
tests/test_image_region_features.py: 3 passed on real OpenCV 4.13.0;
2 passed + 1 skipped (not failed) on real OpenCV 5.0.0.93.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 1 high |
🟢 Metrics 10 complexity · 0 duplication
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.
2 tasks done
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
1 participant
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
Applies the OpenCV 5 compatibility fixes identified in #44's audit:
ImagePointFeatures.py:BRISK/AKAZErelocated tocv2.xfeatures2don OpenCV 5 — resolve via version-aware lookupImageFiducials.py:cv2.aruco.estimatePoseSingleMarkersremoved outright on OpenCV 5 — replaced withcv2.solvePnP(..., flags=cv2.SOLVEPNP_IPPE_SQUARE), works unchanged on both versionsImageFiducials.py: normalize arucoids/matchImagePointsshapes — OpenCV 5 drops the singleton middle dimension ((N,1)→(N,))ImageLineFeatures.py: same singleton-dimension fix forcv2.HoughLinesP's(N,1,4)→(N,4)shape changeImageRegionFeatures.py: handle MSER's empty-tuple bboxes on OpenCV 5, resolve the discrepancy flagged in OpenCV 5's MSER finds fewer/zero regions than OpenCV 4 on identical input #54test_graphics.py: relaxdraw_pointpixel-count assertions — OpenCV 5 rendersFONT_HERSHEY_*via a different embedded fonttest_camera.py: add missing end-to-end coverage forCameraBase.images2C(checkerboard calibration) — verified thefindChessboardCorners/cornerSubPixshape change doesn't break this pipeline, so no source fix needed there, just test coverageProgresses #44 (does not close it — relaxing
pyproject.toml's<5.0.0pin and adding a CI version-matrix axis is tracked separately as future work in that issue).Test plan