Conversation
Refactor check_install.py to support both PyTorch and TensorFlow backends and improve temporary file handling. Introduces TMP_DIR, MODELS_FOLDER, and separate run_pytorch_test/run_tensorflow_test helpers; PyTorch now exports and benchmarks an exported .pt checkpoint, TensorFlow model download logic is preserved. Replaces --nodisplay with --display, centralizes video download and assertions, tightens error handling for downloads, and ensures proper cleanup of temporary files. Also updates imports (urllib.error, export_modelzoo_model) and updates backend availability checks to require at least one backend.
Expose a single_animal flag on benchmark_videos (default False) and forward it to the underlying analysis call. This allows benchmarking to run in single-animal mode when using DeepLabCut-live exported models.
Update .github/workflows/testing.yml to run the Model Benchmark Test with --display instead of --nodisplay. This allows dlc-live-test to run tests that require a display (e.g., visual/benchmarking checks) in the CI workflow.
There was a problem hiding this comment.
Pull request overview
This pull request refactors the DLC-Live installation check script to support both TensorFlow and PyTorch backends, while also applying code style improvements to the benchmarking module. The changes enable backend-agnostic testing by detecting available backends and running appropriate tests for each.
Changes:
- Added PyTorch backend support to check_install.py with new
run_pytorch_test()andrun_tensorflow_test()functions - Changed argument parsing from
--nodisplayto--display, reversing the default display behavior (breaking change) - Applied code formatting and import organization improvements throughout benchmark.py
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| dlclive/check_install/check_install.py | Refactored to support both TensorFlow and PyTorch backends with separate test functions, added backend detection loop, and improved error handling |
| dlclive/benchmark.py | Applied code formatting improvements, reorganized imports with TYPE_CHECKING, added single_animal parameter to benchmark_videos, and corrected spacing/line wrapping throughout |
Comments suppressed due to low confidence (1)
dlclive/benchmark.py:45
- This import of module os is redundant, as it was previously imported on line 16.
import os
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Refactor test startup and error handling for check_install and simplify a type-only import. - dlclive/benchmark.py: replace the try/except tensorflow import under TYPE_CHECKING with a direct import (type ignored) to simplify typing logic. - dlclive/check_install/check_install.py: - Defer importing export_modelzoo_model into run_pytorch_test to avoid importing heavy modules unless PyTorch test runs. - Move MODELS_FOLDER.mkdir to after temporary directory creation. - Add a --nodisplay flag and set default for --display to False so CLI can explicitly disable display. - Comment out resize parameters in test calls and remove an unnecessary model_dir.exists() assertion. - Wrap per-backend test runs in try/except, collect backend failures, allow other backends to continue, and raise an aggregated RuntimeError if all backend tests fail. These changes improve robustness when some backends fail or are unavailable and reduce unnecessary imports during initial checks.
sneakers-the-rat
left a comment
There was a problem hiding this comment.
looks good! some small optional notes.
Re: the formatting comment in the PR body, I see ruff in the dev dependencies, but we don't have a CI linter/formatter check action set up for this. I think it would probably be worth just setting up a set of ruff rules and a ruff CI action to do formatting all in one go and then not have to worry about it again in any further PRs.
Rename MODELS_FOLDER to MODELS_DIR and update references (TORCH_CONFIG checkpoint and TF_MODEL_DIR) for clearer naming. Change missing-backend errors from NotImplementedError to ImportError to better reflect installation issues. Simplify main(): consolidate arg parsing, consistently create TMP_DIR and MODELS_DIR, and add backend_results tracking to report per-backend SUCCESS/ERROR statuses with a printed summary. Improve error recording for backend failures and adjust cleanup check when removing the temporary directory.
|
Thanks for the review @sneakers-the-rat ! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This pull request refactors and enhances the benchmarking and installation check scripts for DLC-Live, with a primary focus on supporting both TensorFlow and PyTorch backends in the installation check. The changes improve code structure, add backend-agnostic testing, and include various code style and clarity improvements.
Major enhancements to installation check and backend support
dlclive/check_install/check_install.pyto add support for testing both TensorFlow and PyTorch backends, including backend detection, model export/download, and backend-specific benchmarking via newrun_pytorch_testandrun_tensorflow_testfunctions. The script now iterates over available backends and runs tests for each, with improved error handling and clearer output. [1] [2] [3]Code style, clarity, and maintainability improvements
dlclive/benchmark.pyfor better readability, including argument formatting, function signatures, and block structure. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15]TYPE_CHECKINGandnoqacomments to avoid unnecessary runtime imports and linter warnings. [1] [2] [3] [4]single_animalinbenchmark_videos). [1] [2]These changes make the benchmarking and installation check scripts more robust, maintainable, and compatible with multiple deep learning backends.