-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: Add visualization module with environment detection for Jupyter notebooks #223
base: development
Are you sure you want to change the base?
Conversation
In old version, bounding box was a single voxel. Now is expanded to at least the minimum dimension default. - **New Features** - Now, users can optionally specify a desired size when generating image bounding boxes for enhanced control. - **Chores** - Updated the software version from 1.21.1 to 1.22.0.
…method to Scan class for improved string representation
…g and error messages
In old version, bounding box was a single voxel. Now is expanded to at least the minimum dimension default. - **New Features** - Now, users can optionally specify a desired size when generating image bounding boxes for enhanced control. - **Chores** - Updated the software version from 1.21.1 to 1.22.0.
… in an image (#221) This works the same way as `_adjust_negative_coordinates`, but requires the image as an addition input and modifies the RegionBox object directly. A message is logged in the debugger if a dimension is adjusted. I also updated the `crop_image` function to call this before applying the crop.
…notebooks (future feature)
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request modifies the image visualization module. The Changes
FeedbackThe changes are clearly segmented, which aids in maintainability. To further enhance readability and long-term maintainability, consider the following:
Overall, the modular design and explicit export declarations are positive steps toward making the codebase more maintainable. 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
Documentation and Community
|
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 (7)
src/imgtools/vizualize/visualizer.py (5)
33-34
: Consider adding a class-level docstring.While the methods in
ImageSlices
are reasonably documented, a brief class-level docstring would improve clarity about its overall purpose.
37-42
: Clarify dictionary keys in thefrom_dict
method.Here, you convert the
dict[tuple[int, int], Image.Image]
into a sorted list of images, but the significance of thetuple[int, int]
keys is not immediately clear from the docstring. A brief explanation would help future maintainers understand their meaning and sorting logic.
73-75
: Class docstring references mask overlay color incorrectly.The docstring says the mask is overlaid in red, but the
_generate_slice
method actually uses the green channel to display the mask. Please update the docstring or the overlay color to keep them consistent.- """Generates 2D slices from a 3D SimpleITK image and optionally overlays a mask.""" + """Generates 2D slices from a 3D SimpleITK image and optionally overlays a green mask."""
185-186
: Remove or update reference to metadata text.The docstring mentions "metadata text in the bottom right corner," but no text is drawn. You might consider removing this statement or implementing the metadata overlay.
343-349
: Unused variablesz
.The logic here conditionally assigns
sz
based ondim
, but immediately overwrites it withSize3D(new_size, new_size, new_size)
on line 349 without further usage. This might be leftover code.- if dim == 0: - sz = Size3D(new_size, new_size, 0) - elif dim == 1: - sz = Size3D(new_size, 0, new_size) - sz = Size3D(new_size, new_size, new_size)src/imgtools/vizualize/__init__.py (1)
1-14
: Add a brief module-level docstring.A short explanation of the module’s purpose, such as how it ties
utils
andvisualizer
together, will enhance readability and maintainability.src/imgtools/vizualize/utils.py (1)
5-23
: Reflect QtConsole detection in enum usage.The docstring mentions a
JUPYTER_QTCONSOLE
environment type, but the code does not explicitly differentiate QtConsole from standard Jupyter Notebook. Consider adjusting the detection logic or updating the docstring to avoid confusion.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pixi.lock
is excluded by!**/*.lock
and included by nonepixi.toml
is excluded by none and included by none
📒 Files selected for processing (3)
src/imgtools/vizualize/__init__.py
(1 hunks)src/imgtools/vizualize/utils.py
(1 hunks)src/imgtools/vizualize/visualizer.py
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`src/**/*.py`: Review the Python code for compliance with PE...
src/**/*.py
: Review the Python code for compliance with PEP 8 and PEP 257 (docstring conventions). Ensure the following: - Variables and functions follow meaningful naming conventions. - Docstrings are present, accurate, and align with the implementation. - Code is efficient and avoids redundancy while adhering to DRY principles. - Consider suggestions to enhance readability and maintainability. - Highlight any potential performance issues, edge cases, or logical errors. - Ensure all imported libraries are used and necessary.
src/imgtools/vizualize/utils.py
src/imgtools/vizualize/__init__.py
src/imgtools/vizualize/visualizer.py
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: Unit-Tests (windows-latest, py313)
- GitHub Check: Unit-Tests (windows-latest, py312)
- GitHub Check: Unit-Tests (windows-latest, py311)
- GitHub Check: Unit-Tests (windows-latest, py310)
- GitHub Check: Unit-Tests (macos-13, py313)
- GitHub Check: Unit-Tests (macos-13, py312)
- GitHub Check: Unit-Tests (macos-13, py311)
- GitHub Check: Unit-Tests (macos-13, py310)
- GitHub Check: Unit-Tests (macos-latest, py310)
- GitHub Check: Unit-Tests (ubuntu-latest, py313)
- GitHub Check: Unit-Tests (ubuntu-latest, py312)
- GitHub Check: Unit-Tests (ubuntu-latest, py311)
- GitHub Check: Unit-Tests (ubuntu-latest, py310)
🔇 Additional comments (3)
src/imgtools/vizualize/visualizer.py (2)
1-2
: Good top-level docstring.This concise top-level docstring accurately summarizes the file's purpose and functionality.
131-134
: Validate the slice dimension to prevent IndexErrors.While the docstring states the dimension should be 0, 1, or 2, there is no explicit check, so passing invalid values (like 3) would cause out-of-bounds issues. Consider adding a validation check.
+ if dim not in (0, 1, 2): + raise ValueError(f"Invalid dimension {dim}. Must be 0, 1, or 2.")src/imgtools/vizualize/utils.py (1)
5-65
: Good coverage of environment detection logic.The structured approach to detect IPython-related environments and fall back to standard Python is clear and comprehensive. Nicely done!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## development #223 +/- ##
============================================
Coverage 63.33% 63.33%
============================================
Files 52 52
Lines 3758 3758
============================================
Hits 2380 2380
Misses 1378 1378 ☔ View full report in Codecov by Sentry. |
…interaction refactor: update import path for get_modality_metadata function
Summary by CodeRabbit