feat: new Negative Controls implementation using strategies and manager #74
Conversation
…ies and region handling
WalkthroughThe pull request includes modifications to several configuration and source files. The Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #74 +/- ##
===========================================
- Coverage 60.92% 48.96% -11.97%
===========================================
Files 10 15 +5
Lines 540 674 +134
===========================================
+ Hits 329 330 +1
- Misses 211 344 +133 ☔ View full report in Codecov by Sentry. |
|
Just tested out a new region (idk if dilation is the right way) but it works pretty straightforward class NonROIWithBorderRegion(RegionStrategy):
"""Region strategy to apply control outside the ROI, including a 2-pixel border."""
region_name = "non_roi_with_border"
# Add a 2-pixel border around the ROI
padding = 2
def __call__(self, image_array: np.ndarray, mask_array: np.ndarray) -> np.ndarray:
"""
Apply the region mask to the image array, adding a border of 2 pixels around the ROI.
Parameters
----------
image_array : np.ndarray
The base image as a numpy array.
mask_array : np.ndarray
The binary mask defining the ROI.
Returns
-------
np.ndarray
A binary mask with 1s outside the ROI and its 2-pixel border.
"""
# Ensure mask_array is binary (0s and 1s)
binary_mask = (mask_array > 0).astype(np.uint8)
roi_with_border = binary_dilation(binary_mask, iterations=self.padding)
# Create the inverse mask for everything outside the ROI and its border
region_mask = np.where(roi_with_border > 0, 0, 1)
# Validate that there are non-zero pixels in the resulting mask
if not region_mask.any():
msg = "ROI with border mask is all 0s. No pixels outside ROI and its border to apply control."
raise ValueError(msg)
return region_mask
|
| def apply(self, base_image: T, mask: T) -> Iterator[tuple[T, str, str]]: | ||
| """Apply the negative control strategies to the region strategies.""" | ||
| for control_strategy, region_strategy in product( | ||
| self.negative_control_strategies, self.region_strategies | ||
| ): | ||
| yield ( | ||
| control_strategy(base_image, mask, region_strategy), | ||
| control_strategy.name(), | ||
| region_strategy.name(), | ||
| ) |
There was a problem hiding this comment.
I need a bit more documentation explaining how this works. I can see that it's applying the negative controls to the images, but I'm not sure how it's doing it.
|
|
||
| region_name = "full" | ||
|
|
||
| def __call__(self, image_array: np.ndarray, mask_array: np.ndarray) -> np.ndarray: |
There was a problem hiding this comment.
image_array isn't actually used in any of the region strategies. Can it be removed?
Co-authored-by: Katy Scott <k.l.scott16@gmail.com>
… array, not a tuple
…eg-nifti' into jjjermiah/sandbox/neg-nifti
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (8)
src/readii/negative_controls_refactor/negative_controls.py (1)
43-43: Use consistent naming conventions for variable namesVariable names like
imgDimensions,flatArrImage,randNumGen,minVoxelVal, andmaxVoxelValuse mixed camelCase and snake_case styles. For consistency with PEP 8 style guidelines, consider usingsnake_casefor variable names.Also applies to: 46-46, 49-49, 69-69, 72-72, 73-73, 76-76
notebooks/viz_neg_controls.ipynb (1)
223-225: Update deprecated Matplotlib functiontostring_rgbtobuffer_rgbaThe
fig.canvas.tostring_rgb()function is deprecated in Matplotlib 3.8 and will be removed in 3.10. Usefig.canvas.buffer_rgba()instead, and adjust the code to handle the RGBA format.Apply this diff to update the code:
# Convert plot to image - img = np.frombuffer(fig.canvas.tostring_rgb(), dtype=np.uint8) + img = np.frombuffer(fig.canvas.buffer_rgba(), dtype=np.uint8) + img = img.reshape(fig.canvas.get_width_height()[::-1] + (4,)) + img = img[:, :, :3] # Discard the alpha channelsrc/readii/negative_controls_refactor/regions.py (3)
11-15: Consider usingnp.ones_like(mask_array, dtype=bool)for memory efficiencyThe region mask is used for boolean operations, so using a boolean dtype would be more memory efficient than the default float64.
- region_mask = np.ones_like(mask_array) + region_mask = np.ones_like(mask_array, dtype=bool)
23-29: Optimize binary mask creation and validationThe implementation is correct but could be optimized by:
- Using boolean dtype for memory efficiency
- Combining the binary conversion and validation
- region_mask = np.where(mask_array > 0, 1, 0) - if not region_mask.any(): + region_mask = mask_array > 0 + if not np.any(region_mask): msg = "ROI mask is all 0s. No pixels in ROI to apply negative control." raise ValueError(msg) return region_mask
37-43: Optimize binary mask creation and validationSimilar to ROIRegion, the implementation could be optimized for memory efficiency.
- region_mask = np.where(mask_array > 0, 0, 1) - if not region_mask.any(): + region_mask = ~(mask_array > 0) + if not np.any(region_mask): msg = "Non-ROI mask is all 0s. No pixels outside ROI to apply negative control." raise ValueError(msg) return region_maskconfig/ruff.toml (2)
3-5: Remove duplicatecache-direntryThe
cache-dirconfiguration is duplicated.cache-dir = "~/.cache/ruff" -# iteratively adding files to the include list -cache-dir = "~/.cache/ruff" -
111-113: Remove duplicate McCabe complexity ruleThe McCabe complexity rule
"C"is listed twice in theselectlist.- # Check for McCabe complexity - # https://docs.astral.sh/ruff/rules/complex-structure/ - "C",src/readii/feature_extraction.py (1)
Line range hint
9-9: Remove unused import.The
loggerimport fromreadii.utilsis not used in this file.-from readii.utils import logger
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
config/ruff.toml(5 hunks)notebooks/viz_neg_controls.ipynb(1 hunks)pyproject.toml(4 hunks)src/readii/feature_extraction.py(12 hunks)src/readii/negative_controls.py(12 hunks)src/readii/negative_controls_refactor/__init__.py(1 hunks)src/readii/negative_controls_refactor/abstract_classes.py(1 hunks)src/readii/negative_controls_refactor/manager.py(1 hunks)src/readii/negative_controls_refactor/negative_controls.py(1 hunks)src/readii/negative_controls_refactor/regions.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
src/readii/negative_controls.py
9-9: readii.utils.logger imported but unused
Remove unused import: readii.utils.logger
(F401)
src/readii/feature_extraction.py
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-38: SyntaxError: Compound statements are not allowed on the same line as simple statements
38-38: SyntaxError: Expected 'in', found name
38-38: SyntaxError: Expected ':', found name
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-38: SyntaxError: Simple statements must be separated by newlines or semicolons
38-39: SyntaxError: Expected an identifier
41-42: SyntaxError: Expected an expression
43-43: SyntaxError: Unexpected indentation
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Compound statements are not allowed on the same line as simple statements
43-43: SyntaxError: Expected an expression
43-44: SyntaxError: Expected an identifier
45-45: SyntaxError: Unexpected indentation
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
46-46: SyntaxError: Expected a statement
47-47: SyntaxError: Unexpected indentation
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Compound statements are not allowed on the same line as simple statements
47-47: SyntaxError: Expected an expression
47-47: SyntaxError: Expected 'in', found name
47-47: SyntaxError: Expected ':', found name
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Compound statements are not allowed on the same line as simple statements
47-47: SyntaxError: Expected ',', found name
47-48: SyntaxError: Expected an identifier
48-48: SyntaxError: Expected ',', found dedent
49-49: SyntaxError: Unexpected indentation
49-49: SyntaxError: Simple statements must be separated by newlines or semicolons
49-49: SyntaxError: Simple statements must be separated by newlines or semicolons
49-49: SyntaxError: Simple statements must be separated by newlines or semicolons
49-49: SyntaxError: Simple statements must be separated by newlines or semicolons
49-49: SyntaxError: Simple statements must be separated by newlines or semicolons
49-49: SyntaxError: Compound statements are not allowed on the same line as simple statements
49-49: SyntaxError: Expected 'in', found name
49-50: SyntaxError: Expected an identifier
51-51: SyntaxError: Expected a statement
52-53: SyntaxError: Expected an expression
54-54: SyntaxError: Unexpected indentation
54-54: SyntaxError: Simple statements must be separated by newlines or semicolons
54-54: SyntaxError: Simple statements must be separated by newlines or semicolons
54-54: SyntaxError: Simple statements must be separated by newlines or semicolons
54-54: SyntaxError: Compound statements are not allowed on the same line as simple statements
54-54: SyntaxError: Expected ',', found name
54-54: SyntaxError: Expected ',', found name
54-54: SyntaxError: Expected ',', found name
54-54: SyntaxError: Expected ',', found name
54-55: SyntaxError: Expected an identifier
55-55: SyntaxError: Expected ',', found dedent
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-85: SyntaxError: Expected an identifier
86-87: SyntaxError: Expected ',', found newline
87-88: SyntaxError: Expected an expression
89-89: SyntaxError: Unexpected indentation
89-89: SyntaxError: Simple statements must be separated by newlines or semicolons
89-89: SyntaxError: Simple statements must be separated by newlines or semicolons
89-89: SyntaxError: Simple statements must be separated by newlines or semicolons
89-89: SyntaxError: Simple statements must be separated by newlines or semicolons
89-90: SyntaxError: Expected an identifier
90-90: SyntaxError: Expected a statement
91-91: SyntaxError: Unexpected indentation
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Compound statements are not allowed on the same line as simple statements
91-91: SyntaxError: Expected ',', found name
91-92: SyntaxError: Expected an identifier
92-92: SyntaxError: Expected ',', found dedent
93-93: SyntaxError: Unexpected indentation
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Compound statements are not allowed on the same line as simple statements
93-94: SyntaxError: Expected an identifier
94-94: SyntaxError: Expected ',', found dedent
95-95: SyntaxError: Unexpected indentation
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
96-96: SyntaxError: Expected a statement
96-96: SyntaxError: Expected a statement
97-97: SyntaxError: Unexpected indentation
97-97: SyntaxError: Simple statements must be separated by newlines or semicolons
97-97: SyntaxError: Simple statements must be separated by newlines or semicolons
97-97: SyntaxError: Simple statements must be separated by newlines or semicolons
97-97: SyntaxError: Simple statements must be separated by newlines or semicolons
97-97: SyntaxError: Simple statements must be separated by newlines or semicolons
97-97: SyntaxError: Compound statements are not allowed on the same line as simple statements
97-97: SyntaxError: Expected 'in', found name
97-98: SyntaxError: Expected an identifier
99-99: SyntaxError: Expected a statement
100-101: SyntaxError: Expected an expression
102-102: SyntaxError: Unexpected indentation
102-102: SyntaxError: Simple statements must be separated by newlines or semicolons
102-102: SyntaxError: Simple statements must be separated by newlines or semicolons
102-102: SyntaxError: Simple statements must be separated by newlines or semicolons
102-103: SyntaxError: Expected an identifier
103-103: SyntaxError: Expected a statement
124-124: SyntaxError: Simple statements must be separated by newlines or semicolons
124-124: SyntaxError: Simple statements must be separated by newlines or semicolons
124-124: SyntaxError: Simple statements must be separated by newlines or semicolons
124-124: SyntaxError: Simple statements must be separated by newlines or semicolons
124-124: SyntaxError: Compound statements are not allowed on the same line as simple statements
124-124: SyntaxError: Expected 'in', found name
124-124: SyntaxError: Expected ':', found name
124-124: SyntaxError: Simple statements must be separated by newlines or semicolons
124-124: SyntaxError: Simple statements must be separated by newlines or semicolons
124-124: SyntaxError: Simple statements must be separated by newlines or semicolons
124-125: SyntaxError: Expected an identifier
150-150: SyntaxError: Expected an indented block after if statement
470-470: SyntaxError: Expected ',', found name
470-470: SyntaxError: Unparenthesized generator expression cannot be used here
🔇 Additional comments (12)
src/readii/negative_controls_refactor/manager.py (1)
52-61: Provide additional documentation for the apply method
As previously noted, adding more detailed documentation to the apply method would help clarify how negative control and region strategies are combined and applied to images.
src/readii/negative_controls_refactor/regions.py (1)
11-11: Remove unused parameter image_array
The image_array parameter is not used in any of the region strategies.
src/readii/negative_controls_refactor/__init__.py (1)
1-20: LGTM! Well-organized module initialization
The module initialization is clean, well-documented, and follows best practices:
- Clear module docstring
- Organized imports
- Comprehensive
__all__list exporting all necessary components
src/readii/negative_controls.py (4)
29-29: LGTM! Efficient type checking implementation.
The use of a tuple in isinstance check is more efficient than multiple individual checks.
214-215: LGTM! Improved error message handling.
Good practice to store error messages in variables before raising exceptions. This improves maintainability and makes it easier to update error messages.
Also applies to: 227-228
286-287: LGTM! Consistent error message handling.
Follows the same improved pattern of storing error messages in variables before raising exceptions.
Also applies to: 299-300
356-357: LGTM! Consistent error handling pattern.
Maintains the improved pattern of storing error messages in variables before raising exceptions.
Also applies to: 359-360
src/readii/feature_extraction.py (5)
40-54: LGTM! Well-documented function parameters and return type.
The comprehensive docstring clearly describes the function's purpose, parameters, and return value.
🧰 Tools
🪛 Ruff (0.8.2)
41-42: SyntaxError: Expected an expression
43-43: SyntaxError: Unexpected indentation
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Simple statements must be separated by newlines or semicolons
43-43: SyntaxError: Compound statements are not allowed on the same line as simple statements
43-43: SyntaxError: Expected an expression
43-44: SyntaxError: Expected an identifier
45-45: SyntaxError: Unexpected indentation
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
45-45: SyntaxError: Simple statements must be separated by newlines or semicolons
46-46: SyntaxError: Expected a statement
47-47: SyntaxError: Unexpected indentation
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Compound statements are not allowed on the same line as simple statements
47-47: SyntaxError: Expected an expression
47-47: SyntaxError: Expected 'in', found name
47-47: SyntaxError: Expected ':', found name
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Simple statements must be separated by newlines or semicolons
47-47: SyntaxError: Compound statements are not allowed on the same line as simple statements
47-47: SyntaxError: Expected ',', found name
47-48: SyntaxError: Expected an identifier
48-48: SyntaxError: Expected ',', found dedent
49-49: SyntaxError: Unexpected indentation
49-49: SyntaxError: Simple statements must be separated by newlines or semicolons
49-49: SyntaxError: Simple statements must be separated by newlines or semicolons
49-49: SyntaxError: Simple statements must be separated by newlines or semicolons
49-49: SyntaxError: Simple statements must be separated by newlines or semicolons
49-49: SyntaxError: Simple statements must be separated by newlines or semicolons
49-49: SyntaxError: Compound statements are not allowed on the same line as simple statements
49-49: SyntaxError: Expected 'in', found name
49-50: SyntaxError: Expected an identifier
51-51: SyntaxError: Expected a statement
52-53: SyntaxError: Expected an expression
54-54: SyntaxError: Unexpected indentation
54-54: SyntaxError: Simple statements must be separated by newlines or semicolons
54-54: SyntaxError: Simple statements must be separated by newlines or semicolons
54-54: SyntaxError: Simple statements must be separated by newlines or semicolons
54-54: SyntaxError: Compound statements are not allowed on the same line as simple statements
54-54: SyntaxError: Expected ',', found name
54-54: SyntaxError: Expected ',', found name
54-54: SyntaxError: Expected ',', found name
54-54: SyntaxError: Expected ',', found name
80-102: LGTM! Improved type annotation and documentation.
Good improvements:
- Changed
segBoundingBoxtype fromtupletonp.ndarrayfor better type accuracy - Added comprehensive docstring with clear parameter descriptions
🧰 Tools
🪛 Ruff (0.8.2)
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-84: SyntaxError: Expected ',', found name
84-85: SyntaxError: Expected an identifier
86-87: SyntaxError: Expected ',', found newline
87-88: SyntaxError: Expected an expression
89-89: SyntaxError: Unexpected indentation
89-89: SyntaxError: Simple statements must be separated by newlines or semicolons
89-89: SyntaxError: Simple statements must be separated by newlines or semicolons
89-89: SyntaxError: Simple statements must be separated by newlines or semicolons
89-89: SyntaxError: Simple statements must be separated by newlines or semicolons
89-90: SyntaxError: Expected an identifier
90-90: SyntaxError: Expected a statement
91-91: SyntaxError: Unexpected indentation
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Simple statements must be separated by newlines or semicolons
91-91: SyntaxError: Compound statements are not allowed on the same line as simple statements
91-91: SyntaxError: Expected ',', found name
91-92: SyntaxError: Expected an identifier
92-92: SyntaxError: Expected ',', found dedent
93-93: SyntaxError: Unexpected indentation
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Simple statements must be separated by newlines or semicolons
93-93: SyntaxError: Compound statements are not allowed on the same line as simple statements
93-94: SyntaxError: Expected an identifier
94-94: SyntaxError: Expected ',', found dedent
95-95: SyntaxError: Unexpected indentation
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
95-95: SyntaxError: Simple statements must be separated by newlines or semicolons
96-96: SyntaxError: Expected a statement
96-96: SyntaxError: Expected a statement
97-97: SyntaxError: Unexpected indentation
97-97: SyntaxError: Simple statements must be separated by newlines or semicolons
97-97: SyntaxError: Simple statements must be separated by newlines or semicolons
97-97: SyntaxError: Simple statements must be separated by newlines or semicolons
97-97: SyntaxError: Simple statements must be separated by newlines or semicolons
97-97: SyntaxError: Simple statements must be separated by newlines or semicolons
97-97: SyntaxError: Compound statements are not allowed on the same line as simple statements
97-97: SyntaxError: Expected 'in', found name
97-98: SyntaxError: Expected an identifier
99-99: SyntaxError: Expected a statement
100-101: SyntaxError: Expected an expression
102-102: SyntaxError: Unexpected indentation
102-102: SyntaxError: Simple statements must be separated by newlines or semicolons
102-102: SyntaxError: Simple statements must be separated by newlines or semicolons
102-102: SyntaxError: Simple statements must be separated by newlines or semicolons
120-120: LGTM! Improved parameter handling and error checks.
Good improvements:
- Changed default parameter to None for better flexibility
- Added clear error handling for missing parameter files
Also applies to: 149-157
281-281: LGTM! Updated error message for accuracy.
Error message now correctly references 'radiogenomic_output' instead of 'readii_output'.
Line range hint 382-406: LGTM! Enhanced function documentation.
The comprehensive docstring clearly describes the function's purpose, parameters, and return value.
| NEGATIVE_CONTROL_REGISTRY[control_type](random_seed=random_seed) | ||
| for control_type in negative_control_types | ||
| ] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for invalid strategy names
Currently, the code assumes that the provided control_type and region_type exist in the registries. If an invalid name is provided, a KeyError will be raised. Consider adding error handling to provide a more informative error message.
Also applies to: 43-45
| minVoxelVal = np.min(image_array) | ||
| maxVoxelVal = np.max(image_array) | ||
|
|
||
| # Set the random seed for np random generator | ||
| randNumGen = np.random.default_rng(seed=self.random_seed) | ||
|
|
||
| # Generate random array with same dimensions as baseImage with values ranging from the minimum to maximum inclusive of the original image | ||
| random3DArr = randNumGen.integers( | ||
| low=minVoxelVal, high=maxVoxelVal, endpoint=True, size=imgDimensions, | ||
| ) |
There was a problem hiding this comment.
Ensure image_array contains integer values for randNumGen.integers
The randNumGen.integers method requires integer values for the low and high parameters. If image_array contains floating-point values, this could result in an error or unexpected behavior. Consider converting image_array to integers or using randNumGen.uniform for floating-point ranges.
Apply this diff to ensure integer values are used:
def transform(self, image_array: np.ndarray) -> np.ndarray:
"""Randomly generate pixel values."""
# Get array dimensions to reshape back to
- imgDimensions = image_array.shape
+ img_dimensions = image_array.shape
+ # Ensure image array is of integer type
+ image_array = image_array.astype(int)
# Get min and max values to set as range for random values
- minVoxelVal = np.min(image_array)
- maxVoxelVal = np.max(image_array)
+ min_voxel_val = np.min(image_array)
+ max_voxel_val = np.max(image_array)
# Set the random seed for np random generator
- randNumGen = np.random.default_rng(seed=self.random_seed)
+ rand_num_gen = np.random.default_rng(seed=self.random_seed)
# Generate random array with same dimensions as baseImage with values ranging from the minimum to maximum inclusive of the original image
- random3DArr = randNumGen.integers(
- low=minVoxelVal, high=maxVoxelVal, endpoint=True, size=imgDimensions,
+ random_array = rand_num_gen.integers(
+ low=min_voxel_val, high=max_voxel_val, endpoint=True, size=img_dimensions,
)
- return random3DArr
+ return random_array📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| minVoxelVal = np.min(image_array) | |
| maxVoxelVal = np.max(image_array) | |
| # Set the random seed for np random generator | |
| randNumGen = np.random.default_rng(seed=self.random_seed) | |
| # Generate random array with same dimensions as baseImage with values ranging from the minimum to maximum inclusive of the original image | |
| random3DArr = randNumGen.integers( | |
| low=minVoxelVal, high=maxVoxelVal, endpoint=True, size=imgDimensions, | |
| ) | |
| # Ensure image array is of integer type | |
| image_array = image_array.astype(int) | |
| # Get min and max values to set as range for random values | |
| min_voxel_val = np.min(image_array) | |
| max_voxel_val = np.max(image_array) | |
| # Set the random seed for np random generator | |
| rand_num_gen = np.random.default_rng(seed=self.random_seed) | |
| # Generate random array with same dimensions as baseImage with values ranging from the minimum to maximum inclusive of the original image | |
| random_array = rand_num_gen.integers( | |
| low=min_voxel_val, high=max_voxel_val, endpoint=True, size=img_dimensions, | |
| ) |
pyproject.toml
Outdated
| jupyterlab = ">=4.3.1,<5" | ||
| pixi-kernel = ">=0.5.1,<0.6" | ||
| ipykernel = ">=6.29.5,<7" |
There was a problem hiding this comment.
Remove duplicate dependency entries
These dependencies are already declared in lines 64-66:
- jupyterlab
- pixi-kernel
- ipykernel
Apply this diff to remove the duplicates:
-jupyterlab = ">=4.3.1,<5"
-pixi-kernel = ">=0.5.1,<0.6"
-ipykernel = ">=6.29.5,<7"…te lines from merge combination
Negative Controls Refactor Module
Overview
The
negative_controls_refactormodule provides a flexible framework for applying negative controlstrategies to medical imaging data, with a focus on radiomics. This framework allows for targeted
manipulation of images by combining control strategies with specific region definitions. The design
emphasizes modularity, extensibility, and ease of use.
With this module, users can:
Core Concepts
Negative Control Strategies
Negative control strategies define the type of manipulation applied to the image. These include:
Users can also define custom negative control strategies by subclassing the
NegativeControlStrategyabstract base class and implementing the
transformmethod.Region Strategies
Region strategies define where the negative control strategy is applied. These include:
Region strategies are implemented by subclassing
RegionStrategyand overriding the__call__method.How It Works
Workflow
NegativeControlManagerto manage and apply these strategies to an image.The
NegativeControlManagerhandles:Registry System
The module uses registries to map strategy names (strings) to their respective classes:
NEGATIVE_CONTROL_REGISTRY: Maps negative control strategy names (e.g., "shuffled") to classes.REGION_REGISTRY: Maps region strategy names (e.g., "roi") to classes.This design allows for extensibility, as new strategies can be added and registered dynamically.
Example Usage
Using Manager
Use strategies directly
Summary by CodeRabbit
New Features
ShuffledControl,SampledControl, andRandomizedControl.FullRegion,ROIRegion, andNonROIRegion.Improvements
Chores