Skip to content

Conversation

strixy16
Copy link
Collaborator

@strixy16 strixy16 commented Sep 24, 2025

This aligns with med-imagetools functionality

Summary by CodeRabbit

  • New Features

    • Cropping accepts resize dimensions as a single value, list, or array; centroid cropping uses a resolved centroid dimension (defaults to 50 when unspecified).
  • Bug Fixes

    • Added validation and explicit error reporting for inconsistent resize lists provided for centroid cropping.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 24, 2025

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • pixi.lock is excluded by !**/*.lock

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

crop_and_resize_image_and_mask now accepts resize_dimension as int, list[int], np.ndarray, or None. Added numpy import. For centroid cropping, list/array inputs are validated and collapsed to a single integer (or raise on mismatch); None defaults centroid dimension to 50 and the value is passed to RegionBox.from_mask_centroid.

Changes

Cohort / File(s) Summary
Image cropping: resize_dimension type expansion and centroid handling
src/readii/process/images/crop.py
Expanded resize_dimension type to int | list[int] | np.ndarray | None; added numpy import. Centroid path validates list/ndarray consistency, collapses identical values to a single int, raises ValueError on mismatch, and uses the resolved centroid dimension with RegionBox.from_mask_centroid. None defaults to 50 for centroid cropping.

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)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibble code like clover sprigs—so neat!
Lists or arrays, I tap my feet.
Centroid’s size? Now clear and tight,
One true measure, set just right.
Hop, crop, resize—what a view! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly and accurately captures the main change of enabling the crop and resize function to accept list inputs, aligns with the PR objectives, and avoids unnecessary detail while using conventional commit style.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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.

@codecov
Copy link

codecov bot commented Sep 24, 2025

Codecov Report

❌ Patch coverage is 45.45455% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.74%. Comparing base (2d88148) to head (cb34889).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/readii/process/images/crop.py 45.45% 6 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 = None

And 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

📥 Commits

Reviewing files that changed from the base of the PR and between cca215f and 25992ad.

⛔ 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".

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 = None

Also 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

📥 Commits

Reviewing files that changed from the base of the PR and between 25992ad and a7c5e63.

📒 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
@strixy16 strixy16 merged commit 69a3993 into main Sep 24, 2025
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant