Update labels videos list on replace #128
Conversation
WalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
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: 1
🧹 Outside diff range and nitpick comments (1)
sleap_io/model/labels.py (1)
496-497: Effective update ofself.videoslistThe new implementation correctly updates the
self.videoslist using thevideo_map. This ensures that all video references are properly maintained after the replacement process.Consider this minor optimization for clarity and potential performance improvement:
- self.videos = [video_map.get(video, video) for video in self.videos] + self.videos = list(map(lambda v: video_map.get(v, v), self.videos))This change uses
mapandlambda, which can be slightly more efficient for larger lists and may be considered more idiomatic in some Python circles.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- sleap_io/model/labels.py (2 hunks)
- tests/model/test_labels.py (1 hunks)
🔇 Additional comments (3)
sleap_io/model/labels.py (2)
476-482: Improved flexibility inreplace_videosmethodThis new conditional block enhances the
replace_videosmethod by allowing users to provide onlynew_videoswithout specifyingold_videos. It automatically assignsself.videostoold_videoswhen the number ofnew_videosmatches the existing videos. This change improves the method's usability while maintaining data integrity.
Line range hint
476-497: Summary ofreplace_videosmethod improvementsThe changes to the
replace_videosmethod in theLabelsclass effectively address the PR objective. The method now correctly updates theLabels.videoslist and handles cases where onlynew_videosare provided. These improvements enhance the flexibility and robustness of the video replacement process.Key improvements:
- Automatic assignment of
old_videoswhen onlynew_videosare provided.- Proper updating of the
self.videoslist after processing labeled frames and suggestions.These changes will likely improve the overall functionality and user experience when working with video replacements in the SLEAP-IO library.
tests/model/test_labels.py (1)
359-363: Test correctly verifies video replacement functionalityThe test ensures that after calling
labels.replace_videos, allLabeledFrameandSuggestionFrameobjects have theirvideo.filenameupdated to'fake.mp4', confirming thatreplace_videosworks as intended.
|
|
||
| for lf in labels: | ||
| assert lf.video.filename == "fake.mp4" | ||
|
|
||
| for sf in labels.suggestions: | ||
| assert sf.video.filename == "fake.mp4" | ||
|
|
||
| assert labels.video.filename == "fake.mp4" |
There was a problem hiding this comment.
Ensure single video access is valid after replacement
Accessing labels.video will raise a ValueError if labels.videos contains more than one video. Since replace_videos replaces the videos, consider verifying that only one video is present before asserting labels.video.filename.
Apply this change to confirm only one video exists:
assert len(labels.videos) == 1
assert labels.video.filename == "fake.mp4"
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #128 +/- ##
=======================================
Coverage 96.38% 96.38%
=======================================
Files 15 15
Lines 2046 2049 +3
=======================================
+ Hits 1972 1975 +3
Misses 74 74 ☔ View full report in Codecov by Sentry. |
Fix:
Labels.replace_videosnow correctly updates theLabels.videoslist in addition to frame and suggestion references.Summary by CodeRabbit
New Features
Bug Fixes
Tests