-
Notifications
You must be signed in to change notification settings - Fork 0
feat: update crop and resize function to work with list of values #136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Walkthroughcrop_and_resize_image_and_mask now accepts Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Cropper as crop_and_resize_image_and_mask
participant Region as RegionBox
participant SITK as SimpleITK
Caller->>Cropper: image, mask, label, crop_method, resize_dimension
alt crop_method == "centroid"
Note over Cropper: Validate resize_dimension\n- None → default 50\n- list/ndarray → all equal?\n- mismatch → error
Cropper->>Region: from_mask_centroid(mask, label, centroid_dimension)
Region-->>Cropper: region_box
Cropper->>SITK: Extract + Resize using region_box, resize_dimension
SITK-->>Cropper: cropped_image, cropped_mask
else other crop methods
Cropper->>SITK: Existing crop/resize flow (unchanged)
SITK-->>Cropper: cropped_image, cropped_mask
end
Cropper-->>Caller: (cropped_image, cropped_mask)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #136 +/- ##
==========================================
- Coverage 75.93% 75.74% -0.19%
==========================================
Files 41 41
Lines 2011 2020 +9
==========================================
+ Hits 1527 1530 +3
- Misses 484 490 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/readii/process/images/crop.py (4)
17-18
: Signature broadened: consider accepting tuples and update docstring.Typical callers pass tuples for sizes. Also align the docstring param section with the new types.
Apply:
- resize_dimension: int | list[int] | np.ndarray | None = None + resize_dimension: int | list[int] | tuple[int, ...] | np.ndarray | None = NoneAnd update the docstring (see lines 31-33 suggestion below).
97-104
: Normalize numpy arrays before calling resize().Coerce np.ndarray to list/tuple to match common size expectations and avoid surprises.
Apply:
- if resize_dimension is not None: - # Resize and resample the cropped image with linear interpolation to desired dimensions - cropped_image = resize(cropped_image, size = resize_dimension, interpolation = 'linear') + if resize_dimension is not None: + # Normalize size arg for downstream APIs + size_arg = resize_dimension.tolist() if isinstance(resize_dimension, np.ndarray) else resize_dimension + # Resize and resample the cropped image with linear interpolation to desired dimensions + cropped_image = resize(cropped_image, size=size_arg, interpolation='linear') # Resize and resample the cropped mask with nearest neighbor interpolation to desired dimensions # This can end up being returned as a float32 image, so need to cast to uint8 to avoid issues with label values - cropped_mask = resize(cropped_mask, size = resize_dimension, interpolation = 'nearest') + cropped_mask = resize(cropped_mask, size=size_arg, interpolation='nearest')
31-33
: Docstring: align parameter name and types.Reflect accepted types and singular parameter name.
Apply:
- resize_dimensions : int, optional - Dimension to resize the image to. Will apply this value in all dimensions, so result will be a cube. + resize_dimension : int | list[int] | tuple[int, ...] | np.ndarray, optional + Target size for resizing. If an int, the value is applied to all dimensions (cube). + If a sequence/array, its length should match the image dimensionality.
50-52
: Docstring: correct module path in reference.The import is from imgtools.transforms.functional, not imgtools.ops.functional.
Apply:
- using `imgtools.ops.functional.resize` with linear interpolation. + using `imgtools.transforms.functional.resize` with linear interpolation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pixi.lock
is excluded by!**/*.lock
📒 Files selected for processing (1)
src/readii/process/images/crop.py
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Unit-Tests (windows-latest, py312)
- GitHub Check: Unit-Tests (windows-latest, py311)
- GitHub Check: Unit-Tests (macos-14, py312)
- GitHub Check: Unit-Tests (windows-latest, py310)
- GitHub Check: Unit-Tests (ubuntu-latest, py312)
- GitHub Check: Unit-Tests (macos-latest, py311)
- GitHub Check: Unit-Tests (macos-latest, py310)
- GitHub Check: Unit-Tests (macos-latest, py312)
🔇 Additional comments (1)
src/readii/process/images/crop.py (1)
3-3
: Confirmed — numpy is declared in pyproject.toml.
pyproject.toml contains "numpy>=2.2.5,<3".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/readii/process/images/crop.py (4)
17-17
: Type hint: include tuple/Sequence to match runtime behavior.You accept tuples at runtime but the type excludes them. Prefer a Sequence to cover list/tuple.
Apply:
- resize_dimension: int | list[int] | np.ndarray | None = None + resize_dimension: int | Sequence[int] | np.ndarray | None = NoneAlso add import:
-from typing import Literal +from typing import Literal, Sequence
97-101
: Validate centroid_dimension > 0 before use.Add a guard to catch zero/negative sizes early.
Apply:
else: # Single scalar provided centroid_dimension = int(resize_dimension) - # Generate a cube bounding box with resize dimension around the centroid of a mask + # Validate centroid size + if centroid_dimension <= 0: + message = f"centroid_dimension must be > 0; got {centroid_dimension}." + logger.error(message) + raise ValueError(message) + # Generate a cube bounding box with resize dimension around the centroid of a mask crop_box = RegionBox.from_mask_centroid(mask, label, centroid_dimension)
115-121
: Normalize resize_dimension to ints before calling resize.Prevents surprises if a numpy array or mixed numeric types are passed; ensures consistent tuple[int, ...] or int.
Apply:
- if resize_dimension is not None: - # Resize and resample the cropped image with linear interpolation to desired dimensions - cropped_image = resize(cropped_image, size = resize_dimension, interpolation = 'linear') + if resize_dimension is not None: + # Normalize size to int or tuple[int, ...] + if isinstance(resize_dimension, np.ndarray): + resize_size = tuple(int(v) for v in resize_dimension.tolist()) + elif isinstance(resize_dimension, (list, tuple)): + resize_size = tuple(int(v) for v in resize_dimension) + else: + resize_size = int(resize_dimension) + # Resize and resample the cropped image with linear interpolation to desired dimensions + cropped_image = resize(cropped_image, size = resize_size, interpolation = 'linear') # Resize and resample the cropped mask with nearest neighbor interpolation to desired dimensions # This can end up being returned as a float32 image, so need to cast to uint8 to avoid issues with label values - cropped_mask = resize(cropped_mask, size = resize_dimension, interpolation = 'nearest') + cropped_mask = resize(cropped_mask, size = resize_size, interpolation = 'nearest')
31-33
: Docstring: align param name/types and library path.
- Use the correct param name (resize_dimension) and accepted types.
- Fix the reference to the resize function’s module path.
Apply:
- resize_dimensions : int, optional - Dimension to resize the image to. Will apply this value in all dimensions, so result will be a cube. + resize_dimension : int | Sequence[int] | np.ndarray, optional + Target size. If an int, applied to all dimensions (cube). If a sequence/array, must match image dimensionality. @@ - using `imgtools.ops.functional.resize` with linear interpolation. + using `imgtools.transforms.functional.resize` with linear interpolation.Also applies to: 49-52
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/readii/process/images/crop.py
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Unit-Tests (macos-latest, py310)
- GitHub Check: Unit-Tests (windows-latest, py311)
- GitHub Check: Unit-Tests (windows-latest, py312)
- GitHub Check: Unit-Tests (macos-14, py310)
- GitHub Check: Unit-Tests (macos-14, py312)
- GitHub Check: Unit-Tests (windows-latest, py310)
- GitHub Check: Unit-Tests (macos-latest, py311)
- GitHub Check: Unit-Tests (ubuntu-latest, py312)
- GitHub Check: Unit-Tests (macos-latest, py312)
🔇 Additional comments (2)
src/readii/process/images/crop.py (2)
68-101
: Centroid input handling: solid fix.Good: no mutation of the input, ndarray handling added, clear error messages, and collapsing to a single int.
3-3
: No action required — numpy is already declared. pyproject.toml lists "numpy>=2.2.5,<3" (lockfiles and Dockerfile also reference numpy).
…roid dimension This list should never be longer than 3 values, so using set is fine
This aligns with med-imagetools functionality
Summary by CodeRabbit
New Features
Bug Fixes