Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new texture featurization module based on Haralick features and introduces unit tests covering scaling, per-object extraction, masking, and error handling.
Changes:
- Implement
scale_image()andcompute_texture()usingmahotas.features.haralick. - Add a new test suite for texture featurization (scaling behavior, schema, masking, error-paths).
- Update project dependencies and mark several roadmap items as completed.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 13 comments.
| File | Description |
|---|---|
src/zedprofiler/featurization/texture.py |
Implements Haralick-based texture features and grayscale scaling helper |
tests/featurization/test_texture.py |
Adds tests for scaling and texture feature extraction behavior |
pyproject.toml |
Adds new runtime dependencies for the texture implementation |
ROADMAP.md |
Updates roadmap checklist/status for completed modules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from typing import Never | ||
|
|
||
| import numpy as np | ||
| from _pytest.monkeypatch import MonkeyPatch |
| """This module generate texture features for each object in the | ||
| image using Haralick features.""" |
| "texture_name": [], | ||
| "texture_value": [], | ||
| } | ||
| for index, label in tqdm.tqdm(enumerate(labels)): |
There was a problem hiding this comment.
concur on progress bar contamination and index not use problem
| selected_label_object = label_object.copy() | ||
| selected_label_object[selected_label_object != label] = 0 |
| f"{feature_name}-{grayscale}-{distance}" | ||
| ) | ||
| output_texture_dict["texture_value"].append(haralick_mean[i]) | ||
| gc.collect() |
| - [x] Implement module | ||
|
|
||
| 6. PR 6: Intensity module and tests | ||
| 1. PR 6: Intensity module and tests |
| - [x] Implement module | ||
|
|
||
| 7. PR 7: Granularity module and tests | ||
| 1. PR 7: Granularity module and tests |
| - [x] Implement module | ||
|
|
||
| 8. PR 8: Neighbors module and tests | ||
| 1. PR 8: Neighbors module and tests |
| - [x] Implement module | ||
|
|
||
| 9. PR 9: Texture module and tests | ||
| 1. PR 9: Texture module and tests |
| import numpy | ||
| import tqdm | ||
|
|
||
| from zedprofiler.IO.loading_classes import ObjectLoader |
| """Placeholder for texture computation implementation.""" | ||
| raise ZedProfilerError("texture.compute is not implemented yet") | ||
|
|
||
| def scale_image(image: numpy.ndarray, num_gray_levels: int = 256) -> numpy.ndarray: |
There was a problem hiding this comment.
would suggest skimage.exposure.rescale_intensity (docs) over custom function with dtype control and potentially better edge case handling.
| return image | ||
|
|
||
|
|
||
| def compute_texture( |
There was a problem hiding this comment.
I an guessing you are intentionally avoiding using the CP measure texture suite for the sake of better integration with the cytomining ecosystem, correct me if i am assuming too much. Maybe some comments explaining design philosophy would help?
I noted some potential differences/potential performance bottlenecks comparing the two, please see comments below, see what can be borrowed from CP measure to benefit your performance perhaps?
Also I wonder if the crop object with mask and intensity image -> preprocess function (bit rescale here) -> featurization is a recurring functionality that could be abstracted as its own module and shared between different featurization suite?
| distance=distance, | ||
| compute_14th_feature=False, | ||
| ) | ||
| haralick_mean = haralick_features.mean(axis=0) |
There was a problem hiding this comment.
Seemed like CP measure preserves directionality with
n_directions = 13 if pixels.ndim > 2 else 4
...
features[:, :, index] = mahotas.features.haralick(
label_data, distance=scale, ignore_zeros=True
)does averaging out help us or hurt us here?
| "texture_value": [], | ||
| } | ||
| for index, label in tqdm.tqdm(enumerate(labels)): | ||
| selected_label_object = label_object.copy() |
There was a problem hiding this comment.
this seemed really expensive with bigger 3d images and more objects, any way to not copy it?
| z_indices, y_indices, x_indices = numpy.where(object_voxels) | ||
| min_z, max_z = numpy.min(z_indices), numpy.max(z_indices) | ||
| min_y, max_y = numpy.min(y_indices), numpy.max(y_indices) | ||
| min_x, max_x = numpy.min(x_indices), numpy.max(x_indices) |
There was a problem hiding this comment.
This may also be expensive. Not entirely sure if the CP measure code does same or is actually different performing voxel bounding coordinates extraction prior to featurization but they use props = skimage.measure.regionprops(masks, pixels) can this be integrated here for efficiency?
| "texture_name": [], | ||
| "texture_value": [], | ||
| } | ||
| for index, label in tqdm.tqdm(enumerate(labels)): |
There was a problem hiding this comment.
concur on progress bar contamination and index not use problem
| f"{feature_name}-{grayscale}-{distance}" | ||
| ) | ||
| output_texture_dict["texture_value"].append(haralick_mean[i]) | ||
| gc.collect() |
| image_object = object_loader.image[ | ||
| min_z : max_z + 1, min_y : max_y + 1, min_x : max_x + 1 | ||
| ].copy() |
There was a problem hiding this comment.
object_loader isn't really an object loader here as you are doing the object extraction explicitly off the image getter, more like just plain data loader?
Description
This PR adds the texture module. Tests will fail due to not having the image loading functions present.
What kind of change(s) are included?
Checklist
Please ensure that all boxes are checked before indicating that this pull request is ready for review.