Pre-release cleanup for PySide6 GUI#54
Conversation
There was a problem hiding this comment.
Pull request overview
This PR prepares the PySide6 GUI for release by reducing logging verbosity and introducing a temporary “engine/backend detection” layer that derives the model backend from a model path, while also removing deprecated GUI behavior and cleaning up comments.
Changes:
- Reduced logging verbosity (mostly
info→debug) and removed per-module logger level overrides. - Added a temporary
Engineenum with helpers to infer backend type from model path/model type, and integrated it into DLC settings creation. - Removed deprecated status-bar behavior and updated a few comments/test notes.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
dlclivegui/cameras/backends/opencv_backend.py |
Removes forced logger level and reduces verbosity for device/resolution logging. |
dlclivegui/gui/camera_config/camera_config_dialog.py |
Removes forced logger level and reduces preview restart logging verbosity. |
dlclivegui/gui/camera_config/loaders.py |
Removes forced logger level to rely on centralized logging configuration. |
dlclivegui/processors/dlc_processor_socket.py |
Removes forced logger level to avoid overriding application-level logging decisions. |
dlclivegui/gui/main_window.py |
Removes debug basicConfig and switches DLC model type from hardcoded to backend auto-detection. |
dlclivegui/services/dlc_processor.py |
Imports and exposes backend detection via DLCLiveProcessor.get_model_backend(). |
dlclivegui/temp/engine.py |
Adds Engine enum and model path/type inference helpers (temporary module). |
tests/services/test_dlc_processor.py |
Updates test comments regarding flaky behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove explicit logger.setLevel / basicConfig calls that force debug/info levels in several modules so the application-wide logging configuration can control log levels (preparing for release). Affected files: dlclivegui/cameras/backends/opencv_backend.py, dlclivegui/gui/camera_config/camera_config_dialog.py, dlclivegui/gui/camera_config/loaders.py, dlclivegui/gui/main_window.py, dlclivegui/processors/dlc_processor_socket.py. This avoids forcing verbose output and prevents accidental global logging reconfiguration.
Lowered several log statements from INFO to DEBUG in the OpenCV camera backend and camera config preview to reduce noisy logs during normal operation. Removed the deprecated auto-loaded myconfig.json statusBar message in the main window. Also clarified a closeEvent comment and updated a test comment to mark a flakiness note as addressed.
Replace the hardcoded 'pytorch' model_type with a value inferred from the selected model file. main_window.py now reads the model_path into a local variable and uses DLCLiveProcessor.get_model_backend(model_path) when building DLCProcessorSettings. dlc_processor.py imports Engine from dlclive and adds a static get_model_backend method that returns Engine.from_model_path(model_path).value. Preserves existing config fields and error handling.
Introduce a temporary dlclivegui.temp package with an Engine enum to detect model engines. engine.py provides Engine.TENSORFLOW and Engine.PYTORCH plus helpers from_model_type and from_model_path (checks extensions/pose_cfg.yaml and existence). Add __init__.py and update dlclivegui/services/dlc_processor.py to import Engine from dlclivegui.temp (with a TODO to switch to the upstream dlclive package when available).
Introduce a temporary Engine enum and expose it via dlclivegui.temp to standardize model engine types. Update DLCProcessorSettings to use the Engine enum for model_type. Improve model backend detection in the main window by calling DLCLiveProcessor.get_model_backend and raising a clear error if detection fails; tighten file dialog filters to focus on PyTorch models. Enhance engine detection to recognize .pth files. Minor import cleanup in dlc_processor and a clarifying test comment about timeouts to reduce flakiness in CI.
Switch DLCProcessorSettings.model_type default from a raw string to the Engine enum (Engine.PYTORCH) and change DLCLiveProcessor.get_model_backend to return an Engine instead of a string for stronger typing. Update the GUI to validate empty model path input, reuse the resolved model backend (model_bknd) when building settings, improve the backend-detection error message, and allow selecting any file/directory in the model file dialog (adjusted name filter label for TensorFlow model directories). Also fix a relative import in dlclivegui.temp.__init__.py. Note: this changes the get_model_backend return type (callers expecting a string must use .value if needed).
Adapt tests to recent refactors and improve CI reliability: - tests/gui/test_ascii_art.py: use LOGO_ALPHA instead of ASCII_IMAGE_PATH, switch color arg from "always" to "auto", and relax the non-TTY assertion to check for the description substring. - tests/gui/test_main.py: make test_start_inference_emits_pose use tmp_path to create a dummy_model.pt file and set the model path to that file so the existence check passes. - tox.ini: set QT_OPENGL=software to reduce OpenGL/CI flakiness, and comment out the [testenv:lint] section (also remove lint from the py312 env mapping). These changes fix tests broken by the logo variable rename and improve test stability in CI environments.
Stop importing and exporting GUI and multi-camera controller symbols at package level to avoid import-time side effects. Removed imports of CameraConfigDialog, DLCLiveMainWindow, MultiCameraController and MultiFrameData from dlclivegui.__init__ and removed those names from __all__. The top-level export list keeps "main" (callers should import GUI/controller types from their modules when needed).
09b689c to
d8da888
Compare
Install Qt/OpenGL runtime dependencies on Ubuntu CI runners (libegl1, libgl1, libopengl0, libxkbcommon-x11-0, libxcb-cursor0) so tests that require OpenGL/Qt can run in the workflow. Also restrict the Codecov upload step to push events on refs/heads/main to avoid uploading coverage for other event types or branches.
Introduce explicit scan state and tidy worker APIs for camera discovery. - Add CameraScanState enum and use it as single source of truth in CameraConfigDialog (_scan_state). - Replace ad-hoc worker checks with _set_scan_state, _finish_scan and _cleanup_scan_worker to manage UI overlays, progress/cancel controls and stability guarantees. - Add request_scan_cancel() to request interruption and handle canceled/result flows; DetectCamerasWorker now emits result (even empty) and canceled when interrupted. - Simplify QThread usage: remove custom finished signals, add typing annotations and clearer run() signatures in loaders; adjust worker signals (canceled) and small API refinements (request_cancel return types). - Update UI hookup (ui_blocks) to call request_scan_cancel instead of private handler. - Update unit and e2e tests to follow new scan lifecycle (wait for scan_started/scan_finished, helper _run_scan_and_wait, and scan state assertions). Files changed: camera_config_dialog.py, loaders.py, ui_blocks.py and related tests to match the new scan lifecycle and worker behaviour.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Allow Codecov uploads for pushes and pull requests targeting main by loosening the workflow condition. Export the package's main entry (from .main import main) in dlclivegui.__init__ so the CLI/function is exposed. Add tox and tox-gh-actions to the test extras in pyproject.toml. Tidy tox.ini by adding a comment that linting is handled by pre-commit/format workflow and removing an optional tox-gh-actions helper section.
Coerce DLCProcessorSettings.model_type to a lowercase string (accepting Enum or string inputs) and validate allowed backends (pytorch, tensorflow). Update UI to handle TensorFlow .pb models by using the parent directory for DLCLive, restrict file dialog to existing files, add existence checks and backend detection when selecting a model. Improve ModelPathStore: robust path normalization, separate helpers for existing file/dir checks, smarter save/load/resolve logic, and better start-dir/suggest-file heuristics. Minor cleanup: remove duplicate import and clarify a TF model detection comment in engine.
Update tests in tests/utils/test_settings_store.py to exercise ModelPathStore normalization more robustly: use tmp_path to create real directories and files, assert _norm_existing_dir and _norm_existing_path return existing absolute paths (and that they are dir/file respectively), and handle None via _norm_existing_* helpers. Also adjust suggest_start_dir test to make cwd invalid by monkeypatching Path.cwd so the fallback to home is exercised. Removed the previous unreliable expanduser assertion.
Introduce a concurrency block to the testing CI workflow so runs for the same PR are grouped and previous in-progress jobs are cancelled when a new update is pushed. The group is keyed by workflow and pull request number (ci-${{ github.workflow }}-pr-${{ github.event.pull_request.number }}) to avoid redundant CI runs and save resources.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Introduce Engine helpers (is_pytorch_model_path, is_tensorflow_model_dir_path) and centralize model-file/TF-dir detection. Replace the old is_model_file usage with these helpers across settings_store and main_window, fixing a bug in suffix checking and ensuring .pb TensorFlow models are validated via their parent directory. Remove legacy is_model_file from utils. Also update camera UI to expect dlg.request_scan_cancel instead of _on_scan_cancel, refine scan-cancel test synchronization, and add/adjust unit tests to cover the new Engine detection logic.
Add sender() checks and debug logs to scan-related handlers so signals from old/stale scan workers are ignored. Update _is_scan_running to also consider the worker's isRunning() state. Only call _finish_scan("cancel") when there is no active worker to avoid prematurely finishing cancellation. These changes reduce race conditions when restarting/canceling camera scans and ensure UI reflects the current worker's state.
Update tests/utils/test_utils.py to import Engine from dlclivegui.temp instead of dlclivegui.temp.engine. Aligns the test import with the package re-export or module relocation so tests reference the correct top-level import.
deruyter92
left a comment
There was a problem hiding this comment.
In general, looks good. After fixing the bug and addressing the question about the allowed model types, I think it can be merged.
Ensure model_check_path is set for non-.pb selections before calling DLCLiveProcessor.get_model_backend. .pb files still use the parent directory (TensorFlow expectation); other file types now pass the file path itself to avoid using an undefined or incorrect path.
Introduce a ModelType Literal type ("pytorch" | "tensorflow") and change DLCProcessorSettings.model_type from a plain str to this constrained type, keeping the default as "pytorch". This ensures pydantic validation enforces allowed model backends.
Set a static version (2.0.0rc1) in pyproject.toml and remove the dynamic setuptools configuration that read dlclivegui.__version__. Also remove the placeholder __version__ from dlclivegui/__init__.py so version metadata is centralized in pyproject.toml.
This pull request primarily refactors logging levels and introduces a temporary mechanism for determining model backend types from model paths, improving maintainability and flexibility.
It also removes deprecated or unnecessary code and clarifies comments.
NOTE
The Engine enum should be imported from dlclive itself once available.
Camera Scan Workflow Improvements
CameraConfigDialogto use a newCameraScanStateenum, ensuring a single source of truth for scan-related UI controls and making scan state transitions explicit and robust. This includes new methods for setting scan state, cleaning up workers, and finalizing scans. [1] [2]canceledsignal, and the dialog ensures UI stability before finishing a scan, including adding placeholder items to the camera list on cancel/error. [1] [2]DetectCamerasWorker) to always emit results (even if empty) and to emit acanceledsignal when interrupted, allowing the UI to deterministically stabilize regardless of scan outcome.CI and Workflow Adjustments
mainbranch, avoiding unnecessary uploads.Logging Level Adjustments
Changed most logger statements from
infotodebugindlclivegui/cameras/backends/opencv_backend.pyanddlclivegui/gui/camera_config/camera_config_dialog.pyto reduce log verbosity in production. Removed explicit logger level settings from several files. [1] [2] [3] [4] [5] [6] [7] [8]Updated logging configuration in
dlclivegui/gui/main_window.pyto prepare for release and removed deprecated status message code. [1] [2]Model Backend Detection
Engineenum and methods indlclivegui/temp/engine.pyto determine backend type (TensorFlow or PyTorch) from model path or type.dlclivegui/services/dlc_processor.pyand updateddlclivegui/gui/main_window.pyto use it instead of hardcoding model type. [1] [2] [3]Code Cleanup and Comment Updates
dlclivegui/gui/main_window.pyand test files, and updated test notes regarding random failures. [1] [2]