feat: add reading skew from czi files #1205
Conversation
Reviewer's Guide by SourceryThis pull request adds functionality to read and handle skew information from CZI files, and incorporates this information into the image processing pipeline. The changes primarily affect the image reading process, data structures, and how the image data is passed to layers in the visualization. Sequence diagram for reading and applying shear from CZI filessequenceDiagram
participant Reader as ImageReader
participant Metadata as Metadata
participant Image as Image
participant Layers as Layers
Reader->>Metadata: _read_shear(metadata)
Metadata->>Reader: return shear
Reader->>Image: create Image with shear
Image->>Layers: pass shear to layers
Layers->>Layers: apply shear to image layers
Updated class diagram for Image classclassDiagram
class Image {
+list[ChannelInfo | ChannelInfoFull] channel_info
+str axes_order
+Spacing shift
+np.ndarray shear
+str name
+dict metadata_dict
+np.ndarray | None shear()
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe changes include modifications to the Changes
Possibly related PRs
Poem
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 (
|
![sourcery-ai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Czaki - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def project_to_layers(project_info: typing.Union[ProjectTuple, MaskProjectTuple]): | ||
| res_layers = [] | ||
| if project_info.image is not None and not isinstance(project_info.image, str): | ||
| scale = project_info.image.normalized_scaling() |
There was a problem hiding this comment.
issue (code-quality): Extract code out into function (extract-method)
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/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 (3)
package/PartSegImage/image.py (1)
Line range hint
227-247: LGTM! Consider adding parameter documentation.The addition of the
shearparameter to the__init__method is well-implemented. The new parameter is correctly added to the method signature and assigned to the_shearinstance variable.Consider adding a docstring or comment to explain the purpose and expected type of the
shearparameter for better code documentation.package/PartSegImage/image_reader.py (2)
415-416: Adjuststacklevelinwarnings.warnfor correct warning location.Using
stacklevel=1inwarnings.warnmay not display the warning at the expected location in the call stack. Settingstacklevel=2will show the warning as originating from the caller of_read_skew, providing clearer context.Apply this diff to adjust the
stacklevel:if not shear_value.startswith("Skew"): - warnings.warn(f"Unknown shear value {shear_value}", stacklevel=1) + warnings.warn(f"Unknown shear value {shear_value}", stacklevel=2) continue
395-421: Add docstrings to new methods for better documentation.Including docstrings for the
_read_shearand_read_skewmethods will improve code readability and maintainability by explaining their purpose and usage.Apply this diff to add docstrings:
def _read_shear(self, metadata: dict): + """ + Compute the shear transformation matrix based on skew values extracted from metadata. + + Args: + metadata (dict): The metadata dictionary containing image information. + + Returns: + numpy.ndarray: A shear transformation matrix. + """ skew = self._read_skew(metadata) shear = np.diag([1.0] * len(skew)) for i, val in enumerate(skew): if val == 0: continue + if i + 1 < len(skew): shear[i, i + 1] = np.tan(np.radians(val)) return shear @staticmethod def _read_skew(metadata: dict): + """ + Extract skew values from the metadata for each dimension (T, Z, Y, X). + + Args: + metadata (dict): The metadata dictionary containing image information. + + Returns: + List[float]: A list of skew angles for each dimension. + """ dimensions = metadata["ImageDocument"]["Metadata"]["Information"]["Image"]["Dimensions"] res = [0.0] * 4 for i, dim in enumerate("TZYX"): if dim not in dimensions: continue if f"{dim}AxisShear" not in dimensions[dim]: continue shear_value = dimensions[dim][f"{dim}AxisShear"] if not shear_value.startswith("Skew"): warnings.warn(f"Unknown shear value {shear_value}", stacklevel=2) continue + try: res[i] = float(shear_value[4:]) + except ValueError: + warnings.warn(f"Invalid shear value '{shear_value}' cannot be converted to float.", stacklevel=2) + res[i] = 0.0 + continue return res
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- package/PartSegCore/napari_plugins/loader.py (5 hunks)
- package/PartSegImage/image.py (3 hunks)
- package/PartSegImage/image_reader.py (2 hunks)
🧰 Additional context used
🔇 Additional comments (9)
package/PartSegImage/image.py (2)
374-377: LGTM! Well-implemented property.The
shearproperty is correctly implemented as a read-only property, providing access to the_shearinstance variable. The return type annotation is accurate and includes the possibility of returningNone.
Line range hint
227-377: Verify the usage of theshearattribute in the codebase.The
shearattribute has been added to theImageclass, but it's not yet used within the class itself. To ensure this addition is fully utilized:
- Check if there are any methods in this class that should be updated to use the
shearattribute.- Verify if any subclasses or external code need to be updated to take advantage of this new attribute.
To help verify the usage of the
shearattribute, you can run the following script:This script will help identify any current usage of 'shear' and places where the
Imageclass is used or extended, which might need updates to incorporate the newshearattribute.✅ Verification successful
Shear Attribute Usage Verified
The
shearattribute added to theImageclass is actively used in other parts of the codebase, includingloader.pyandimage_reader.py. This confirms that the addition is utilized as intended.
- Files using
shear:
package/PartSegCore/napari_plugins/loader.pypackage/PartSegImage/image_reader.py🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential usage or updates needed for the new 'shear' attribute # Search for 'shear' usage in Python files echo "Searching for 'shear' usage in Python files:" rg --type python 'shear' -C 3 # Search for 'Image' class usage echo "\nSearching for 'Image' class usage:" rg --type python 'class\s+\w+\(Image\)' -C 3 # Search for 'Image' instantiation echo "\nSearching for 'Image' instantiation:" rg --type python 'Image\(' -C 3Length of output: 490
Script:
#!/bin/bash # Description: Check for potential usage or updates needed for the new 'shear' attribute # Search for 'shear' usage in Python files echo "Searching for 'shear' usage in Python files:" rg --type py 'shear' -C 3 # Search for 'Image' class usage echo "\nSearching for 'Image' class usage:" rg --type py 'class\s+\w+\(Image\)' -C 3 # Search for 'Image' instantiation echo "\nSearching for 'Image' instantiation:" rg --type py 'Image\(' -C 3Length of output: 103085
package/PartSegCore/napari_plugins/loader.py (5)
60-60: Verify thatshearis accepted in labels layer propertiesYou've added
'shear': shearto the layer properties for a labels layer. Ensure that the labels layer in napari accepts theshearparameter and that it behaves as expected. Not all parameters are supported across different layer types.
74-74: Confirmshearparameter compatibility with image layersIncluding
'shear': shearin the properties of image layers is a significant change. Make sure that the image layer in napari supports theshearparameter and that it correctly applies the shear transformation to the displayed image.
97-97: Checkshearusage in ROI labels layerThe addition of
'shear': shearto the ROI labels layer properties may affect how ROIs are rendered. Verify that including theshearparameter is appropriate for labels layers representing ROIs and that it doesn't introduce any rendering issues.
110-110: Ensureshearis correctly applied to alternative ROI layersFor alternative ROIs, adding the
shearparameter should be done cautiously. Confirm that each alternative ROI layer correctly interprets theshearparameter and that it doesn't cause any unintended side effects.
121-121: Verifyshearparameter in Mask layer propertiesIncluding
'shear': shearin the Mask layer properties is a meaningful update. Ensure that the Mask layer supports theshearparameter and that it functions as intended without causing any errors.package/PartSegImage/image_reader.py (2)
4-4: Importing 'warnings' module is appropriate.The 'warnings' module is required for issuing warnings in the
_read_skewmethod.
392-392: Adding 'shear' parameter to 'Image' object is appropriate.Including the 'shear' parameter enhances the
CziImageReaderto account for shear transformations in images.
|
|
||
|
|
||
| def _image_to_layers(project_info, scale, translate): | ||
| def _image_to_layers(project_info, scale, translate, shear): |
There was a problem hiding this comment.
Ensure 'shear' parameter is compatible with all targeted napari versions
The addition of the shear parameter to the _image_to_layers function enhances functionality. However, it's important to verify that the shear parameter is supported in all versions of napari that your application targets. Using unsupported parameters could lead to runtime errors in older versions of napari.
Consider adding a version check, similar to the one used for the add_color function, to conditionally include the shear parameter based on the napari version.
| res_layers = [] | ||
| if project_info.image is not None and not isinstance(project_info.image, str): | ||
| scale = project_info.image.normalized_scaling() | ||
| shear = project_info.image.shear |
There was a problem hiding this comment.
Handle potential None values for shear
When assigning shear = project_info.image.shear, there's a possibility that project_info.image.shear might be None or not defined, especially if the image doesn't have shear information. Passing None to layer properties could cause unexpected behavior or errors.
Consider adding a check to handle None values or provide a default shear value to ensure robust code execution.
Apply this diff to handle possible None values:
- shear = project_info.image.shear
+ shear = project_info.image.shear if project_info.image.shear is not None else np.zeros((ndim, ndim))Replace ndim with the appropriate number of dimensions for your shear matrix.
Committable suggestion was skipped due to low confidence.
| if not shear_value.startswith("Skew"): | ||
| warnings.warn(f"Unknown shear value {shear_value}", stacklevel=1) | ||
| continue | ||
| res[i] = float(shear_value[4:]) |
There was a problem hiding this comment.
Handle potential ValueError when converting shear value to float.
Converting shear_value[4:] to float may raise a ValueError if the substring cannot be converted to a number. Adding error handling will prevent the program from crashing in such cases.
Apply this diff to add a try-except block:
if not shear_value.startswith("Skew"):
warnings.warn(f"Unknown shear value {shear_value}", stacklevel=2)
continue
+try:
res[i] = float(shear_value[4:])
+except ValueError:
+ warnings.warn(f"Invalid shear value '{shear_value}' cannot be converted to float.", stacklevel=2)
+ res[i] = 0.0
continue📝 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.
| if not shear_value.startswith("Skew"): | |
| warnings.warn(f"Unknown shear value {shear_value}", stacklevel=1) | |
| continue | |
| res[i] = float(shear_value[4:]) | |
| if not shear_value.startswith("Skew"): | |
| warnings.warn(f"Unknown shear value {shear_value}", stacklevel=2) | |
| continue | |
| try: | |
| res[i] = float(shear_value[4:]) | |
| except ValueError: | |
| warnings.warn(f"Invalid shear value '{shear_value}' cannot be converted to float.", stacklevel=2) | |
| res[i] = 0.0 | |
| continue |
| def _read_shear(self, metadata: dict): | ||
| skew = self._read_skew(metadata) | ||
| shear = np.diag([1.0] * len(skew)) | ||
| for i, val in enumerate(skew): | ||
| if val == 0: | ||
| continue | ||
| shear[i, i + 1] = np.tan(np.radians(val)) | ||
| return shear | ||
|
|
There was a problem hiding this comment.
Potential IndexError in _read_shear due to out-of-bounds access.
In the _read_shear method, the loop may attempt to access shear[i, i + 1] when i + 1 equals the size of the matrix, resulting in an IndexError. This occurs when i reaches the last index.
Apply this diff to prevent the IndexError by adjusting the loop:
def _read_shear(self, metadata: dict):
skew = self._read_skew(metadata)
shear = np.diag([1.0] * len(skew))
- for i, val in enumerate(skew):
+ for i, val in enumerate(skew[:-1]):
if val == 0:
continue
shear[i, i + 1] = np.tan(np.radians(val))
return shearAlternatively, add a boundary check within the loop:
def _read_shear(self, metadata: dict):
skew = self._read_skew(metadata)
shear = np.diag([1.0] * len(skew))
for i, val in enumerate(skew):
if val == 0:
continue
+ if i + 1 >= len(skew):
+ continue
shear[i, i + 1] = np.tan(np.radians(val))
return shear📝 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.
| def _read_shear(self, metadata: dict): | |
| skew = self._read_skew(metadata) | |
| shear = np.diag([1.0] * len(skew)) | |
| for i, val in enumerate(skew): | |
| if val == 0: | |
| continue | |
| shear[i, i + 1] = np.tan(np.radians(val)) | |
| return shear | |
| def _read_shear(self, metadata: dict): | |
| skew = self._read_skew(metadata) | |
| shear = np.diag([1.0] * len(skew)) | |
| for i, val in enumerate(skew[:-1]): | |
| if val == 0: | |
| continue | |
| shear[i, i + 1] = np.tan(np.radians(val)) | |
| return shear |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1205 +/- ##
===========================================
- Coverage 93.15% 93.12% -0.03%
===========================================
Files 210 210
Lines 33169 33198 +29
===========================================
+ Hits 30898 30916 +18
- Misses 2271 2282 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
package/PartSegImage/image.py (2)
227-227: LGTM! Consider adding docstring for the newshearparameter.The addition of the
shearparameter is well-implemented. It allows for optional shear transformations to be specified for the image.Consider adding a brief description of the
shearparameter in the method's docstring to improve documentation.Also applies to: 247-247
374-377: LGTM! Consider adding a docstring to theshearproperty.The
shearproperty is correctly implemented, providing read-only access to the_shearattribute.Consider adding a brief docstring to the
shearproperty to describe its purpose and return value.package/PartSegImage/image_reader.py (2)
396-404: Add a docstring to the_read_shearmethod for better documentation.Including a descriptive docstring will clarify the purpose and functionality of the
_read_shearmethod, improving code maintainability.
405-422: Add a docstring to the_read_skewmethod for enhanced clarity.Providing a docstring for the
_read_skewmethod will help others understand its role in extracting skew values from metadata.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- package/PartSegCore/napari_plugins/loader.py (5 hunks)
- package/PartSegImage/image.py (3 hunks)
- package/PartSegImage/image_reader.py (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package/PartSegCore/napari_plugins/loader.py
🧰 Additional context used
🔇 Additional comments (2)
package/PartSegImage/image_reader.py (2)
4-4: Importing thewarningsmodule is appropriate and necessary.The addition of
import warningsallows the code to issue warnings for unexpected shear values, enhancing error reporting.
393-393: Inclusion of theshearparameter enhances image metadata handling.Passing the
shearparameter to theImageclass ensures that shear transformations are properly incorporated into the image object.
Summary by Sourcery
Introduce the ability to read shear values from CZI file metadata and apply these transformations to images, enhancing the image processing capabilities by accounting for skew.
New Features:
Enhancements:
_image_to_layersandproject_to_layersfunctions to include shear transformations in the image layer metadata.Summary by CodeRabbit
New Features
shearparameter for enhanced image layer construction and color adjustments.CziImageReaderclass.Bug Fixes
adjust_colorfunction, ensuring better compatibility with different formats.Documentation