Skip to content

Fix frame extraction failure with pathlib video selections - #3449

Draft
C-Achard wants to merge 5 commits into
mainfrom
cy/fix-frame-extraction-path-matching
Draft

Fix frame extraction failure with pathlib video selections#3449
C-Achard wants to merge 5 commits into
mainfrom
cy/fix-frame-extraction-path-matching

Conversation

@C-Achard

Copy link
Copy Markdown
Collaborator

Motivation

Frame extraction could process zero videos when videos_list contained Path objects and configuration keys were strings.
This caused valid videos to be incorrectly reported as corrupted.

Closes #3448.

Fix

  • Paths are now normalized for comparison while preserving the original config keys
  • Improved the empty selection error handling
  • Added regression/test coverage

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.
@C-Achard
C-Achard requested review from deruyter92 and a lite review from Copilot August 14, 2026 11:33
@C-Achard C-Achard added GUI issues relating to GUI bug fix! fix for a real buggy one... config Related to config.yaml, ruamel, YAML parsing, ... labels Aug 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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_list as 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, but extract_frames() writes via skimage.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.

Comment thread deeplabcut/generate_training_dataset/frame_extraction.py Outdated
Comment thread tests/generate_training_dataset/test_frame_extraction.py Outdated
C-Achard and others added 3 commits August 14, 2026 13:41
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug fix! fix for a real buggy one... config Related to config.yaml, ruamel, YAML parsing, ... GUI issues relating to GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extract frames failing on new and old previously successful projects

3 participants