fix: Drop imagej LUTs of size 24*x
#1227
Conversation
Reviewer's Guide by SourceryThis PR fixes a bug in handling ImageJ LUTs (Look-Up Tables) where they were previously written using 64 bits instead of 8 bits. The implementation adds validation for LUT data and adjusts color handling in the napari plugin loader. Class diagram for Image Reader LUT HandlingclassDiagram
class ImageReader {
+read_resolution_from_tags(image_file)
+read_imagej_metadata(image_file)
+_read_imagej_colors(image_file)
}
ImageReader : +_read_imagej_colors(image_file)
ImageReader : +read_imagej_metadata(image_file)
ImageReader : -colors
ImageReader : +colors = _read_imagej_colors(image_file)
note for ImageReader "_read_imagej_colors is a new static method to handle LUTs validation."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughThe pull request introduces changes to the Changes
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (1)package/tests/test_PartSegImage/test_image_writer.py (1)
The change from Let's verify that this is the only test affected by the LUT changes: ✅ Verification successfulThe change is correct, but there are related test cases that need attention The verification revealed two important findings:
The current change in
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Search for other tests that might need similar updates
# Look for tests using default_coloring or ImageJ-related color handling
# Search for tests using default_coloring
rg -l "default_coloring" "package/tests"
# Search for ImageJ-related color tests
rg -l "imagej.*color" "package/tests"
Length of output: 228 Script: #!/bin/bash
# Let's examine the content of test_image.py to see if it needs similar updates
rg "default_coloring" "package/tests/test_PartSegImage/test_image.py" -B 2 -A 2
# Also check for any other ImageJ color-related assertions in test_image_writer.py
rg "assert.*color" "package/tests/test_PartSegImage/test_image_writer.py" -B 2 -A 2
Length of output: 2656 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 (
|
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: 1 issue found
- 🟢 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.
| @@ -30,7 +30,7 @@ def adjust_color(color: typing.Union[str, list[int]]) -> typing.Union[str, list[ | |||
| # case when color is in format #RGBA | |||
| return color[:4] | |||
| elif isinstance(color, list): | |||
| return [color[i] / 255 for i in range(3)] | |||
| return (color[i] / 255 for i in range(3)) | |||
There was a problem hiding this comment.
issue (bug_risk): Convert generator expression to tuple explicitly
The generator expression should be converted to a tuple explicitly using tuple(). Currently, it returns a generator that can only be iterated once, which could cause issues if the caller needs to use the values multiple times.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1227 +/- ##
===========================================
- Coverage 93.14% 93.12% -0.03%
===========================================
Files 210 210
Lines 33172 33178 +6
===========================================
- Hits 30899 30897 -2
- Misses 2273 2281 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
In the past, The PartSeg wrote LUTs using 64 bits instead of 8 bits. I do not know how to decode it, so better drop it.
Summary by Sourcery
Fix the handling of LUTs in PartSeg by dropping those with 64-bit integers and refactor the adjust_color function to return a tuple instead of a list.
Bug Fixes:
Enhancements:
Summary by CodeRabbit
New Features
Bug Fixes
Tests