Skip to content

feat: add __array__ protocol to Image and Kernel, ndim to Kernel - #84

Open
petercorke wants to merge 1 commit into
mainfrom
feat/array-protocol-kernel-image
Open

feat: add __array__ protocol to Image and Kernel, ndim to Kernel#84
petercorke wants to merge 1 commit into
mainfrom
feat/array-protocol-kernel-image

Conversation

@petercorke

Copy link
Copy Markdown
Owner

Summary

  • Neither Image nor Kernel implemented the plain NumPy __array__ protocol. Image already implements __array_ufunc__/__array_function__ (covers ufuncs and the specific np.* functions it opts into), but anything outside that -- SciPy functions, Matplotlib, or any code calling np.asarray() internally rather than going through those protocols -- falls through to NumPy's default handling, which silently boxes the whole object into a useless 0-d object-dtype array instead of exposing the real pixel/kernel data:
    np.asarray(kernel)                          # -> 0-d object array
    np.linalg.matrix_rank(kernel)                # -> silently wrong
    scipy.signal.convolve2d(image, kernel)       # -> ValueError
  • Root-caused via RVC3-python's chap11.ipynb, which hands a Kernel directly to scipy.signal.convolve2d(), np.linalg.svd(), and Matplotlib's plot_surface() -- all broke the same way.
  • idisp()/plot_surface() additionally access .ndim directly (duck-typing, no array coercion involved), so Kernel also gets a plain ndim property (kernels are always 2D per the class's own constructor invariant).
  • Image.__array__ mirrors the existing .array property: read-only view by default; a dtype conversion or explicit copy=True produces a new, independent writeable array.
  • This is purely additive -- ufunc and array-function dispatch (already implemented on Image) are tried first by NumPy and are unaffected; __array__ only kicks in as NumPy's fallback for everything else.

Test plan

  • New regression tests for both classes covering np.asarray(), SciPy interop (convolve2d), dtype conversion, and the read-only-view default
  • Verified against pre-fix code: 4/5 Image tests and both Kernel tests fail with the original error shapes (ValueError: convolve2d inputs must both be 2-D arrays, AttributeError: 'Kernel' object has no attribute 'ndim', etc.); pass with the fix
  • Full existing suites (test_image_core.py, test_image_spatial.py, test_image_processing_kernel.py) pass: 119 passed

Neither Image nor Kernel implemented the plain NumPy __array__
protocol. Image already implements __array_ufunc__/__array_function__,
which cover ufuncs and the specific np.* functions it opts into, but
anything outside that -- SciPy functions, Matplotlib, or any code that
calls np.asarray() internally rather than going through those
protocols -- falls through to NumPy's default handling. Without
__array__, that default silently boxes the whole object into a
useless 0-d object-dtype array instead of exposing the real pixel/
kernel data:

    np.asarray(kernel)                          # -> 0-d object array
    np.linalg.matrix_rank(kernel)                # -> silently wrong
    scipy.signal.convolve2d(image, kernel)       # -> ValueError

Root-caused via RVC3-python's chap11.ipynb, which hands a Kernel
directly to scipy.signal.convolve2d(), np.linalg.svd(), and
matplotlib's plot_surface() -- all broke the same way. idisp()/
plot_surface() additionally access .ndim directly (duck-typing, no
array coercion involved), so Kernel also gets a plain ndim property
(kernels are always 2D by the class's own constructor invariant).

Image's __array__ mirrors the existing .array property (read-only
view by default; dtype conversion or an explicit copy=True produce a
new, independent writeable array). This is purely additive -- ufunc
and array-function dispatch (already implemented) are tried first and
are unaffected; __array__ only kicks in as NumPy's fallback.

Added regression tests for both classes covering np.asarray(),
SciPy interop (convolve2d), dtype conversion, and the read-only-view
default -- all verified to fail against the pre-fix code with the
original error shapes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 1 high

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category Results
ErrorProne 1 high

View in Codacy

🟢 Metrics 5 complexity · 0 duplication

Metric Results
Complexity 5
Duplication 0

View in Codacy

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.

@petercorke

Copy link
Copy Markdown
Owner Author

Re: the Codacy F405 alert on line 478 ('Dtype' may be undefined, or defined from star imports) — checked this one. It's not a real bug: Dtype comes from from machinevisiontoolbox.mvtb_types import * at ImageCore.py:49, and this file already uses Dtype the exact same way 6 other times (lines 83, 352, 1924, 1956, 2152, 2204, 2252) — those just aren't flagged since they predate this PR. Codacy is dinging new code for matching the file's existing style, not for anything wrong with the code itself.

The actual tech debt is that ImageCore.py/ImageWholeFeatures.py are the only two files still doing import * from mvtb_types, while everywhere else (Camera.py, ImageConstants.py, ImageIO.py, Kernel.py, _image_typing.py) imports names explicitly. That's a file-wide change, out of scope here — logged as another instance on #41, which already tracks this exact pattern. Merging as-is.

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.

1 participant