Skip to content

Conversation

@vtushar06
Copy link
Contributor

@vtushar06 vtushar06 commented Jun 29, 2025

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

This PR addresses issue #573, where setting layer.color_by to a missing feature (e.g., “individual_factorized”) in the Napari Tracks layer raises an error, which can break the user experience during GUI usage.

A fallback mechanism is required to ensure robust, user-friendly behavior, especially when input data or properties are dynamically generated and may not always include the expected column.

What does this PR do?
• Introduces a utility function set_tracks_color_by() in movement/utils/napari_utils.py that:
• Tries to set layer.color_by to the desired feature (e.g., “individual_factorized”)
Falls back to a safe default ("track_id") if the preferred feature is missing
• Optionally shows a message in the Napari overlay (if viewer is passed) to inform the user
• Replaces the direct layer.color_by = ... call in DataLoader._add_tracks_layer() with this new utility for safe assignment
• Adds a unit test test_set_tracks_color_by() under tests/test_unit/test_napari_plugin/test_napari_utils.py to verify:
• Preferred feature is set when available
• Fallback feature is used when preferred is missing
Function behaves without error in both cases

🔗 References
Fixes: #573
• Related discussions: napari color_by PR thread

🧪 How has this PR been tested?
• ✅ Manually tested through GUI (movement launch) using different datasets and missing features
• ✅ Ran unit tests via pytest and verified correct behavior
• ✅ Confirmed with pre-commit that code style, docstrings, and static checks pass
• ✅ Verified fallback message appears in Napari when needed
• ✅ Added tests simulate both success and fallback scenarios

⚠️ Is this a breaking change?
• No
• Yes (please explain):

This is a non-breaking enhancement to improve fault-tolerance when assigning track color styles.

📚 Does this PR require an update to the documentation?
• No
• Yes (please explain):

The new utility function is internal and used within the plugin — it’s fully documented via docstrings, but doesn’t require user-facing documentation updates.

✅ 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

@sonarqubecloud
Copy link

@codecov
Copy link

codecov bot commented Jun 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (6fe3ffc) to head (0032bb0).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #625   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           32        33    +1     
  Lines         1786      1799   +13     
=========================================
+ Hits          1786      1799   +13     

☔ 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.

@sfmig
Copy link
Member

sfmig commented Jun 30, 2025

Hi @vtushar06, thanks for having a go!

Just to clarify, the PR description says this PR addresses an issue "where setting layer.color_by to a missing feature (e.g., individual_factorized) in the Napari Tracks layer raises an error".

But this seems incorrect - setting color_by to a missing feature raises a warning in napari by default, and napari automatically sets the color of the tracks layer to track_id, which is always defined. So I am confused why this PR is needed, if that behaviour already exists in napari. Note that the issue referes to something slightly different.

Issue #573 refers to the warning showing up when the feature / property to color by is defined and the tracks are correctly coloured. So the warning is a false positive warning.

Let me know if this makes it more clear!

@vtushar06
Copy link
Contributor Author

Thanks for the clarification @sfmig! You’re right — napari already falls back to track_id, so this PR doesn’t intend to override that logic. My aim was to catch the situation early and avoid triggering that false positive warning, which can confuse users during GUI usage.
I’ll double-check if the warning is still emitted with this fallback, and update the logic accordingly — possibly suppressing the warning or handling it in a cleaner way. Let me know if you’d prefer a different direction here.

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.

napari warning re color of tracks

3 participants