feat(feature): GLCM texture features + regionprops extra_properties - #49
Conversation
Replace `list(set(properties + ["label"]))` with an order-preserving `list(dict.fromkeys(...))`. A bare `set()` iterates in an order that depends on the interpreter hash seed, which differs per spawned process worker. Under a process-based executor each worker would therefore emit regionprops_table columns in a different order, misaligning the pooled cross-worker feature matrix. The default serial executor masked this, but the dedup must be deterministic for a stable named-column contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s_table Add an `extra_properties` parameter to `regionprops` and `regionprops_table` that forwards user-defined `func(regionmask, intensity)` callables to the underlying skimage/cucim implementation (both accept it). This lets callers compute custom per-region statistics (e.g. intensity percentiles, gradient/Laplacian summaries) in the same device-agnostic pass as the built-in properties. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add `cubic.feature.glcm_features`, a Haralick gray-level co-occurrence matrix extractor for 2D (H, W) and 3D (D, H, W) images that runs on NumPy or CuPy arrays through duck typing. cuCIM has no graycomatrix (rapidsai/cucim#530) and skimage's is 2D-only, so the co-occurrence matrix is accumulated with `np.bincount` (works on both devices) over the unique half-space directions (4 in 2D, 13 in 3D), and the seven Haralick properties are computed with closed-form reductions that reproduce `skimage.feature.graycoprops` exactly, including its correlation near-zero-std guard (1.0 rather than NaN). Quantization is per-image by default, making the features scale invariant; an explicit value_range quantizes several regions over a shared set of levels. An optional boolean mask restricts counting to foreground pairs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Surfaces the new cubic.feature.glcm_features API and the regionprops_table extra_properties passthrough so downstream consumers (dynacell CP feature track) can pin against it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reviewer's GuideAdds a device-agnostic GLCM Haralick texture feature primitive (2D/3D, CPU/GPU), exposes it via the feature API, forwards extra_properties through regionprops/regionprops_table with deterministic property order, and bumps the package version. Sequence diagram for regionprops_table extra_properties passthroughsequenceDiagram
actor Client
participant Voxel as voxel.regionprops_table
participant Measure as measure.regionprops_table
Client->>Voxel: regionprops_table(label_image, intensity_image, properties, spacing, extra_properties)
Voxel->>Voxel: dict.fromkeys([*properties, label])
Voxel->>Voxel: list(...) → ordered properties
Voxel->>Voxel: spacing_arg = tuple(spacing)
Voxel->>Measure: regionprops_table(label_image, intensity_image, properties, spacing_arg, extra_properties)
Measure-->>Voxel: table dict
Voxel-->>Client: table dict
Flow diagram for glcm_features texture computationflowchart TD
Image["image (2D/3D NumPy or CuPy)"] --> Validate["validate ndim, mask, levels"]
Validate --> Range["derive value_range (lo, hi)"]
Range --> Quant["_quantize → quantized levels"]
Quant --> Offsets["_unit_offsets → unit directions"]
Offsets --> ScaleOffsets["build offsets for distances"]
ScaleOffsets --> LoopOffsets{{"for each offset"}}
LoopOffsets --> DirMat["_direction_matrix → GLCM (NumPy)"]
DirMat --> Props["_haralick_props → per-direction props"]
Props --> Accum["accumulate totals[prop]"]
Accum --> LoopOffsets
Accum --> Avg["average over directions"]
Avg --> Output["dict of Haralick features"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="cubic/feature/texture.py" line_range="246-254" />
<code_context>
+ for unit in _unit_offsets(image.ndim)
+ ]
+
+ totals = {prop: 0.0 for prop in _PROPS}
+ for off in offsets:
+ glcm = _direction_matrix(quant, off, levels, mask, symmetric, normed)
+ props = _haralick_props(glcm, levels)
+ for prop in _PROPS:
+ totals[prop] += props[prop]
+
+ n = len(offsets)
+ return {prop: totals[prop] / n for prop in _PROPS}
</code_context>
<issue_to_address>
**issue:** Averaging without handling empty offsets assumes at least one valid direction
This relies on `offsets` being non-empty (`n > 0`). If upstream validation or `_unit_offsets` change, this could become a divide-by-zero. Consider an explicit guard (e.g., raise `ValueError` when `n == 0`) to fail fast and keep behavior well-defined.
</issue_to_address>
### Comment 2
<location path="tests/feature/test_voxel.py" line_range="57-66" />
<code_context>
+ assert keys.index("centroid-0") < keys.index("label")
+
+
+def test_regionprops_table_extra_properties() -> None:
+ """extra_properties callables are forwarded and keyed by function name."""
+
+ def intensity_range(regionmask: np.ndarray, intensity: np.ndarray) -> float:
+ values = intensity[regionmask]
+ return float(values.max() - values.min())
+
+ labels = np.array([[0, 1, 1], [2, 2, 0]], dtype=np.int32)
+ intensity = np.array([[0, 10, 20], [5, 5, 0]], dtype=np.float32)
+
+ out = voxel.regionprops_table(
+ labels,
+ intensity_image=intensity,
+ properties=["area"],
+ extra_properties=(intensity_range,),
+ )
+
+ assert "intensity_range" in out
+ by_label = dict(zip(out["label"].tolist(), out["intensity_range"].tolist()))
+ assert by_label[1] == 10.0 # intensities {10, 20}
+ assert by_label[2] == 0.0 # intensities {5, 5}
</code_context>
<issue_to_address>
**suggestion (testing):** Add a companion test for `regionprops` forwarding `extra_properties`, not just `regionprops_table`.
This only exercises forwarding via `regionprops_table`. Please also add a test that calls `voxel.regionprops(..., extra_properties=(func,))` and checks that the returned regions expose the custom attribute and that its values per label are correct, so both APIs stay covered and in sync.
Suggested implementation:
```python
def test_regionprops_table_extra_properties() -> None:
"""extra_properties callables are forwarded and keyed by function name."""
def intensity_range(regionmask: np.ndarray, intensity: np.ndarray) -> float:
values = intensity[regionmask]
return float(values.max() - values.min())
labels = np.array([[0, 1, 1], [2, 2, 0]], dtype=np.int32)
intensity = np.array([[0, 10, 20], [5, 5, 0]], dtype=np.float32)
out = voxel.regionprops_table(
labels,
intensity_image=intensity,
properties=["area"],
extra_properties=(intensity_range,),
)
assert "intensity_range" in out
by_label = dict(zip(out["label"].tolist(), out["intensity_range"].tolist()))
assert by_label[1] == 10.0 # intensities {10, 20}
assert by_label[2] == 0.0 # intensities {5, 5}
def test_regionprops_extra_properties() -> None:
"""extra_properties callables are forwarded and exposed as attributes."""
def intensity_range(regionmask: np.ndarray, intensity: np.ndarray) -> float:
values = intensity[regionmask]
return float(values.max() - values.min())
labels = np.array([[0, 1, 1], [2, 2, 0]], dtype=np.int32)
intensity = np.array([[0, 10, 20], [5, 5, 0]], dtype=np.float32)
regions = voxel.regionprops(
labels,
intensity_image=intensity,
extra_properties=(intensity_range,),
)
# Ensure custom property is exposed as an attribute and values are correct per label.
by_label = {region.label: getattr(region, "intensity_range") for region in regions}
assert by_label[1] == 10.0 # intensities {10, 20}
assert by_label[2] == 0.0 # intensities {5, 5}
```
This patch assumes that `np` (NumPy) and `voxel` are already imported in `tests/feature/test_voxel.py`, consistent with the existing tests. If they are not, corresponding imports should be present at the top of the file (e.g., `import numpy as np` and the appropriate import for `voxel`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
When a direction has no valid pairs — an empty overlap region (e.g. a single-row image's vertical offsets) or a fully masked-out direction — the co-occurrence index array is empty. `cupy.bincount` raises on empty input (it reduces `x.max()` to size the output, which has no identity), whereas `numpy.bincount` returns zeros. Short-circuit the empty case to build the zero GLCM directly, so the matrix stays device-agnostic and yields the same properties skimage gives for an empty co-occurrence matrix (contrast/entropy 0, correlation 1.0 via the std guard). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The empty-pair guard assigns a float64 zero matrix, which makes mypy infer `counts` as float64; the populated branch then reassigned the int result of `np.bincount(...).reshape(...)` before the cast, which the stricter CI mypy stubs reject as an incompatible assignment. Produce the float64 matrix in a single expression so both branches share the type. Runtime behavior is unchanged (the matrix was always cast to float64). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds device-agnostic Haralick GLCM texture feature extraction (2D/3D; NumPy/CuPy) and extends the voxel feature wrappers to forward extra_properties to underlying skimage/cucim regionprops* APIs, while fixing nondeterministic regionprops_table property ordering to be stable across multiprocessing workers.
Changes:
- Introduces
cubic.feature.texture.glcm_featureswith direction-averaged Haralick GLCM properties (including entropy) and masking support. - Forwards
extra_propertiesthroughcubic.feature.voxel.regionpropsandregionprops_table. - Fixes
regionprops_tableproperty deduplication to preserve requested order deterministically; bumps package version.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
cubic/feature/texture.py |
New device-agnostic GLCM accumulation + Haralick reductions for 2D/3D images. |
cubic/feature/voxel.py |
Adds extra_properties passthrough and makes property dedup order-preserving. |
cubic/feature/__init__.py |
Exposes glcm_features from the feature package. |
cubic/__init__.py |
Version bump to 0.7.0a12. |
tests/feature/test_texture.py |
New test coverage for 2D parity vs skimage, 3D support, masking, CPU/GPU parity, and validation. |
tests/feature/test_voxel.py |
Adds tests for deterministic property ordering and extra_properties forwarding. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two cleanups from a quality pass, both behavior-preserving (graycoprops parity unchanged at <=1e-9): - Compute the direction-independent level-index ramps (i, j, diff**2, |diff|) once in glcm_features and pass them to _haralick_props instead of rebuilding them on every one of the up-to-13 per-direction calls. - Drop the module-level _PROPS tuple, which duplicated the keys returned by _haralick_props and could drift from them; derive the output keys and their order directly from the returned dict, and average a list of per-direction dicts rather than hand-accumulating into a totals dict. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reject an empty `distances` (which left `offsets` empty and divided the per-direction average by zero) and any non-positive distance (distance 0 yields the all-zero self-offset; negative distances yield negated offsets that are meaningless for a GLCM). Fail fast with a ValueError, matching the existing levels/ndim/mask validations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The object API regionprops gained the same extra_properties passthrough as regionprops_table but was untested. Add a companion test that checks the custom property is exposed as a per-region attribute with the correct value, keeping both APIs covered and in sync. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Cubic-side (Part A) of the CP feature-enrichment plan. Adds the texture
and extensibility primitives the dynacell "conventional image" feature
track needs, so the dynacell change (separate VisCy PR) can pin a
released cubic version.
cubic.feature.glcm_features(new): device-agnostic Haralick GLCMtexture features for 2D
(H, W)and 3D(D, H, W)images. cuCIM hasno
graycomatrix([FEA] add graycomatrix and graycoprops in skimage.feature rapidsai/cucim#530) and skimage's is 2D-only, so theco-occurrence matrix is accumulated with
np.bincount(works on NumPyand CuPy) over the unique half-space directions (4 in 2D, 13 in 3D).
The seven Haralick properties (contrast, dissimilarity, homogeneity,
ASM, energy, correlation, entropy) are closed-form reductions that
reproduce
skimage.feature.graycopropsexactly (≤1e-9 in tests),including its correlation near-zero-std guard (→ 1.0, not NaN).
Quantization is per-image by default (scale-invariant); an explicit
value_rangequantizes several regions over shared levels, and anoptional boolean
maskrestricts counting to foreground pairs.regionprops/regionprops_tableextra_propertiespassthrough:forwards user-defined
func(regionmask, intensity)callables to theunderlying skimage/cucim implementation (both accept it), so callers
can compute custom per-region stats in the same device-agnostic pass.
Property-order fix (
fix:):regionprops_tablededuped propertieswith a bare
set(), whose iteration order depends on the interpreterhash seed and so differs per spawned process worker — scrambling output
column order across workers and misaligning a pooled feature matrix.
Replaced with order-preserving
list(dict.fromkeys(...)).Version bump
0.7.0a11 → 0.7.0a12.Why
The dynacell CP track extends its per-cell feature matrix with
distribution + texture stats and needs (a) custom
regionpropsproperties, (b) a GLCM texture primitive that works on both CPU and GPU
and in 3D, and (c) deterministic column ordering for the pooled
cross-FOV matrix that feeds FID/KID. None exist in cubic today.
Tests
tests/feature/test_texture.py(new) +tests/feature/test_voxel.py:2D parity vs
graycopropsfor all 7 props, hand-computed entropy,3D direction count / half-space property, CPU↔GPU parity (runs on
this GPU host), affine scale-invariance, mask excludes background pairs,
constant-region correlation = 1.0, input validation; plus
extra_propertiesforwarding and property-order determinism.Checks
ruff check .✅ruff format --check✅mypy --ignore-missing-imports cubic/✅ (no issues, 51 files)pytest✅ 327 passed (315 prior + 12 new; GPU tests executed)🤖 Generated with Claude Code
Summary by Sourcery
Add device-agnostic GLCM texture feature extraction and extend region-based feature APIs with custom property support and deterministic output ordering.
New Features:
Bug Fixes:
Enhancements:
Tests:
Chores: