Allow read-only arrays as input to remap by stefanv · Pull Request #7535 · scikit-image/scikit-image

stefanv · 2024-09-09T20:39:26Z

Closes #6378.

This requires adding consts across a much larger portion of our Cython code, but this WIP illustrates how it can be done.

Release note

For maintainers and optionally contributors, please refer to the instructions on how to document this PR for the release notes.

In `skimage.util.map_array`, allow read-only arrays 
(`WRITEABLE` flag set to `False`) as input to the parameters
`input_arr`, `input_vals`, and `output_vals`.

lagru · 2024-09-17T13:41:47Z

Is this ready for review? I feel like we should add a test for this right?

galenlynch · 2025-04-08T00:53:39Z

Would love to see a similar change made to marching_cubes.

Workaround for scikit-image/scikit-image#6378 Can be removed after scikit-image/scikit-image#7535 is released.

coderabbitai · 2026-02-06T10:58:24Z

📝 Walkthrough

Walkthrough

Bumped minimum Cython from 3.0.8 to 3.0.10 in build configs, added const qualifiers to _map_array input parameters, and added a parameterized test verifying behavior with read-only input/output arrays.

Changes

Cohort / File(s)	Summary
Dependency Version Updates `environment.yml`, `pyproject.toml`, `requirements/build.txt`	Bumped minimum Cython requirement from `>=3.0.8` to `>=3.0.10` (kept `!=3.2.0b1`) in build and requirement files.
Function Signature `src/skimage/util/_remap.pyx`	Updated `_map_array` signature to add `const` qualifiers for `inarr`, `inval`, and `outval`, enforcing immutability of those inputs.
Test Coverage `tests/skimage/util/test_map_array.py`	Added `itertools` import and `test_map_array_read_only`, parameterized over writable/read-only combinations; verifies read-only inputs are accepted and a read-only output raises `ValueError`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately summarizes the main change: allowing read-only arrays as input to the remap function, which is evidenced by the const qualifiers added to function parameters and the new test validating read-only input handling.
Description check	✅ Passed	The PR description clearly relates to the changeset, explaining the need to allow read-only arrays in skimage.util.map_array by adding const qualifiers.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pyproject.toml (1)
123-131: ⚠️ Potential issue | 🔴 Critical

[build-system] Cython version not updated — build may fail with const memoryview syntax.

Line 127 still pins Cython>=3.0.8, while [project.optional-dependencies] build (line 51) was bumped to >=3.0.10. The comment on line 48 explicitly warns to keep these in sync. Since the const memoryview qualifier in _remap.pyx requires ≥3.0.10, a build using only [build-system] requirements (e.g., pip install .) could pick Cython 3.0.8/3.0.9 and fail to compile.
Proposed fix
 [build-system]
 build-backend = 'mesonpy'
 requires = [
     'meson-python>=0.16',
-    'Cython>=3.0.8,!=3.2.0b1',
+    'Cython>=3.0.10,!=3.2.0b1',
     'pythran>=0.16',
     'lazy_loader>=0.4',
     'numpy>=2.0',
 ]

tests/skimage/util/test_map_array.py

jni · 2026-02-06T11:03:55Z

@lagru @stefanv I just ran into this in jni/skan#256 (and have heard of several other cases, just didn't have time to investigate), so I am hoping to revive this. 😊 I've merged in main, added some tests, and pushed to @stefanv's branch, so I hope this is good to go! I'm going to approve but another read through by @lagru would be appreciated! 😊

jni · 2026-02-06T11:04:01Z

@galenlynch thanks for chiming in! We should definitely do this for many of our Cython functions, but we should probably keep PRs small to help get them in quickly. If you would like to directly contribute a fix for marching cubes, we would appreciate it, otherwise, we'll make a follow-up issue and get to it when we can... 😅 As you have seen it can take a while, but we do eventually get to things... 😅

See: https://github.com/scikit-image/scikit-image/pull/7535\#pullrequestreview-3762383679

lagru

I couldn't help but tweak the test a little bit since I had it checked out anyway.

Will merge if CI stays green. Thanks everyone. 😊

* origin/main: (27 commits) Move `_shared` to `_skimage2` (#8102) Port `pad_footprint` & `mirror_footprint` to `skimage2` (#8094) Add rescaling API to skimage2 (#8075) Move `skimage2` implementation into `_skimage2` (#8093) Undo (double) mirroring in `ski2.morphology.dilation` (#8060) Grammar (#8091) Move `ensure_spacing` into `skimage2` (#8067) Ensure that `skimage2` does not eagerly import `skimage` (#8087) Use lazy-loader 0.5 (#8080) MAINT: make ellipse fitting forward compatible (#8054) Avoid deprecated assign to ndarray.shape (#8020) Avoid circular import in `feature/corner.py` (#8077) Turn off coderabbit auto-labeling & label checks (#8070) Fix GIL being re-enabled by C++ extensions (#8059) Fix conventions following docstub and misc. (#8055) Port grayscale morphology operators to skimage2 (#8046) Add revised `peak_local_max` to `skimage2` (#8039) Allow read-only arrays as input to remap (#7535) Add `prescale` parameter to "blob functions" (#7858) Try to make coderabbit respect exisiting type labels (#8042) ...

Allow read-only arrays as input to remap

4908222

stefanv mentioned this pull request Sep 9, 2024

utils.map_array: error for non-writeable input arrays #6378

Closed

stefanv added the 🩹 type: Bug fix Fixes unexpected or incorrect behavior label Sep 9, 2024

Up minimum Cython dependency

a2a9110

This comment was marked as outdated.

Sign in to view

github-actions bot added the 😴 Dormant No recent activity label Mar 17, 2025

github-actions bot removed the 😴 Dormant No recent activity label Apr 8, 2025

jni added a commit to jni/skan that referenced this pull request Feb 6, 2026

Fix 'buffer source array is read-only' error

41b03f4

Workaround for scikit-image/scikit-image#6378 Can be removed after scikit-image/scikit-image#7535 is released.

jni mentioned this pull request Feb 6, 2026

Fix 'buffer source array is read-only' error jni/skan#256

Merged

jni added a commit to jni/skan that referenced this pull request Feb 6, 2026

Fix 'buffer source array is read-only' error (#256)

4eacfa6

Workaround for scikit-image/scikit-image#6378 Can be removed after scikit-image/scikit-image#7535 is released.

jni added 2 commits February 6, 2026 21:34

Merge branch 'main' into const-remap

070b367

Add test for map_array with read-only array

2950248

coderabbitai bot reviewed Feb 6, 2026

View reviewed changes

tests/skimage/util/test_map_array.py Outdated Show resolved Hide resolved

Fill in missing value fill value for np.full

e0e65b3

jni approved these changes Feb 6, 2026

View reviewed changes

Update Cython min req in build section of pyproject.toml

09b94cd

See: https://github.com/scikit-image/scikit-image/pull/7535\#pullrequestreview-3762383679

lagru self-requested a review February 8, 2026 16:00

Check result and test other dtypes too

6c905c5

lagru approved these changes Feb 8, 2026

View reviewed changes

lagru merged commit 70ab2a6 into scikit-image:main Feb 8, 2026
35 of 37 checks passed

lagru added this to the 0.27 milestone Feb 8, 2026

This was referenced Feb 13, 2026

Temporary patch for skimage ArrayMap's read-only issues royerlab/ultrack#260

Closed

Fix ValueError: buffer source array is read-only in MIPSolver royerlab/ultrack#252

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow read-only arrays as input to remap#7535

Allow read-only arrays as input to remap#7535
lagru merged 7 commits intoscikit-image:mainfrom
stefanv:const-remap

stefanv commented Sep 9, 2024 •

edited by lagru

Loading

Uh oh!

lagru commented Sep 17, 2024

Uh oh!

This comment was marked as outdated.

galenlynch commented Apr 8, 2025

Uh oh!

coderabbitai bot commented Feb 6, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

jni commented Feb 6, 2026

Uh oh!

jni commented Feb 6, 2026

Uh oh!

lagru left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

stefanv commented Sep 9, 2024 • edited by lagru Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Release note

Uh oh!

lagru commented Sep 17, 2024

Uh oh!

This comment was marked as outdated.

galenlynch commented Apr 8, 2025

Uh oh!

coderabbitai bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jni commented Feb 6, 2026

Uh oh!

jni commented Feb 6, 2026

Uh oh!

lagru left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

stefanv commented Sep 9, 2024 •

edited by lagru

Loading

coderabbitai bot commented Feb 6, 2026 •

edited

Loading