Fix frame extraction failure with pathlib video selections - #3449
Draft
C-Achard wants to merge 5 commits into
Draft
Fix frame extraction failure with pathlib video selections#3449C-Achard wants to merge 5 commits into
C-Achard wants to merge 5 commits into
Conversation
Improve frame extraction and GUI cropping to match selected videos against config entries using normalized `Path` values instead of raw string equality. This preserves original config keys for updates, prevents silent no-op runs by raising explicit errors when no selected videos match or none are processed, and ensures GUI-selected files are passed as strings while still resolving path-format differences.
Adds a new test module for `generate_training_dataset.frame_extraction` to cover video path normalization behavior. The tests verify that `extract_frames` accepts `Path` objects in `videos_list`, that `_filter_config_videos` correctly matches `str` and `Path` values while preserving original config keys/types, and that edge cases like `None`, non-matching selections, and Windows case-insensitive matching behave as expected.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a regression where frame extraction could silently process zero videos when videos_list contained pathlib.Path objects while config video-set keys were strings (leading to incorrect “corrupted video” reporting), and adds regression coverage.
Changes:
- Add path normalization +
_filter_config_videos()to compare selected videos against config keys while preserving original config keys for lookups. - Improve error handling when no videos are processed / when a selection matches none of the configured videos.
- Update GUI cropping + extraction to match videos via normalized paths and to pass
videos_listas strings; add new tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
deeplabcut/generate_training_dataset/frame_extraction.py |
Adds normalization/filtering helpers and updates extraction logic + empty-selection handling (but currently introduces a mode=="match" filtering bug). |
deeplabcut/gui/tabs/extract_frames.py |
Uses normalized path matching when updating crop entries and ensures videos_list passed to extraction is stringified. |
tests/generate_training_dataset/test_frame_extraction.py |
Adds regression tests for Path-vs-string filtering (but currently contains import/monkeypatch issues). |
Suppressed comments (1)
tests/generate_training_dataset/test_frame_extraction.py:63
- This monkeypatch targets
deeplabcut.utils.io.imsave, butextract_frames()writes viaskimage.io.imsave(imported inside the function). As written, the test will still write PNGs to disk and won’t isolate I/O as intended.
# Avoid writing an actual PNG.
monkeypatch.setattr(io, "imsave", lambda *args, **kwargs: None)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Update `test_frame_extraction.py` to import `io` from `skimage` and remove the incorrect `deeplabcut.utils` `io` import. This aligns the test with the intended image I/O dependency and avoids using the wrong module.
…b.com/DeepLabCut/DeepLabCut into cy/fix-frame-extraction-path-matching
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
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.
Motivation
Frame extraction could process zero videos when
videos_listcontainedPathobjects and configuration keys were strings.This caused valid videos to be incorrectly reported as corrupted.
Closes #3448.
Fix