Skip to content

Comments

Pre-release cleanup for PySide6 GUI#54

Open
C-Achard wants to merge 24 commits intocy/update-installfrom
cy/pre-release-cleanup
Open

Pre-release cleanup for PySide6 GUI#54
C-Achard wants to merge 24 commits intocy/update-installfrom
cy/pre-release-cleanup

Conversation

@C-Achard
Copy link

@C-Achard C-Achard commented Feb 20, 2026

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.

  • NEW :
    • Started minimal cleanup of camera scanning workers to have a more centralized and robust state logic + updated tests accordingly.
    • CI changes to properly install OpenGL.
    • Cancel ongoing checks (tests) on commits to PRs to avoid unnecessary runs

NOTE

The Engine enum should be imported from dlclive itself once available.


Camera Scan Workflow Improvements

  • Refactored the camera scan state management in CameraConfigDialog to use a new CameraScanState enum, 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]
  • Improved scan cancellation and error handling: The scan can now be canceled more reliably, with clear UI feedback for cancellation and errors. The scan worker emits a canceled signal, and the dialog ensures UI stability before finishing a scan, including adding placeholder items to the camera list on cancel/error. [1] [2]
  • Updated the camera scan worker (DetectCamerasWorker) to always emit results (even if empty) and to emit a canceled signal when interrupted, allowing the UI to deterministically stabilize regardless of scan outcome.

CI and Workflow Adjustments

  • Added installation of Qt/OpenGL runtime dependencies in the CI workflow for Ubuntu to ensure tests run reliably in headless environments.
  • Restricted Codecov upload in CI to only occur on pushes to the main branch, avoiding unnecessary uploads.

Logging Level Adjustments

  • Changed most logger statements from info to debug in dlclivegui/cameras/backends/opencv_backend.py and dlclivegui/gui/camera_config/camera_config_dialog.py to 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.py to prepare for release and removed deprecated status message code. [1] [2]

Model Backend Detection

  • Added the Engine enum and methods in dlclivegui/temp/engine.py to determine backend type (TensorFlow or PyTorch) from model path or type.
  • Integrated the new backend detection logic in dlclivegui/services/dlc_processor.py and updated dlclivegui/gui/main_window.py to use it instead of hardcoding model type. [1] [2] [3]

Code Cleanup and Comment Updates

  • Fixed typos and clarified comments in dlclivegui/gui/main_window.py and test files, and updated test notes regarding random failures. [1] [2]

@C-Achard C-Achard self-assigned this Feb 20, 2026
@C-Achard C-Achard added the enhancement New feature or request label Feb 20, 2026
@C-Achard C-Achard requested a review from Copilot February 20, 2026 15:17
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 infodebug) and removed per-module logger level overrides.
  • Added a temporary Engine enum 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.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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).
@C-Achard C-Achard force-pushed the cy/pre-release-cleanup branch from 09b689c to d8da888 Compare February 23, 2026 08:51
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.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.
@C-Achard C-Achard requested a review from Copilot February 23, 2026 11:01
@C-Achard C-Achard marked this pull request as ready for review February 23, 2026 11:03
@C-Achard C-Achard requested a review from deruyter92 February 23, 2026 11:03
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.
Copy link

@deruyter92 deruyter92 left a comment

Choose a reason for hiding this comment

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

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.
@deruyter92 deruyter92 self-requested a review February 24, 2026 15:54
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants