fix: Second fix for napari repository tests - #1445
Merged
Merged
Conversation
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts napari-based tests and scale bar handling to be compatible with newer napari versions while preserving behavior on older versions. Flow diagram for updated napari scale bar toggling logicflowchart TD
A[toggle_scale_bar] --> B{_napari_le_7_0}
B -- true --> C[viewer.scale_bar.unit = nm]
B -- false --> D[skip unit change]
C --> E{_napari_gt_8_0}
D --> E
E -- true --> F[viewer.canvas.overlays.scale_bar.visible = not viewer.canvas.overlays.scale_bar.visible]
E -- false --> G[viewer.scale_bar.visible = not viewer.scale_bar.visible]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
toggle_scale_bar, consider handling the unit consistently for both napari versions (e.g., when_napari_gt_8_0is true, set the unit onviewer.canvas.overlays.scale_baras well) to avoid version-dependent discrepancies. - The version checks
_napari_le_7_0and_napari_gt_8_0intoggle_scale_barassume mutually exclusive ranges; if there are intermediate versions, you may want a more explicit branching or a fallback to prevent unexpected AttributeErrors. - In
test_image_view_integration_filter, you might wrap theimage_view.show()/hide()calls in a try/finally to ensurehide()is always executed even if the test fails midway.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `toggle_scale_bar`, consider handling the unit consistently for both napari versions (e.g., when `_napari_gt_8_0` is true, set the unit on `viewer.canvas.overlays.scale_bar` as well) to avoid version-dependent discrepancies.
- The version checks `_napari_le_7_0` and `_napari_gt_8_0` in `toggle_scale_bar` assume mutually exclusive ranges; if there are intermediate versions, you may want a more explicit branching or a fallback to prevent unexpected AttributeErrors.
- In `test_image_view_integration_filter`, you might wrap the `image_view.show()`/`hide()` calls in a try/finally to ensure `hide()` is always executed even if the test fails midway.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
📝 WalkthroughWalkthroughThe PR adds version-aware napari scale-bar visibility handling. It also adds type annotations and explicit image-view visibility management to an integration test. ChangesNapari compatibility
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package/PartSeg/common_gui/napari_image_view.py (1)
248-251: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for both scale-bar API branches.
The changed test covers channel filtering, but it does not call
toggle_scale_bar. Add a focused test that verifies visibility toggling throughviewer.scale_barfor legacy napari versions andviewer.canvas.overlays.scale_barfor newer versions. napari 0.8.0 exposesviewer.scale_bar; newer source documents the overlay path from 0.8.1. (github.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package/PartSeg/common_gui/napari_image_view.py` around lines 248 - 251, Add focused regression coverage for toggle_scale_bar, exercising both version branches: verify legacy versions toggle viewer.scale_bar.visible and newer versions toggle viewer.canvas.overlays.scale_bar.visible. Use the existing napari-version branching or fixtures and assert visibility changes in both directions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@package/PartSeg/common_gui/napari_image_view.py`:
- Around line 248-251: Add focused regression coverage for toggle_scale_bar,
exercising both version branches: verify legacy versions toggle
viewer.scale_bar.visible and newer versions toggle
viewer.canvas.overlays.scale_bar.visible. Use the existing napari-version
branching or fixtures and assert visibility changes in both directions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d35f7e9e-5a8c-43b5-868f-e7c1fdbcfa7d
📒 Files selected for processing (2)
package/PartSeg/common_gui/napari_image_view.pypackage/tests/test_PartSeg/test_channel_control.py
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1445 +/- ##
===========================================
- Coverage 92.56% 92.56% -0.01%
===========================================
Files 211 211
Lines 33224 33230 +6
===========================================
+ Hits 30755 30760 +5
- Misses 2469 2470 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #1438
Summary by Sourcery
Adjust napari image view behavior and related tests for compatibility with newer napari versions and stable test execution.
Enhancements:
Tests:
Summary by CodeRabbit