Fix duplicate skeletons during labels merge #2075
Conversation
WalkthroughThis pull request introduces modifications to the SLEAP (Social Leap) application's data handling, focusing on improvements in the Changes
Sequence DiagramsequenceDiagram
participant Labels
participant Skeletons
participant Nodes
participant Tracks
Labels->>Skeletons: Check if empty
alt Skeletons list is empty
Labels->>Skeletons: Update skeletons
end
Labels->>Nodes: Check if empty
alt Nodes list is empty
Labels->>Nodes: Construct from skeletons
end
Labels->>Tracks: Check if empty
alt Tracks list is empty
Labels->>Tracks: Update tracks
end
Labels->>Labels: Merge matching components
Possibly related PRs
Suggested Labels
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
sleap/io/dataset.py (1)
486-503: Nested loops may add skeletons repeatedly or hamper performance.This triple-nested loop can re-check skeleton matches an excessive number of times. Once a match has been found, consider breaking early to avoid redundant checks. Additionally, partial matches across multiple frames might make merges fail or run slower than necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
sleap/io/dataset.py(4 hunks)tests/gui/test_commands.py(1 hunks)tests/io/test_dataset.py(0 hunks)
💤 Files with no reviewable changes (1)
- tests/io/test_dataset.py
🔇 Additional comments (1)
tests/gui/test_commands.py (1)
73-73: Change in the number of expected tracks from 3 to 2.
This updated assertion likely reflects the new logic that merges or removes duplicates. Confirm that the new expectation accurately represents the final track count after the improved merging routine.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2075 +/- ##
===========================================
+ Coverage 75.43% 75.59% +0.15%
===========================================
Files 134 134
Lines 24749 24437 -312
===========================================
- Hits 18670 18473 -197
+ Misses 6079 5964 -115 ☔ View full report in Codecov by Sentry. |
Description
This PR fixes the duplicate skeleton issue when merging labels file. After every update to the labels file, we check if there's a existing skeleton that matches with a new Skeleton associated with an instance in the Labeled frame. If the skeleton doesn't match, then we add it to the list of skeletons in the Labels object.
Types of changes
Does this address any currently open issues?
#2025, #1090, #713
Outside contributors checklist
Thank you for contributing to SLEAP!
❤️
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores