fix(itkwasm): keep the Image bufferedRegion consistent with data - #1574
Merged
thewtex merged 6 commits intoAug 5, 2026
Merged
Conversation
For:
WARN Encountered 1 warning while parsing the manifest:
⚠ The `project` field is deprecated. Use `workspace` instead.
╭─[/home/matt/src/ITK-Wasm/packages/core/python/itkwasm/pyproject.toml:51:1]
50 │
51 │ ╭─▶ [tool.pixi.project]
52 │ │ channels = ["conda-forge"]
53 │ ├─▶ platforms = ["win-64", "linux-64", "linux-aarch64", "osx-arm64"]
· ╰──── replace this with 'workspace'
54 │
╰────
The buffered region was only derived when an Image was constructed, so it went stale when the size and data were assigned afterwards: image = Image() image.size = [4, 4] image.data = np.arange(16, dtype=np.uint8).reshape((4, 4)) Since d72c150 made the buffered region the source of the pixel data shape, converting such an image raised "cannot reshape array of size 16 into shape (1,1)" in itkwasm.pyodide.to_py. Derive the buffered region size from the data buffer whenever data is assigned, keeping an explicitly provided region index. A new region is assigned instead of mutating the existing one so that assigning data to a shallow copy of an image does not modify the region of the image it was copied from. The default region no longer aliases the largest possible region. Data that does not describe a region, e.g. a raveled buffer or a data: URI, leaves it untouched. Convert dict imageType and bufferedRegion values however they are assigned, not only when they are passed to the constructor. Install the wheel built for the current version in the Pyodide tests -- the version was hardcoded, so a stale wheel was silently installed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes a correctness issue in the Python itkwasm.Image model where bufferedRegion could become stale when data is assigned after construction, which in turn broke JS↔Py round-trips after bufferedRegion became the source of pixel data shape.
Changes:
- Derive and maintain
Image.bufferedRegion.sizefrom assigneddata.shape(when the shape describes a region), including shallow-copy safety by assigning a newImageRegion. - Prevent default
bufferedRegionfrom aliasingsize, and convert dict-assignedimageType/bufferedRegionconsistently via__setattr__. - Update/expand tests (including Pyodide wheel version selection via
itkwasm.__version__) and apply formatting adjustments.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/core/python/itkwasm/itkwasm/image.py | Keeps bufferedRegion consistent with data assignments and avoids aliasing with size; adds dict conversion on assignment. |
| packages/core/python/itkwasm/itkwasm/init.py | Bumps package version to 1.0b199. |
| packages/core/python/itkwasm/test/test_image.py | Adds coverage for buffered region behavior (post-construction data, shallow copy, dict assignment, etc.). |
| packages/core/python/itkwasm/test/test_pyodide.py | Uses itkwasm.__version__ to select the correct wheel for Pyodide tests; formatting cleanup. |
| packages/core/python/itkwasm/test/test_pipeline.py | Formatting-only update for transform parameter assertions. |
| packages/core/python/itkwasm/test/test_transform.py | Fixes formatting/indentation around a transform verification call. |
| packages/core/python/itkwasm/test/test_image_from_array.py | Minor formatting fix in buffered region assertions. |
| packages/core/python/itkwasm/README.md | Updates developer test instructions to use pixi. |
| packages/core/python/itkwasm/pyproject.toml | Adjusts pixi config section name and retains pixi tasks for download/build/test flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The docstring read as though a data: URI were a supported value for Image.data. It is a transient state: the C++ ImageJSON emits the buffer address as a string, so Image(**image_json) briefly holds it before pipeline.py reads the buffer into an array. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
itkwasm.Image.bufferedRegionwas only derived when anImagewas constructed, so it went stale as soon assizeanddatawere assigned afterwards. Since d72c150 made the buffered region the source of the pixel data shape (to support information-only image outputs), a plain round trip raised:This was failing
pixi run testinpackages/core/python/itkwasm, intest_pyodide.py::test_image_conversion.Changes
itkwasm/image.py_buffered_region_size(data, dimension)helper derives the region size from the shape of a pixel buffer, excluding the trailing components axis.Image.__setattr__keepsbufferedRegionconsistent whendatais assigned after construction. An explicitly provided region index is preserved, and a newImageRegionis assigned instead of mutating the existing one, so that assigning data to a shallow copy of an image -- asitkwasm-downsample-cucimdoes viacopy.copy-- does not modify the region of the image it was copied from.image.size[0] = 8previously changedbufferedRegion.sizealong with it.data:URI found in pipeline output JSON.imageTypeandbufferedRegionvalues are converted however they are assigned, not only when passed to the constructor. This removes the duplicated conversions from__post_init__and avoids anAttributeErrorwhen a dictimageType-- permitted by theUnion[ImageType, Dict]annotation -- is assigned beforedata.An explicitly provided region still wins, so information-only images (
bufferedRegion.size == [0, 0]withsize == [4, 4]) are unaffected, and assigningsizealone never touches the buffered region: the largest possible region and the buffered region remain independent.test/test_pyodide.pyTake the wheel version from
itkwasm.__version__. It was hardcoded to1.0b195while the package is at1.0b199, so the Pyodide tests silently installed a stale wheel, and would not find a wheel at all on a clean checkout.test/test_image.pyEight tests for the buffered region: data assigned after construction, non-aliasing with
size, index preservation, shallow copy independence, the vector components axis, raveled anddata:URI buffers, information-only images, and dict field assignment. Five of them fail against the previousimage.py.Also on this branch:
[tool.pixi.workspace]inpyproject.toml, README instructions for running the tests with pixi, a version bump to 1.0b199, and black formatting of the test modules.Testing
pixi run testinpackages/core/python/itkwasm: 53 passed, 2 skipped. The skips are thecupytests, which is not installed locally.🤖 Generated with Claude Code