Skip to content

Added a Widget for drawing region of interest in napari. feature addition disscussed in #378 #489

Closed
harsh-bhanushali-05 wants to merge 9 commits intoneuroinformatics-unit:mainfrom
harsh-bhanushali-05:roi_layer
Closed

Added a Widget for drawing region of interest in napari. feature addition disscussed in #378 #489
harsh-bhanushali-05 wants to merge 9 commits intoneuroinformatics-unit:mainfrom
harsh-bhanushali-05:roi_layer

Conversation

@harsh-bhanushali-05
Copy link
Copy Markdown
Contributor

@harsh-bhanushali-05 harsh-bhanushali-05 commented Mar 13, 2025

Before submitting a pull request (PR), please read the contributing guide.

Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Added a Few ScreenShots to give everyone idea of what i have implemented.
Why is this PR needed?
This is regarding the feature request in issue #378 , Adding a widget in order to draw region of interest.
The widget adds a layer where user can draw using the mouse using various shapes.
What does this PR do?
This PR introduces a dedicated widget for creating and managing Regions of Interest (ROIs) within napari, designed for movement analysis workflows.
Screenshot 2025-03-12 201435
Screenshot 2025-03-12 210455

References

Please reference any existing issues/PRs that relate to this PR.

How has this PR been tested?

Used the feature on local system trying to break it with all the edges cases.
Wrote test case as well to check the working of the widget.

Please explain how any new code has been tested, and how you have ensured that no existing functionality has changed.

Is this a breaking change?

If this PR breaks any existing functionality, please explain how and why.
No its a feature addition only

Does this PR require an update to the documentation?

It might require as there is entirely new menu on the home screen.
If any features have changed, or have been added. Please explain how the
documentation has been updated.

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit
    Screenshot 2025-03-13 152741

@niksirbi
Copy link
Copy Markdown
Member

Thanks for starting work on this @harsh-bhanushali-05, this is a feature much requested and anticipated by our users.
I think you've made a solid start. One of us will take a closer look at your PR later, but meanwhile try and address the GitHub actions failures

There are a few changes done in addition to the earlier implementation
@harsh-bhanushali-05
Copy link
Copy Markdown
Contributor Author

Tried solving the test cases which were failing in the before commit.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.87%. Comparing base (e199c86) to head (e48f11b).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #489   +/-   ##
=======================================
  Coverage   99.87%   99.87%           
=======================================
  Files          28       29    +1     
  Lines        1568     1640   +72     
=======================================
+ Hits         1566     1638   +72     
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sonarqubecloud
Copy link
Copy Markdown

@niksirbi niksirbi self-requested a review March 24, 2025 11:49
Copy link
Copy Markdown
Member

@niksirbi niksirbi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harsh-bhanushali-05 — thanks for your work on this, and for following our contributing guidelines and implementing the feature using the general patterns we've established for widget creation and testing.

This PR lays a solid foundation, and it's great to see you engaging with the project. That said, there are a few key areas where the implementation doesn’t quite align with what we’re aiming for — particularly regarding the new UI interactions:

  • You've added a button to create a new ROI layer, but this doesn’t offer much beyond what’s already available through napari's built-in “New shapes layer” button. Additionally, there’s a bug: if you delete an existing ROI layer and then click your “Create ROI Layer” button, it incorrectly reports that an ROI layer already exists — likely because the reference to self.roi_layer isn’t being cleared properly.
  • You’ve introduced a right-click interaction to select shapes, which is a clever idea — but napari already provides a set of tools and shortcuts for selecting, editing, and deleting shapes. Overriding or duplicating this built-in behaviour can cause confusion and potential conflicts.

The broader point here is that a plugin like ours should extend napari’s functionality, rather than modify or replicate behaviour that’s already well-supported by the base application.

That might not have been clear from the issue description — so that’s on us! What we actually need is:

  • A way for users to assign a name to each created ROI, which napari doesn’t support out of the box.
  • Functionality to convert shapes drawn in napari into movement ROI objects (e.g. PolygonOfInterest or LineOfInterest).
  • A way to save these ROIs — along with their names — to file, so they can be reused or shared across sessions.

These are the key features we’re hoping to support, and your work here could be a great starting point for that.

I’ll go ahead and close this PR for now, but if you’re interested in continuing, please feel free to open a new draft PR that focuses on any of the above tasks. Happy to support and provide guidance as you go.

Thanks again for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants