Looks like the marker is defined optionally here. So, why is pytest_run_parallel not available on macos?

This is how we set it up in NumPy, FWIW. I didn't want to make pytest_run_parallel a required test dependency for everyone who wants to test scikit-image.

But I think you did identify the issue: scikit-image has two different conftest.py files! I totally missed that.

⚠️ Potential issue | 🟠 Major

#!/bin/bash
set -euo pipefail

echo "ImageCollection multi-frame path:"
rg -n -C3 "_frame_index|img_num|unexpected keyword argument 'img_num'" src/skimage/io/collection.py

echo
echo "Plugin imread definitions and img_num handling:"
fd '_plugin\.py$' src/skimage/io/_plugins -x sh -c '
  echo "== $1 =="
  rg -n -C2 "def imread\(" "$1"
  rg -n -C2 "\bimg_num\b" "$1" || true
' sh {}

Don't fall back to loading without img_num on the multi-frame path.

We only enter this block after n has already been resolved through self._frame_index, i.e. a specific frame inside a multi-frame file is being requested. Retrying without img_num silently changes the semantics for n > 0 and can return the backend's default frame instead of the requested one.

Several shipped backends do not accept img_num (simpleitk, fits, imread_plugin, gdal), so this fallback will always trigger for them on multi-frame paths, causing silent data loss when accessing frames other than the first.

This is commenting on pre-existing issues, so I'm ignoring it.

⚠️ Potential issue | 🟠 Major

Reset the curvature operator on each API call.

threading.local() only isolates threads. It does not reset the _fcycle between calls on the same worker thread, so the next invocation starts on the opposite operator whenever the previous call consumed an odd number of smoothing steps. That makes results call-order-dependent.

-_curvop = threading.local()
+_curvop = threading.local()
@@
-    if not hasattr(_curvop, 'c'):
-        _curvop.c = _fcycle(
-            [
-                lambda u: sup_inf(inf_sup(u)),  # SIoIS
-                lambda u: inf_sup(sup_inf(u)),
-            ]
-        )  # ISoSI
+    curvop = _fcycle(
+        [
+            lambda u: sup_inf(inf_sup(u)),  # SIoIS
+            lambda u: inf_sup(sup_inf(u)),
+        ]
+    )  # ISoSI
@@
-            u = _curvop.c(u)
+            u = curvop(u)
@@
-    if not hasattr(_curvop, 'c'):
-        _curvop.c = _fcycle(
-            [
-                lambda u: sup_inf(inf_sup(u)),  # SIoIS
-                lambda u: inf_sup(sup_inf(u)),
-            ]
-        )  # ISoSI
+    curvop = _fcycle(
+        [
+            lambda u: sup_inf(inf_sup(u)),  # SIoIS
+            lambda u: inf_sup(sup_inf(u)),
+        ]
+    )  # ISoSI
@@
-            u = _curvop.c(u)
+            u = curvop(u)

Also applies to: 290-296, 321-322, 414-420, 458-459

The suggested diff is nonsense. While it's correct that making this state be thread-local doesn't fix the fact that API calls won't be independent, that's also the pre-existing single-threaded behavior.

A better way to fix this is probably to store the _fcycle state in a context variable and use with statements in the implementation to manage the context state. One way to think of context variables is that they're "souped up" thread locals that are async-local as well as thread-local and that work syntactically with the Python with statement.

I could do that refactor, but I'd rather not because the preexisting behavior is probably fine for most uses.

⚠️ Potential issue | 🟠 Major

#!/bin/bash
# Verify whether tests/skimage2/ has its own conftest.py that registers the marker
fd -t f 'conftest.py' tests/

# Check if thread_unsafe marker is registered elsewhere
rg -n "thread_unsafe" pyproject.toml tests/conftest.py 2>/dev/null || echo "Not found in pyproject.toml or tests/conftest.py"

# Check if tests/skimage2/ exists and what files are in it
fd -t f . tests/skimage2/ 2>/dev/null || echo "tests/skimage2/ does not exist or is empty"

# Search for thread_unsafe marker usage across all test files
rg "thread_unsafe" tests/ -t py

# Verify --strict-markers in pyproject.toml
rg "strict-markers" pyproject.toml

# Confirm pytest's strict-markers setting in pyproject.toml
rg -A 2 -B 2 "strict-markers" pyproject.toml

# Also verify if tests/conftest.py might register marker differently
head -50 tests/conftest.py | grep -i "thread_unsafe\|pytest_configure"

Marker registration does not cover tests/skimage2/ tests.

The thread_unsafe marker is registered only in tests/skimage/conftest.py, which applies only to tests in the tests/skimage/ subtree. However, tests/skimage2/test_skimage2.py uses @pytest.mark.thread_unsafe, and since tests/skimage2/ is a sibling directory (not a subdirectory of tests/skimage/), it does not inherit this marker registration.

With --strict-markers enabled in pyproject.toml, this will cause pytest to report an unknown marker error when running tests in tests/skimage2/.

Move this marker registration to tests/conftest.py (root-level conftest) so it applies to all test subdirectories.

This is a real issue.

⚠️ Potential issue | 🟠 Major

Mark the whole module thread-unsafe.

The autouse _use_pil_plugin() fixture mutates global plugin state before and after every test in this file. test_failed_use() and the parametrized test_deprecation_warnings_on_plugin_funcs() still run under that fixture but remain unmarked, so this module can still race under pytest-run-parallel. A module-level thread_unsafe marker is safer than tagging only three tests.

+pytestmark = pytest.mark.thread_unsafe(
+    reason="module mutates global plugin state via _use_pil_plugin"
+)
+
 `@pytest.fixture`(autouse=True)
 def _use_pil_plugin():
     io.use_plugin('pil')
     yield
     io.reset_plugins()

Also applies to: 48-49, 64-65

I disagree that a module-level mark is better, since that means the other tests in this file which are currently being run won't be run.

⚠️ Potential issue | 🟡 Minor

Add a regression test for the new rng hook.

These helpers now expose behavior that is central to the parallelization work, but the changed tests only cover the default seeded path. Please add one direct test that passes explicit RNG instances into both helpers and asserts identical output for identically seeded RNGs, so a future refactor cannot silently ignore the injected RNG.

+@testing.parametrize(
+    "make_data, shape",
+    [
+        (make_2d_syntheticdata, (70, 100)),
+        (make_3d_syntheticdata, (30, 30, 30)),
+    ],
+)
+def test_syntheticdata_respects_explicit_rng(make_data, shape):
+    rng1 = np.random.RandomState(0)
+    rng2 = np.random.RandomState(0)
+
+    data1, seeds1 = make_data(*shape, rng=rng1)
+    data2, seeds2 = make_data(*shape, rng=rng2)
+
+    np.testing.assert_array_equal(data1, data2)
+    np.testing.assert_array_equal(seeds1, seeds2)

As per coding guidelines, **/*.py: All code should have tests covering all modifications in behavior.

Also applies to: 32-39

@stefanv or another reviewer, please let me know if you want me to do this

Why not np.random.default_rng(5)?

Becuase default_rng isn't subject to the strict backward compatibility guarantees from RandomState. See numpy/numpy#29729 for where we did a similar refactor in NumPy. Robert Kern wrote the new RNG infrastructure so I generally defer to his advice on this stuff.

In an ideal world, we wouldn't even set any seeds in our test suite; but we do it to avoid running into edge cases. I think any seed that works is fine (and if you find one that isn't, then we want to know about that too!).