Skip to content

Fix hard-coded overwrite kwarg in pytorch analyze_videos - #3514

Merged
deruyter92 merged 8 commits into
devfrom
cy/fix-overwrite-torch-kwarg
Sep 14, 2026
Merged

deruyter92 merged 8 commits into
devfrom
cy/fix-overwrite-torch-kwarg

Conversation

@C-Achard

Copy link
Copy Markdown
Collaborator

Fixes what appears to be hard-coded args from the original torch API, removing overwrite=False and adding an upstream argument and docstring to the relevant function.

Fixed for:

  • Video analysis
  • Tracklet generation

Closes #3513.

Expose an `overwrite` option in the compatibility `analyze_videos` wrapper, document its PyTorch-only behavior, and pass it through so callers can re-run analysis even when prediction files already exist.
To avoid silently re-running on stale outputs when overwrite is true

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.

🟡 Changes recommended

The public signature breaks trailing positional arguments, and the fix lacks regression coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes PyTorch video re-analysis by propagating the public overwrite option through analysis and tracklet generation.

Changes:

  • Adds overwrite to the compatibility API.
  • Forwards it to PyTorch analysis and automatic tracking.
File summaries
File Description
deeplabcut/compat.py Exposes and forwards overwrite.
deeplabcut/pose_estimation_pytorch/apis/videos.py Propagates overwrite to tracklet generation.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread deeplabcut/compat.py Outdated
Comment thread deeplabcut/compat.py
C-Achard and others added 4 commits September 14, 2026 10:04
This change adds a regression test to ensure the compatibility wrapper passes the `overwrite` flag through to the PyTorch `analyze_videos` API. It fixes the bug where the flag was effectively pinned to `False` when using `Engine.PYTORCH`.

@deruyter92 deruyter92 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

good fix! Works as intended.

I pushed a small change to the docstring + some minor improvements to the regression test. Let me know what you think @C-Achard.

Side note: compat.py is removed in #3382 I will verify using the regression test that the fix is still in place.

@deruyter92
deruyter92 marked this pull request as ready for review September 14, 2026 15:13
@C-Achard C-Achard added the 3.0.2 label Sep 14, 2026
@C-Achard

Copy link
Copy Markdown
Collaborator Author

I pushed a small change to the docstring + some minor improvements to the regression test.

Amazing, thanks!

@deruyter92
deruyter92 merged commit 60998ef into dev Sep 14, 2026
43 of 46 checks passed
@deruyter92
deruyter92 deleted the cy/fix-overwrite-torch-kwarg branch September 14, 2026 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hardcode bug on analyze_videos() for overwrite arg

3 participants