diff --git a/.github/workflows/testing-ci.yml b/.github/workflows/testing-ci.yml index 1c2f91e..2fa4c5b 100644 --- a/.github/workflows/testing-ci.yml +++ b/.github/workflows/testing-ci.yml @@ -4,6 +4,10 @@ on: pull_request: types: [opened, synchronize, reopened] +concurrency: + group: ci-${{ github.workflow }}-pr-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs: unit: name: Unit + Smoke (no hardware) • ${{ matrix.os }} • py${{ matrix.python }} @@ -38,6 +42,17 @@ jobs: python -m pip install -U pip wheel python -m pip install -U tox tox-gh-actions + - name: Install Qt/OpenGL runtime deps (Ubuntu) + if: startsWith(matrix.os, 'ubuntu') + run: | + sudo apt-get update + sudo apt-get install -y \ + libegl1 \ + libgl1 \ + libopengl0 \ + libxkbcommon-x11-0 \ + libxcb-cursor0 + - name: Run tests (exclude hardware) with coverage via tox run: | tox -q @@ -54,6 +69,7 @@ jobs: echo '```' >> "$GITHUB_STEP_SUMMARY" - name: Upload coverage to Codecov + if: github.event_name == 'pull_request' && (github.base_ref == 'main' || github.base_ref == 'master') uses: codecov/codecov-action@v5 with: files: ./coverage.xml diff --git a/dlclivegui/__init__.py b/dlclivegui/__init__.py index f302f5b..60e5b29 100644 --- a/dlclivegui/__init__.py +++ b/dlclivegui/__init__.py @@ -7,10 +7,7 @@ MultiCameraSettings, RecordingSettings, ) -from .gui.camera_config.camera_config_dialog import CameraConfigDialog -from .gui.main_window import DLCLiveMainWindow from .main import main -from .services.multi_camera_controller import MultiCameraController, MultiFrameData __all__ = [ "ApplicationSettings", @@ -18,10 +15,5 @@ "DLCProcessorSettings", "MultiCameraSettings", "RecordingSettings", - "DLCLiveMainWindow", - "MultiCameraController", - "MultiFrameData", - "CameraConfigDialog", "main", ] -__version__ = "2.0.0rc0" # PLACEHOLDER diff --git a/dlclivegui/cameras/backends/opencv_backend.py b/dlclivegui/cameras/backends/opencv_backend.py index 5a742dc..74fdede 100644 --- a/dlclivegui/cameras/backends/opencv_backend.py +++ b/dlclivegui/cameras/backends/opencv_backend.py @@ -25,7 +25,6 @@ ) logger = logging.getLogger(__name__) -logger.setLevel(logging.DEBUG) # FIXME @C-Achard remove before release if TYPE_CHECKING: from dlclivegui.config import CameraSettings @@ -169,7 +168,7 @@ def open(self) -> None: ns["device_pid"] = int(chosen.pid) if chosen.name: ns["device_name"] = chosen.name - logger.info("Persisted OpenCV device_id=%s", chosen.stable_id) + logger.debug("Persisted OpenCV device_id=%s", chosen.stable_id) self._capture, spec = open_with_fallbacks(index, backend_flag) @@ -399,7 +398,7 @@ def _configure_capture(self) -> None: self._actual_fps = float(self._capture.get(cv2.CAP_PROP_FPS) or 0.0) # For clarity in logs - logger.info("Resolution requested=Auto, actual=%sx%s", self._actual_width, self._actual_height) + logger.debug("Resolution requested=Auto, actual=%sx%s", self._actual_width, self._actual_height) elif not self._fast_start: # Verified, robust path (tries candidates + verifies) @@ -432,7 +431,7 @@ def _configure_capture(self) -> None: if (self._actual_width or 0) > 0 and (self._actual_height or 0) > 0: actual_res = (int(self._actual_width), int(self._actual_height)) - logger.info( + logger.debug( "Resolution requested=%s, actual=%s", f"{req_w}x{req_h}" if (req_w > 0 and req_h > 0) else "Auto", f"{actual_res[0]}x{actual_res[1]}" if actual_res else "unknown", diff --git a/dlclivegui/config.py b/dlclivegui/config.py index 645371c..6d9e1de 100644 --- a/dlclivegui/config.py +++ b/dlclivegui/config.py @@ -2,6 +2,7 @@ from __future__ import annotations import json +from enum import Enum from pathlib import Path from typing import Any, Literal @@ -10,6 +11,7 @@ Rotation = Literal[0, 90, 180, 270] TileLayout = Literal["auto", "2x2", "1x4", "4x1"] Precision = Literal["FP32", "FP16"] +ModelType = Literal["pytorch", "tensorflow"] class CameraSettings(BaseModel): @@ -239,7 +241,7 @@ class DLCProcessorSettings(BaseModel): resize: float = Field(default=1.0, gt=0) precision: Precision = "FP32" additional_options: dict[str, Any] = Field(default_factory=dict) - model_type: Literal["pytorch"] = "pytorch" + model_type: ModelType = "pytorch" single_animal: bool = True @field_validator("dynamic", mode="before") @@ -247,6 +249,38 @@ class DLCProcessorSettings(BaseModel): def _coerce_dynamic(cls, v): return DynamicCropModel.from_tupleish(v) + @field_validator("model_type", mode="before") + @classmethod + def _coerce_model_type(cls, v): + """ + Accept: + - "pytorch"/"tensorflow"/etc as strings + - Enum instances (e.g. Engine.PYTORCH) and store their .value + Always return a lowercase string. + """ + if v is None or v == "": + return "pytorch" + + # If caller passed Engine enum or any Enum, use its value + if isinstance(v, Enum): + v = v.value + + # If caller passed something with a `.value` attribute (defensive) + if not isinstance(v, str) and hasattr(v, "value"): + v = v.value + + if not isinstance(v, str): + raise TypeError(f"model_type must be a string or Enum, got {type(v)!r}") + + v = v.strip().lower() + + # Optional: enforce allowed values + allowed = {"pytorch", "tensorflow"} + if v not in allowed: + raise ValueError(f"Unknown model type: {v!r}. Allowed: {sorted(allowed)}") + + return v + class BoundingBoxSettings(BaseModel): enabled: bool = False diff --git a/dlclivegui/gui/camera_config/camera_config_dialog.py b/dlclivegui/gui/camera_config/camera_config_dialog.py index aba582b..5f2caff 100644 --- a/dlclivegui/gui/camera_config/camera_config_dialog.py +++ b/dlclivegui/gui/camera_config/camera_config_dialog.py @@ -19,12 +19,11 @@ from ...cameras.factory import CameraFactory, DetectedCamera, apply_detected_identity, camera_identity_key from ...config import CameraSettings, MultiCameraSettings -from .loaders import CameraLoadWorker, CameraProbeWorker, DetectCamerasWorker +from .loaders import CameraLoadWorker, CameraProbeWorker, CameraScanState, DetectCamerasWorker from .preview import PreviewSession, PreviewState, apply_crop, apply_rotation, resize_to_fit, to_display_pixmap from .ui_blocks import setup_camera_config_dialog_ui LOGGER = logging.getLogger(__name__) -LOGGER.setLevel(logging.DEBUG) # TODO @C-Achard remove for release class CameraConfigDialog(QDialog): @@ -65,6 +64,7 @@ def __init__( # Camera detection worker self._scan_worker: DetectCamerasWorker | None = None + self._scan_state: CameraScanState = CameraScanState.IDLE # UI elements for eventFilter (assigned in _setup_ui) self._settings_scroll: QScrollArea | None = None @@ -172,7 +172,8 @@ def _on_close_cleanup(self) -> None: pass # Keep this short to reduce UI freeze sw.wait(300) - self._scan_worker = None + self._set_scan_state(CameraScanState.IDLE) + self._cleanup_scan_worker() # Cancel probe worker pw = getattr(self, "_probe_worker", None) @@ -261,7 +262,7 @@ def _connect_signals(self) -> None: self.cancel_btn.clicked.connect(self.reject) self.scan_started.connect(lambda _: setattr(self, "_dialog_active", True)) self.scan_finished.connect(lambda: setattr(self, "_dialog_active", False)) - self.scan_cancel_btn.clicked.connect(self._on_scan_cancel) + self.scan_cancel_btn.clicked.connect(self.request_scan_cancel) def _mark_dirty(*_args): self.apply_settings_btn.setEnabled(True) @@ -313,29 +314,6 @@ def _update_button_states(self) -> None: available_row = self.available_cameras_list.currentRow() self.add_camera_btn.setEnabled(available_row >= 0 and not scan_running) - def _sync_scan_ui(self) -> None: - """ - Sync *scan-related* UI controls based on scan state. - - Conservative policy during scan: - - Allow editing/previewing already configured cameras (Active list) - - Disallow structural changes (add/remove/reorder) and available-list actions - """ - scanning = self._is_scan_running() - - # Discovery controls - self.backend_combo.setEnabled(not scanning) - self.refresh_btn.setEnabled(not scanning) - - # Available camera list + add flow is blocked during scan - self.available_cameras_list.setEnabled(not scanning) - self.add_camera_btn.setEnabled(False if scanning else (self.available_cameras_list.currentRow() >= 0)) - - # Scan cancel button visibility is already managed in your scan start/finish, - # but keeping enabled state here makes it robust. - if hasattr(self, "scan_cancel_btn"): - self.scan_cancel_btn.setEnabled(scanning) - def _sync_preview_ui(self) -> None: """Update buttons/overlays based on preview state only.""" st = self._preview.state @@ -479,93 +457,174 @@ def _on_backend_changed(self, _index: int) -> None: self._refresh_available_cameras() def _is_scan_running(self) -> bool: - return bool(self._scan_worker and self._scan_worker.isRunning()) + if self._scan_state in (CameraScanState.RUNNING, CameraScanState.CANCELING): + return True + w = self._scan_worker + return bool(w and w.isRunning()) + + def _set_scan_state(self, state: CameraScanState, message: str | None = None) -> None: + """Single source of truth for scan-related UI controls.""" + self._scan_state = state + + scanning = state in (CameraScanState.RUNNING, CameraScanState.CANCELING) + + # Overlay message + if scanning: + self._show_scan_overlay( + message or ("Canceling discovery…" if state == CameraScanState.CANCELING else "Discovering cameras…") + ) + else: + self._hide_scan_overlay() + + # Progress + cancel controls + self.scan_progress.setVisible(scanning) + if scanning: + self.scan_progress.setRange(0, 0) # indeterminate + self.scan_cancel_btn.setVisible(scanning) + self.scan_cancel_btn.setEnabled(state == CameraScanState.RUNNING) # disabled while canceling + + # Disable discovery inputs while scanning + self.backend_combo.setEnabled(not scanning) + self.refresh_btn.setEnabled(not scanning) + + # Available list + add flow blocked while scanning (structure edits disallowed) + self.available_cameras_list.setEnabled(not scanning) + self.add_camera_btn.setEnabled(False if scanning else (self.available_cameras_list.currentRow() >= 0)) + + self._update_button_states() + + def _cleanup_scan_worker(self) -> None: + # worker is truly finished now + w = self._scan_worker + self._scan_worker = None + if w is not None: + w.deleteLater() + + def _finish_scan(self, reason: str) -> None: + """Mark scan UX complete (idempotent) and emit scan_finished queued.""" + if self._scan_state in (CameraScanState.DONE, CameraScanState.IDLE): + return + + # Transition scan UX to DONE (UI controls restored) + self._set_scan_state(CameraScanState.DONE) + + QTimer.singleShot(0, self.scan_finished.emit) + + LOGGER.debug("[Scan] finished reason=%s", reason) def _refresh_available_cameras(self) -> None: """Refresh the list of available cameras asynchronously.""" - backend = self.backend_combo.currentData() - if not backend: - backend = self.backend_combo.currentText().split()[0] + backend = self.backend_combo.currentData() or self.backend_combo.currentText().split()[0] - # If already scanning, ignore new requests to avoid races - if getattr(self, "_scan_worker", None) and self._scan_worker.isRunning(): + if self._is_scan_running(): self._show_scan_overlay("Already discovering cameras…") return - # Reset list UI and show progress + # Reset UI/list self.available_cameras_list.clear() self._detected_cameras = [] - msg = f"Discovering {backend} cameras…" - self._show_scan_overlay(msg) - self.scan_progress.setRange(0, 0) - self.scan_progress.setVisible(True) - self.scan_cancel_btn.setVisible(True) - self.available_cameras_list.setEnabled(False) - self.add_camera_btn.setEnabled(False) - self.refresh_btn.setEnabled(False) - self.backend_combo.setEnabled(False) - - self._sync_scan_ui() - self._update_button_states() + + self._set_scan_state(CameraScanState.RUNNING, message=f"Discovering {backend} cameras…") # Start worker - self._scan_worker = DetectCamerasWorker(backend, max_devices=10, parent=self) - self._scan_worker.progress.connect(self._on_scan_progress) - self._scan_worker.result.connect(self._on_scan_result) - self._scan_worker.error.connect(self._on_scan_error) - self._scan_worker.finished.connect(self._on_scan_finished) + w = DetectCamerasWorker(backend, max_devices=10, parent=self) + self._scan_worker = w + + w.progress.connect(self._on_scan_progress) + w.result.connect(self._on_scan_result) + w.error.connect(self._on_scan_error) + w.canceled.connect(self._on_scan_canceled) + + # Cleanup only + w.finished.connect(self._cleanup_scan_worker) + self.scan_started.emit(f"Scanning {backend} cameras…") - self._scan_worker.start() + w.start() def _on_scan_progress(self, msg: str) -> None: + if self.sender() is not self._scan_worker: + LOGGER.debug("[Scan] Ignoring progress from old worker: %s", msg) + return + if self._scan_state not in (CameraScanState.RUNNING, CameraScanState.CANCELING): + return self._show_scan_overlay(msg or "Discovering cameras…") def _on_scan_result(self, cams: list) -> None: + if self.sender() is not self._scan_worker: + LOGGER.debug("[Scan] Ignoring result from old worker: %d cameras", len(cams) if cams else 0) + return + if self._scan_state not in (CameraScanState.RUNNING, CameraScanState.CANCELING): + return + + # Apply results to UI first (stability guarantee) self._detected_cameras = cams or [] - self.available_cameras_list.clear() # replace list contents + self.available_cameras_list.clear() if not self._detected_cameras: placeholder = QListWidgetItem("No cameras detected.") placeholder.setFlags(Qt.ItemIsEnabled) self.available_cameras_list.addItem(placeholder) - return - - for cam in self._detected_cameras: - item = QListWidgetItem(f"{cam.label} (index {cam.index})") - item.setData(Qt.ItemDataRole.UserRole, cam) - self.available_cameras_list.addItem(item) + else: + for cam in self._detected_cameras: + item = QListWidgetItem(f"{cam.label} (index {cam.index})") + item.setData(Qt.ItemDataRole.UserRole, cam) + self.available_cameras_list.addItem(item) + self.available_cameras_list.setCurrentRow(0) - self.available_cameras_list.setCurrentRow(0) + # Now UI is stable: finish scan UX and emit scan_finished queued + self._finish_scan("result") def _on_scan_error(self, msg: str) -> None: + if self.sender() is not self._scan_worker: + LOGGER.debug("[Scan] Ignoring error from old worker: %s", msg) + return + if self._scan_state not in (CameraScanState.RUNNING, CameraScanState.CANCELING): + return + QMessageBox.warning(self, "Camera Scan", f"Failed to detect cameras:\n{msg}") - def _on_scan_finished(self) -> None: - self._hide_scan_overlay() - self.scan_progress.setVisible(False) - self._scan_worker = None + # Ensure UI is stable (list is stable even if empty) before finishing + if self.available_cameras_list.count() == 0: + placeholder = QListWidgetItem("Scan failed.") + placeholder.setFlags(Qt.ItemIsEnabled) + self.available_cameras_list.addItem(placeholder) - self.scan_cancel_btn.setVisible(False) - self.scan_cancel_btn.setEnabled(True) - self.available_cameras_list.setEnabled(True) - self.refresh_btn.setEnabled(True) - self.backend_combo.setEnabled(True) + self._finish_scan("error") - self._sync_scan_ui() - self._update_button_states() - self.scan_finished.emit() + def request_scan_cancel(self) -> None: + if not self._is_scan_running(): + return - def _on_scan_cancel(self) -> None: - """User requested to cancel discovery.""" - if self._scan_worker and self._scan_worker.isRunning(): + self._set_scan_state(CameraScanState.CANCELING, message="Canceling discovery…") + + w = self._scan_worker + if w is not None: try: - self._scan_worker.requestInterruption() + w.requestInterruption() except Exception: pass - # Keep the busy bar, update texts - self._show_scan_overlay("Canceling discovery…") - self.scan_progress.setVisible(True) # stay visible as indeterminate - self.scan_cancel_btn.setEnabled(False) + + # Guarantee UI stability before scan_finished: + if self.available_cameras_list.count() == 0: + placeholder = QListWidgetItem("Scan canceled.") + placeholder.setFlags(Qt.ItemIsEnabled) + self.available_cameras_list.addItem(placeholder) + + if w is None or not w.isRunning(): + self._finish_scan("cancel") + + def _on_scan_canceled(self) -> None: + if self.sender() is not self._scan_worker: + LOGGER.debug("[Scan] Ignoring canceled signal from old worker.") + return + self._set_scan_state(CameraScanState.CANCELING, message="Finalizing cancellation…") + # If cancel is requested without clicking cancel (e.g., dialog closing), ensure UI finishes + if self._scan_state in (CameraScanState.RUNNING, CameraScanState.CANCELING): + if self.available_cameras_list.count() == 0: + placeholder = QListWidgetItem("Scan canceled.") + placeholder.setFlags(Qt.ItemIsEnabled) + self.available_cameras_list.addItem(placeholder) + self._finish_scan("canceled") def _on_available_camera_selected(self, row: int) -> None: if self._scan_worker and self._scan_worker.isRunning(): @@ -1394,7 +1453,7 @@ def _execute_pending_restart(self, *, reason: str) -> None: if not cam: return - LOGGER.info("[Preview] executing restart reason=%s", reason) + LOGGER.debug("[Preview] executing restart reason=%s", reason) self._begin_preview_load(cam, reason="restart") def _cancel_loading(self) -> None: diff --git a/dlclivegui/gui/camera_config/loaders.py b/dlclivegui/gui/camera_config/loaders.py index 6305b8d..e77edf4 100644 --- a/dlclivegui/gui/camera_config/loaders.py +++ b/dlclivegui/gui/camera_config/loaders.py @@ -1,38 +1,57 @@ """Workers and state logic for loading cameras in the GUI.""" -# dlclivegui/gui/camera_loaders.py +# dlclivegui/gui/loaders.py +from __future__ import annotations + import copy import logging +from enum import Enum, auto +from typing import TYPE_CHECKING from PySide6.QtCore import QThread, Signal from PySide6.QtWidgets import QWidget -from ...cameras.base import CameraSettings from ...cameras.factory import CameraBackend, CameraFactory +from ...config import CameraSettings + +if TYPE_CHECKING: + pass # only for typing LOGGER = logging.getLogger(__name__) -LOGGER.setLevel(logging.DEBUG) + + +class CameraScanState(Enum): + IDLE = auto() + RUNNING = auto() + CANCELING = auto() + DONE = auto() # ------------------------------- # Background worker to detect cameras # ------------------------------- class DetectCamerasWorker(QThread): - """Background worker to detect cameras for the selected backend.""" + """Background worker to detect cameras for the selected backend. - progress = Signal(str) # human-readable text - result = Signal(list) # list[DetectedCamera] + Signals: + - progress(str): human-readable status + - result(list): list of DetectedCamera (may be empty) + - error(str): error message (on exception) + - canceled(): emitted if interruption was requested during/after discovery + """ + + progress = Signal(str) + result = Signal(list) # list[DetectedCamera] at runtime error = Signal(str) - finished = Signal() + canceled = Signal() def __init__(self, backend: str, max_devices: int = 10, parent: QWidget | None = None): super().__init__(parent) self.backend = backend self.max_devices = max_devices - def run(self): + def run(self) -> None: try: - # Initial message self.progress.emit(f"Scanning {self.backend} cameras…") cams = CameraFactory.detect_cameras( @@ -41,11 +60,17 @@ def run(self): should_cancel=self.isInterruptionRequested, progress_cb=self.progress.emit, ) - self.result.emit(cams) + + # Always emit result (even if empty) so UI can stabilize deterministically. + self.result.emit(cams or []) + + # If canceled, emit canceled so UI can set ScanState.CANCELING/DONE if desired. + if self.isInterruptionRequested(): + self.canceled.emit() + except Exception as exc: self.error.emit(f"{type(exc).__name__}: {exc}") - finally: - self.finished.emit() + # No custom finished signal: QThread.finished is emitted automatically when run() returns. class CameraProbeWorker(QThread): @@ -54,7 +79,6 @@ class CameraProbeWorker(QThread): progress = Signal(str) success = Signal(object) # emits CameraSettings error = Signal(str) - finished = Signal() def __init__(self, cam: CameraSettings, parent: QWidget | None = None): super().__init__(parent) @@ -67,10 +91,10 @@ def __init__(self, cam: CameraSettings, parent: QWidget | None = None): if isinstance(ns, dict): ns.setdefault("fast_start", True) - def request_cancel(self): + def request_cancel(self) -> None: self._cancel = True - def run(self): + def run(self) -> None: try: self.progress.emit("Probing device defaults…") if self._cancel: @@ -78,8 +102,7 @@ def run(self): self.success.emit(self._cam) except Exception as exc: self.error.emit(f"{type(exc).__name__}: {exc}") - finally: - self.finished.emit() + # QThread.finished will fire automatically. # ------------------------------- @@ -88,27 +111,24 @@ def run(self): class CameraLoadWorker(QThread): """Open/configure a camera backend off the UI thread with progress and cancel support.""" - progress = Signal(str) # Human-readable status updates - success = Signal(object) # Emits the ready backend (CameraBackend) - error = Signal(str) # Emits error message - canceled = Signal() # Emits when canceled before success + progress = Signal(str) + success = Signal(object) # emits CameraSettings for GUI-thread open + error = Signal(str) + canceled = Signal() def __init__(self, cam: CameraSettings, parent: QWidget | None = None): super().__init__(parent) self._cam = copy.deepcopy(cam) - self._cancel = False self._backend: CameraBackend | None = None - # Do not use fast_start here as we want to actually open the camera to probe capabilities - # If you want a quick probe without full open, use CameraProbeWorker instead which sets fast_start=True # Ensure preview open never uses fast_start probe mode if isinstance(self._cam.properties, dict): ns = self._cam.properties.setdefault(self._cam.backend.lower(), {}) if isinstance(ns, dict): ns["fast_start"] = False - def request_cancel(self): + def request_cancel(self) -> None: self._cancel = True def _check_cancel(self) -> bool: @@ -117,15 +137,16 @@ def _check_cancel(self) -> bool: return True return False - def run(self): + def run(self) -> None: try: self.progress.emit("Creating backend…") if self._check_cancel(): self.canceled.emit() return - LOGGER.debug("Creating camera backend for %s:%d", self._cam.backend, self._cam.index) + LOGGER.debug("Preparing camera open for %s:%d", self._cam.backend, self._cam.index) self.progress.emit("Opening device…") + # Open only in GUI thread to avoid simultaneous opens self.success.emit(self._cam) diff --git a/dlclivegui/gui/camera_config/ui_blocks.py b/dlclivegui/gui/camera_config/ui_blocks.py index 28395c0..86e4f19 100644 --- a/dlclivegui/gui/camera_config/ui_blocks.py +++ b/dlclivegui/gui/camera_config/ui_blocks.py @@ -193,10 +193,9 @@ def build_available_cameras_group(dlg: CameraConfigDialog) -> QGroupBox: dlg.scan_cancel_btn.setIcon(dlg.style().standardIcon(QStyle.StandardPixmap.SP_BrowserStop)) dlg.scan_cancel_btn.setVisible(False) - # The original UI block connects cancel here; preserve that. - # dlg must provide _on_scan_cancel - if hasattr(dlg, "_on_scan_cancel"): - dlg.scan_cancel_btn.clicked.connect(dlg._on_scan_cancel) # type: ignore[attr-defined] + # dlg must provide request_scan_cancel() + if hasattr(dlg, "request_scan_cancel"): + dlg.scan_cancel_btn.clicked.connect(dlg.request_scan_cancel) # type: ignore[attr-defined] available_layout.addWidget(dlg.scan_cancel_btn) diff --git a/dlclivegui/gui/main_window.py b/dlclivegui/gui/main_window.py index 9f738a6..380eda0 100644 --- a/dlclivegui/gui/main_window.py +++ b/dlclivegui/gui/main_window.py @@ -82,8 +82,6 @@ from .recording_manager import RecordingManager from .theme import LOGO, LOGO_ALPHA, AppStyle, apply_theme -# logging.basicConfig(level=logging.INFO) -logging.basicConfig(level=logging.DEBUG) # FIXME @C-Achard set back to INFO for release logger = logging.getLogger("DLCLiveGUI") @@ -196,11 +194,6 @@ def __init__(self, config: ApplicationSettings | None = None): self._display_timer.timeout.connect(self._update_display_from_pending) self._display_timer.start() - # Show status message if myconfig.json was loaded - # FIXME @C-Achard deprecated behavior, remove later - if self._config_path and self._config_path.name == "myconfig.json": - self.statusBar().showMessage(f"Auto-loaded configuration from {self._config_path}", 5000) - # Validate cameras from loaded config (deferred to allow window to show first) # NOTE IMPORTANT (tests/CI): This is scheduled via a QTimer and may fire during pytest-qt teardown. QTimer.singleShot(100, self._validate_configured_cameras) @@ -210,7 +203,7 @@ def __init__(self, config: ApplicationSettings | None = None): # Mitigations for tests/CI: # - Disable this timer by monkeypatching _validate_configured_cameras in GUI tests # - OR monkeypatch/override _show_warning/_show_error to no-op in GUI tests (easiest) - # - OR use a cancellable QTimer attribute and stop() it in closeEven + # - OR use a cancellable QTimer attribute and stop() it in closeEvent def resizeEvent(self, event): super().resizeEvent(event) @@ -882,14 +875,29 @@ def _parse_json(self, value: str) -> dict: return json.loads(text) def _dlc_settings_from_ui(self) -> DLCProcessorSettings: + model_path = self.model_path_edit.text().strip() + if Path(model_path).exists() and Path(model_path).suffix == ".pb": + # IMPORTANT NOTE: DLClive expects a directory for TensorFlow models, + # so if user selects a .pb file, we should pass the parent directory to DLCLive + model_path = str(Path(model_path).parent) + if model_path == "": + raise ValueError("Model path cannot be empty. Please enter a valid path to a DLCLive model file.") + try: + model_bknd = DLCLiveProcessor.get_model_backend(model_path) + except Exception as e: + raise RuntimeError( + "Could not determine model backend from path. " + "Please ensure the model file is valid and has an appropriate extension " + "(.pt, .pth for PyTorch or model directory for TensorFlow)." + ) from e return DLCProcessorSettings( - model_path=self.model_path_edit.text().strip(), + model_path=model_path, model_directory=self._config.dlc.model_directory, # Preserve from config device=self._config.dlc.device, # Preserve from config dynamic=self._config.dlc.dynamic, # Preserve from config resize=self._config.dlc.resize, # Preserve from config precision=self._config.dlc.precision, # Preserve from config - model_type="pytorch", # FIXME @C-Achard hardcoded for now, we should allow tf models too + model_type=model_bknd, # additional_options=self._parse_json(self.additional_options_edit.toPlainText()), ) @@ -975,10 +983,9 @@ def _action_browse_model(self) -> None: dlg.setFileMode(QFileDialog.FileMode.ExistingFile) dlg.setNameFilters( [ - "Model files (*.pt *.pth *.pb)", + "Model files (*.pt *.pth)", "PyTorch models (*.pt *.pth)", "TensorFlow models (*.pb)", - "All files (*.*)", ] ) dlg.setDirectory(start_dir) @@ -991,7 +998,25 @@ def _action_browse_model(self) -> None: selected = dlg.selectedFiles() if not selected: return - file_path = selected[0] + file_path = Path(selected[0]).expanduser() + if not file_path.exists(): + QMessageBox.warning(self, "File not found", f"The selected file does not exist:\n{file_path}") + return + + try: + if file_path.suffix == ".pb": + # For TensorFlow, DLCLive expects a directory, so we pass the parent directory for validation + model_check_path = file_path.parent + else: + model_check_path = file_path + DLCLiveProcessor.get_model_backend(str(model_check_path)) + except FileNotFoundError as e: + QMessageBox.warning(self, "Model selection error", str(e)) + return + except ValueError as e: + QMessageBox.warning(self, "Model selection error", str(e)) + return + file_path = str(file_path) self.model_path_edit.setText(file_path) # Persist model path + directory diff --git a/dlclivegui/processors/dlc_processor_socket.py b/dlclivegui/processors/dlc_processor_socket.py index 0974002..d999690 100644 --- a/dlclivegui/processors/dlc_processor_socket.py +++ b/dlclivegui/processors/dlc_processor_socket.py @@ -17,7 +17,6 @@ from dlclive import Processor # type: ignore logger = logging.getLogger("dlc_processor_socket") -logger.setLevel(logging.INFO) # Avoid duplicate handlers if module is imported multiple times if not any(isinstance(h, logging.StreamHandler) for h in logger.handlers): diff --git a/dlclivegui/services/dlc_processor.py b/dlclivegui/services/dlc_processor.py index 052c952..42b5868 100644 --- a/dlclivegui/services/dlc_processor.py +++ b/dlclivegui/services/dlc_processor.py @@ -16,9 +16,8 @@ from PySide6.QtCore import QObject, Signal from dlclivegui.config import DLCProcessorSettings - -# from dlclivegui.config import DLCProcessorSettings from dlclivegui.processors.processor_utils import instantiate_from_scan +from dlclivegui.temp import Engine # type: ignore # TODO use main package enum when released logger = logging.getLogger(__name__) @@ -26,7 +25,9 @@ ENABLE_PROFILING = True try: # pragma: no cover - optional dependency - from dlclive import DLCLive # type: ignore + from dlclive import ( + DLCLive, # type: ignore + ) except Exception as e: # pragma: no cover - handled gracefully logger.error(f"dlclive package could not be imported: {e}") DLCLive = None # type: ignore[assignment] @@ -96,6 +97,10 @@ def __init__(self) -> None: self._gpu_inference_times: deque[float] = deque(maxlen=60) self._processor_overhead_times: deque[float] = deque(maxlen=60) + @staticmethod + def get_model_backend(model_path: str) -> Engine: + return Engine.from_model_path(model_path) + def configure(self, settings: DLCProcessorSettings, processor: Any | None = None) -> None: self._settings = settings self._processor = processor diff --git a/dlclivegui/temp/__init__.py b/dlclivegui/temp/__init__.py new file mode 100644 index 0000000..d27f406 --- /dev/null +++ b/dlclivegui/temp/__init__.py @@ -0,0 +1,11 @@ +""" +This dlclivegui.temp package is a temporary location for code +that is needed but duplicated from dlclive or other packages, +and are not yet released in the main dlclive or other packages. +This is a strictly temporary location and should be removed +as soon as the code is released in the main dlclive or other packages. +""" + +from .engine import Engine # type: ignore + +__all__ = ["Engine"] diff --git a/dlclivegui/temp/engine.py b/dlclivegui/temp/engine.py new file mode 100644 index 0000000..a6bb225 --- /dev/null +++ b/dlclivegui/temp/engine.py @@ -0,0 +1,50 @@ +from enum import Enum +from pathlib import Path + + +# TODO @C-Achard decide if this moves to utils, +# or if we update dlclive.Engine to have these methods and use that instead of a separate enum here. +# The latter would be more cohesive but also creates a dependency from utils to dlclive, +# pending release of dlclive +class Engine(Enum): + TENSORFLOW = "tensorflow" + PYTORCH = "pytorch" + + @staticmethod + def is_pytorch_model_path(model_path: str | Path) -> bool: + path = Path(model_path) + return path.is_file() and path.suffix.lower() in (".pt", ".pth") + + @staticmethod + def is_tensorflow_model_dir_path(model_path: str | Path) -> bool: + path = Path(model_path) + if not path.is_dir(): + return False + has_cfg = (path / "pose_cfg.yaml").is_file() + has_pb = any(p.is_file() and p.suffix.lower() == ".pb" for p in path.iterdir()) + return has_cfg and has_pb + + @classmethod + def from_model_type(cls, model_type: str) -> "Engine": + if model_type.lower() == "pytorch": + return cls.PYTORCH + elif model_type.lower() in ("tensorflow", "base", "tensorrt", "lite"): + return cls.TENSORFLOW + else: + raise ValueError(f"Unknown model type: {model_type}") + + @classmethod + def from_model_path(cls, model_path: str | Path) -> "Engine": + path = Path(model_path) + + if not path.exists(): + raise FileNotFoundError(f"Model path does not exist: {model_path}") + + if path.is_dir(): + if cls.is_tensorflow_model_dir_path(path): + return cls.TENSORFLOW + elif path.is_file(): + if cls.is_pytorch_model_path(path): + return cls.PYTORCH + + raise ValueError(f"Could not determine engine from model path: {model_path}") diff --git a/dlclivegui/utils/settings_store.py b/dlclivegui/utils/settings_store.py index 51d9fa9..fcf36fd 100644 --- a/dlclivegui/utils/settings_store.py +++ b/dlclivegui/utils/settings_store.py @@ -1,11 +1,13 @@ # dlclivegui/utils/settings_store.py +from __future__ import annotations + import logging from pathlib import Path from PySide6.QtCore import QSettings from ..config import ApplicationSettings -from .utils import is_model_file +from ..temp import Engine # type: ignore # TODO use main package enum when released logger = logging.getLogger(__name__) @@ -70,124 +72,196 @@ class ModelPathStore: def __init__(self, settings: QSettings | None = None): self._settings = settings or QSettings("DeepLabCut", "DLCLiveGUI") - def _norm(self, p: str | None) -> str | None: + # ------------------------- + # Normalization helpers + # ------------------------- + def _as_path(self, p: str | None) -> Path | None: + """Best-effort conversion to Path (expand ~, interpret '.' as cwd).""" if not p: return None + s = str(p).strip() + if not s: + return None try: - return str(Path(p).expanduser().resolve()) + pp = Path(s).expanduser() + if s in (".", "./"): + pp = Path.cwd() + return pp except Exception: - logger.debug("Failed to normalize path: %s", p) + logger.debug("Failed to parse path: %s", p) + return None + + def _norm_existing_dir(self, p: str | None) -> str | None: + """Return an absolute, resolved existing directory path, else None.""" + pp = self._as_path(p) + if pp is None: + return None + try: + # If a file was given, use its parent directory + if pp.exists() and pp.is_file(): + pp = pp.parent + + if pp.exists() and pp.is_dir(): + return str(pp.resolve()) + except Exception: + logger.debug("Failed to normalize directory: %s", p) + return None + + def _norm_existing_path(self, p: str | None) -> str | None: + """Return an absolute, resolved existing path (file or dir), else None.""" + pp = self._as_path(p) + if pp is None: return None + try: + if pp.exists(): + return str(pp.resolve()) + except Exception: + logger.debug("Failed to normalize path: %s", p) + return None + # ------------------------- + # Load + # ------------------------- def load_last(self) -> str | None: + """Return last model path if it still exists and looks usable.""" val = self._settings.value("dlc/last_model_path") - path = self._norm(str(val)) if val else None + path = self._norm_existing_path(str(val)) if val else None if not path: return None + try: - return path if is_model_file(path) else None + pp = Path(path) + # Accept a valid model *file* + if pp.is_file() and (Engine.is_pytorch_model_path(pp) or Engine.is_tensorflow_model_dir_path(pp.parent)): + return str(pp) except Exception: - logger.debug("Last model path is not a valid model file: %s", path) - return None + logger.debug("Last model path not valid/usable: %s", path) + + return None def load_last_dir(self) -> str | None: + """Return last directory if it still exists and is a directory.""" val = self._settings.value("dlc/last_model_dir") - d = self._norm(str(val)) if val else None - if not d: - return None - try: - p = Path(d) - return str(p) if p.exists() and p.is_dir() else None - except Exception: - logger.debug("Last model dir is not a valid directory: %s", d) - return None + d = self._norm_existing_dir(str(val)) if val else None + return d + # ------------------------- + # Save + # ------------------------- def save_if_valid(self, path: str) -> None: - """Save last model *file* if it looks valid, and always save its directory.""" - path = self._norm(path) or "" - if not path: + """ + Save last model path if it looks valid/usable, and always save its directory. + - For files: always save parent directory. + - For directories: save directory itself if it looks like a TF model dir. + """ + norm = self._norm_existing_path(path) + if not norm: return + try: - parent = str(Path(path).parent) - self._settings.setValue("dlc/last_model_dir", parent) + p = Path(norm) + + # Always persist a *directory* that is safe for QFileDialog.setDirectory(...) + if p.is_dir(): + model_dir = p + else: + model_dir = p.parent + + model_dir_norm = self._norm_existing_dir(str(model_dir)) + if model_dir_norm: + self._settings.setValue("dlc/last_model_dir", model_dir_norm) + + # Persist model path if it is a valid model file, or a TF model directory + if Engine.is_pytorch_model_path(p): + self._settings.setValue("dlc/last_model_path", str(p)) + elif p.parent.is_dir() and Engine.is_tensorflow_model_dir_path(p.parent): + self._settings.setValue("dlc/last_model_path", str(p)) + # elif p.is_dir() and Engine.is_tensorflow_model_dir_path(p): + # self._settings.setValue("dlc/last_model_path", str(p)) - if is_model_file(path): - self._settings.setValue("dlc/last_model_path", str(Path(path))) except Exception: - logger.debug("Failed to save last model path: %s", path) - pass + logger.debug("Failed to save model path: %s", path, exc_info=True) def save_last_dir(self, directory: str) -> None: - directory = self._norm(directory) or "" - if not directory: + d = self._norm_existing_dir(directory) + if not d: return try: - p = Path(directory) - if p.exists() and p.is_dir(): - self._settings.setValue("dlc/last_model_dir", str(p)) + self._settings.setValue("dlc/last_model_dir", d) except Exception: - pass + logger.debug("Failed to save last model dir: %s", d, exc_info=True) + # ------------------------- + # Resolve + # ------------------------- def resolve(self, config_path: str | None) -> str: - """Resolve the best model path to display in the UI.""" - config_path = self._norm(config_path) - if config_path: + """ + Resolve the best model path to display in the UI. + Preference: + 1) config_path if valid/usable + 2) persisted last model path if valid/usable + 3) empty + """ + cfg = self._norm_existing_path(config_path) + if cfg: try: - if is_model_file(config_path): - return config_path + p = Path(cfg) + if p.is_file() and Engine.is_pytorch_model_path(p): + return cfg + if p.is_dir() and Engine.is_tensorflow_model_dir_path(p): + return cfg except Exception: - logger.debug("Config path is not a valid model file: %s", config_path) - pass + logger.debug("Config path not usable: %s", cfg) persisted = self.load_last() if persisted: - try: - if is_model_file(persisted): - return persisted - except Exception: - pass + return persisted return "" def suggest_start_dir(self, fallback_dir: str | None = None) -> str: - """Pick the best directory to start the file dialog in.""" + """ + Pick the best directory to start file dialogs in. + Guarantees: returns an existing absolute directory (never '.'). + """ # 1) last dir last_dir = self.load_last_dir() if last_dir: return last_dir - # 2) directory of last valid model file - last_file = self.load_last() - if last_file: + # 2) directory of last valid model path + last = self.load_last() + if last: try: - parent = Path(last_file).parent - if parent.exists(): - return str(parent) + p = Path(last) + if p.is_file(): + parent = self._norm_existing_dir(str(p.parent)) + if parent: + return parent + elif p.is_dir(): + d = self._norm_existing_dir(str(p)) + if d: + return d except Exception: - logger.debug("Failed to get parent of last model file: %s", last_file) - pass + logger.debug("Failed to derive start dir from last model: %s", last) - # 3) fallback dir (config.model_directory) if valid - if fallback_dir: - try: - p = Path(fallback_dir).expanduser() - if p.exists() and p.is_dir(): - return str(p) - except Exception: - logger.debug("Fallback dir is not a valid directory: %s", fallback_dir) - pass + # 3) fallback dir (e.g. config.dlc.model_directory) + fb = self._norm_existing_dir(fallback_dir) + if fb: + return fb - # 4) last resort: home - return str(Path.home()) + # 4) last resort: cwd if exists else home + cwd = self._norm_existing_dir(str(Path.cwd())) + return cwd or str(Path.home()) def suggest_selected_file(self) -> str | None: - """Optional: return a file to preselect if it exists.""" - last_file = self.load_last() - if not last_file: + """Return a file to preselect if it exists (only files, not directories).""" + last = self.load_last() + if not last: return None try: - p = Path(last_file) + p = Path(last) return str(p) if p.exists() and p.is_file() else None except Exception: - logger.debug("Failed to check existence of last model file: %s", last_file) + logger.debug("Failed to check existence of last model: %s", last) return None diff --git a/dlclivegui/utils/utils.py b/dlclivegui/utils/utils.py index 3d3a4ff..6af003d 100644 --- a/dlclivegui/utils/utils.py +++ b/dlclivegui/utils/utils.py @@ -8,18 +8,9 @@ from datetime import datetime from pathlib import Path -SUPPORTED_MODELS = [".pt", ".pth", ".pb"] _INVALID_CHARS = re.compile(r"[^A-Za-z0-9._-]+") -def is_model_file(file_path: Path | str) -> bool: - if not isinstance(file_path, Path): - file_path = Path(file_path) - if not file_path.is_file(): - return False - return file_path.suffix.lower() in SUPPORTED_MODELS - - def sanitize_name(name: str, *, fallback: str = "session") -> str: """Make a user-provided string safe for filesystem paths.""" name = (name or "").strip() diff --git a/pyproject.toml b/pyproject.toml index 7fba79a..bb84eac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,6 +4,7 @@ requires = [ "setuptools>=68" ] [project] name = "deeplabcut-live-gui" +version = "2.0.0rc1" description = "PySide6-based GUI to run real time pose estimation experiments with DeepLabCut" readme = "README.md" keywords = [ "deep learning", "deeplabcut", "gui", "pose estimation", "real-time" ] @@ -21,7 +22,6 @@ classifiers = [ "Programming Language :: Python :: 3.12", "Topic :: Scientific/Engineering :: Artificial Intelligence", ] -dynamic = [ "version" ] # version is set in dlclivegui/__init__.py dependencies = [ "cv2-enumerate-cameras", "deeplabcut-live==1.1", @@ -60,6 +60,8 @@ test = [ "pytest-cov>=4", "pytest-mock>=3.10", "pytest-qt>=4.2", + "tox", + "tox-gh-actions", ] tf = [ "deeplabcut-live[tf]==1.1", @@ -72,9 +74,6 @@ Documentation = "https://github.com/DeepLabCut/DeepLabCut-live-GUI" # FIXME @C- Homepage = "https://github.com/DeepLabCut/DeepLabCut-live-GUI" Repository = "https://github.com/DeepLabCut/DeepLabCut-live-GUI" -[tool.setuptools.dynamic] -version = { attr = "dlclivegui.__version__" } - # [tool.setuptools] # include-package-data = true [tool.setuptools.package-data] diff --git a/tests/gui/camera_config/test_cam_dialog_e2e.py b/tests/gui/camera_config/test_cam_dialog_e2e.py index 160bf12..49867b8 100644 --- a/tests/gui/camera_config/test_cam_dialog_e2e.py +++ b/tests/gui/camera_config/test_cam_dialog_e2e.py @@ -8,11 +8,11 @@ from PySide6.QtCore import Qt from PySide6.QtWidgets import QMessageBox -from dlclivegui.cameras import CameraFactory from dlclivegui.cameras.base import CameraBackend -from dlclivegui.cameras.factory import DetectedCamera +from dlclivegui.cameras.factory import CameraFactory, DetectedCamera from dlclivegui.config import CameraSettings, MultiCameraSettings -from dlclivegui.gui.camera_config.camera_config_dialog import CameraConfigDialog, CameraLoadWorker +from dlclivegui.gui.camera_config.camera_config_dialog import CameraConfigDialog +from dlclivegui.gui.camera_config.loaders import CameraLoadWorker from dlclivegui.gui.camera_config.preview import PreviewState # --------------------------------------------------------------------- @@ -20,6 +20,22 @@ # --------------------------------------------------------------------- +def _run_scan_and_wait(dialog: CameraConfigDialog, qtbot, timeout: int = 2000) -> None: + """ + Trigger a scan via UI and wait for the dialog's scan_finished, + which now means: UI is stable and available list is populated (or placeholder). + """ + qtbot.waitUntil(lambda: not dialog._is_scan_running(), timeout=timeout) + qtbot.wait(50) + + # Wait for the scan started by *this click* to both start and finish + with qtbot.waitSignals([dialog.scan_started, dialog.scan_finished], timeout=timeout, order="strict"): + qtbot.mouseClick(dialog.refresh_btn, Qt.LeftButton) + + # Now the list should be stable + qtbot.waitUntil(lambda: dialog.available_cameras_list.count() > 0, timeout=timeout) + + def _select_backend_for_active_cam(dialog: CameraConfigDialog, cam_row: int = 0) -> str: """ Ensure backend combo is set to the backend of the active camera at cam_row. @@ -119,9 +135,10 @@ def dialog(qtbot, patch_detect_cameras): except Exception: d.close() - qtbot.waitUntil(lambda: getattr(d, "_loader", None) is None, timeout=2000) - qtbot.waitUntil(lambda: getattr(d, "_scan_worker", None) is None, timeout=2000) - qtbot.waitUntil(lambda: not getattr(d, "_preview_active", False), timeout=2000) + qtbot.waitUntil(lambda: d._preview.loader is None, timeout=2000) + qtbot.waitUntil(lambda: not d._is_scan_running(), timeout=2000) + qtbot.wait(50) + qtbot.waitUntil(lambda: d._preview.state == PreviewState.IDLE, timeout=2000) # --------------------------------------------------------------------- @@ -131,9 +148,7 @@ def dialog(qtbot, patch_detect_cameras): @pytest.mark.gui def test_e2e_async_camera_scan(dialog, qtbot): - qtbot.mouseClick(dialog.refresh_btn, Qt.LeftButton) - with qtbot.waitSignal(dialog.scan_finished, timeout=2000): - pass + _run_scan_and_wait(dialog, qtbot, timeout=2000) assert dialog.available_cameras_list.count() == 2 @@ -247,19 +262,17 @@ def read(self): @pytest.mark.gui def test_e2e_selection_change_auto_commits(dialog, qtbot): - """ - Guard contract: switching selection commits pending edits. - Use FPS (supported) rather than gain (OpenCV gain is intentionally disabled). - """ - # Ensure backend combo matches active cam (important for add/dup logic) _select_backend_for_active_cam(dialog, cam_row=0) - # Add second camera deterministically - dialog._on_scan_result([DetectedCamera(index=1, label="ExtraCam")]) - dialog.available_cameras_list.setCurrentRow(0) + # Discover cameras via UI + _run_scan_and_wait(dialog, qtbot, timeout=2000) + assert dialog.available_cameras_list.count() == 2 + + # Select the second detected camera to avoid duplicate (index 1) + dialog.available_cameras_list.setCurrentRow(1) qtbot.mouseClick(dialog.add_camera_btn, Qt.LeftButton) - assert len(dialog._working_settings.cameras) >= 2 + qtbot.waitUntil(lambda: len(dialog._working_settings.cameras) >= 2, timeout=1000) dialog.active_cameras_list.setCurrentRow(0) qtbot.waitUntil(lambda: dialog._current_edit_index == 0, timeout=1000) @@ -291,19 +304,17 @@ def slow_detect(backend, max_devices=10, should_cancel=None, progress_cb=None, * qtbot.mouseClick(dialog.scan_cancel_btn, Qt.LeftButton) + # scan_finished = UI stable, not necessarily worker fully stopped / controls unlocked with qtbot.waitSignal(dialog.scan_finished, timeout=3000): pass - assert dialog.refresh_btn.isEnabled() - assert dialog.backend_combo.isEnabled() + # Wait until scan controls are unlocked (worker finished) + qtbot.waitUntil(lambda: dialog.refresh_btn.isEnabled(), timeout=3000) + qtbot.waitUntil(lambda: dialog.backend_combo.isEnabled(), timeout=3000) @pytest.mark.gui def test_duplicate_camera_prevented(dialog, qtbot, monkeypatch): - """ - Duplicate detection compares identity keys including backend. - Ensure backend combo is set to match existing active camera backend. - """ calls = {"n": 0} def _warn(parent, title, text, *args, **kwargs): @@ -312,14 +323,15 @@ def _warn(parent, title, text, *args, **kwargs): monkeypatch.setattr(QMessageBox, "warning", staticmethod(_warn)) - backend = _select_backend_for_active_cam(dialog, cam_row=0) - + _select_backend_for_active_cam(dialog, cam_row=0) initial_count = dialog.active_cameras_list.count() - # Same backend + same index -> duplicate - dialog._on_scan_result([DetectedCamera(index=0, label=f"{backend}-X")]) - dialog.available_cameras_list.setCurrentRow(0) + # Scan normally + _run_scan_and_wait(dialog, qtbot, timeout=2000) + assert dialog.available_cameras_list.count() == 2 + # Choose the entry that matches index 0 (duplicate) + dialog.available_cameras_list.setCurrentRow(0) qtbot.mouseClick(dialog.add_camera_btn, Qt.LeftButton) assert dialog.active_cameras_list.count() == initial_count @@ -328,9 +340,6 @@ def _warn(parent, title, text, *args, **kwargs): @pytest.mark.gui def test_max_cameras_prevented(qtbot, monkeypatch, patch_detect_cameras): - """ - Dialog enforces MAX_CAMERAS enabled cameras. - """ calls = {"n": 0} def _warn(parent, title, text, *args, **kwargs): @@ -354,14 +363,13 @@ def _warn(parent, title, text, *args, **kwargs): try: _select_backend_for_active_cam(d, cam_row=0) - initial_count = d.active_cameras_list.count() - qtbot.waitUntil(lambda: not d._is_scan_running(), timeout=1000) - d._on_scan_result([DetectedCamera(index=4, label="Extra")]) - d._on_scan_finished() - d.available_cameras_list.setCurrentRow(0) + _run_scan_and_wait(d, qtbot, timeout=2000) + assert d.available_cameras_list.count() == 2 + # Try to add any detected camera (should hit MAX_CAMERAS guard) + d.available_cameras_list.setCurrentRow(1) qtbot.mouseClick(d.add_camera_btn, Qt.LeftButton) assert d.active_cameras_list.count() == initial_count diff --git a/tests/gui/camera_config/test_cam_dialog_unit.py b/tests/gui/camera_config/test_cam_dialog_unit.py index fc73f75..2abe022 100644 --- a/tests/gui/camera_config/test_cam_dialog_unit.py +++ b/tests/gui/camera_config/test_cam_dialog_unit.py @@ -219,9 +219,14 @@ def test_add_camera_populates_working_settings(dialog_unit, qtbot): Add camera should append a new CameraSettings into _working_settings. We directly call _on_scan_result to populate available list deterministically. """ - dialog_unit._on_scan_result([DetectedCamera(index=2, label="ExtraCam2")]) - dialog_unit.available_cameras_list.setCurrentRow(0) + from dlclivegui.gui.camera_config.loaders import CameraScanState + + dialog_unit._set_scan_state(CameraScanState.RUNNING, message="Test scan running") + dialog_unit._on_scan_result([DetectedCamera(label="ExtraCam2", index=2)]) + with qtbot.waitSignal(dialog_unit.scan_finished, timeout=1000): + pass + dialog_unit.available_cameras_list.setCurrentRow(0) qtbot.mouseClick(dialog_unit.add_camera_btn, Qt.LeftButton) added = dialog_unit._working_settings.cameras[-1] diff --git a/tests/gui/test_ascii_art.py b/tests/gui/test_ascii_art.py index 50d702c..2e5800e 100644 --- a/tests/gui/test_ascii_art.py +++ b/tests/gui/test_ascii_art.py @@ -265,14 +265,14 @@ def test_print_ascii_writes_file(tmp_png_gray, force_tty, tmp_path): def test_build_help_description_tty(tmp_png_bgra_logo, monkeypatch, force_tty): - monkeypatch.setattr(ascii_mod, "ASCII_IMAGE_PATH", Path(tmp_png_bgra_logo)) - desc = ascii_mod.build_help_description(static_banner=None, color="always", min_width=60) + monkeypatch.setattr(ascii_mod, "LOGO_ALPHA", Path(tmp_png_bgra_logo)) + desc = ascii_mod.build_help_description(static_banner=None, color="auto", min_width=60) assert "DeepLabCut-Live GUI" in desc assert "\x1b[36m" in desc # cyan wrapper now present since TTY is mocked correctly def test_build_help_description_notty(tmp_png_bgra_logo, monkeypatch, force_notty): - monkeypatch.setattr(ascii_mod, "ASCII_IMAGE_PATH", Path(tmp_png_bgra_logo)) - desc = ascii_mod.build_help_description(static_banner=None, color="always", min_width=60) + monkeypatch.setattr(ascii_mod, "LOGO_ALPHA", Path(tmp_png_bgra_logo)) + desc = ascii_mod.build_help_description(static_banner=None, color="auto", min_width=60) # Not a TTY -> no banner, just the plain description - assert desc.strip() == "DeepLabCut-Live GUI — launch the graphical interface." + assert "DeepLabCut-Live GUI — launch the graphical interface." in desc diff --git a/tests/gui/test_main.py b/tests/gui/test_main.py index 83d4bcc..b9ed7da 100644 --- a/tests/gui/test_main.py +++ b/tests/gui/test_main.py @@ -42,7 +42,7 @@ def test_preview_renders_frames(qtbot, window, multi_camera_controller): @pytest.mark.gui @pytest.mark.functional -def test_start_inference_emits_pose(qtbot, window, multi_camera_controller, dlc_processor): +def test_start_inference_emits_pose(qtbot, window, multi_camera_controller, dlc_processor, tmp_path): """ Validate that: - Preview is running @@ -67,7 +67,9 @@ def test_start_inference_emits_pose(qtbot, window, multi_camera_controller, dlc_ timeout=6000, ) - w.model_path_edit.setText("dummy_model.pt") + model_weights = tmp_path / "dummy_model.pt" + model_weights.touch() # create an empty file to satisfy existence check + w.model_path_edit.setText(str(model_weights)) pose_count = [0] def _on_pose(result): diff --git a/tests/services/test_dlc_processor.py b/tests/services/test_dlc_processor.py index 2ae5e3a..3f5e0cb 100644 --- a/tests/services/test_dlc_processor.py +++ b/tests/services/test_dlc_processor.py @@ -63,9 +63,10 @@ def test_worker_processes_frames(qtbot, monkeypatch_dlclive, settings_model): proc.enqueue_frame(frame, timestamp=2.0 + i) qtbot.wait(5) # ms - # FIXME @C-Achard this still fails randomly - # the timeout has to be surprisingly large here - # not sure if it's qtbot or threading scheduling delays + # NOTE @C-Achard The timeout here is intentionally large to account for potential + # Qt event-loop and threading scheduling delays in CI environments. + # This was previously flaky with a smaller timeout; increasing it should + # keep the test stable. qtbot.waitUntil(lambda: proc.get_stats().frames_processed >= 3, timeout=3000) finally: diff --git a/tests/utils/test_settings_store.py b/tests/utils/test_settings_store.py index f379b76..7eba56a 100644 --- a/tests/utils/test_settings_store.py +++ b/tests/utils/test_settings_store.py @@ -98,15 +98,29 @@ def model_validate_json(raw: str): # ----------------------------- # ModelPathStore helpers # ----------------------------- -def test_model_path_store_norm_handles_none_and_invalid(monkeypatch): +def test_model_path_store_norm_handles_none_and_invalid(tmp_path: Path): s = InMemoryQSettings() mps = store.ModelPathStore(settings=s) - assert mps._norm(None) is None # type: ignore[arg-type] + # None should normalize to None + assert mps._norm_existing_path(None) is None # type: ignore[arg-type] + assert mps._norm_existing_dir(None) is None # type: ignore[arg-type] - # Force Path.expanduser() to raise by passing something weird? Hard to do reliably. - # Instead just assert normal path expands/returns str. - assert mps._norm("~/somewhere") is not None + # Existing dir should normalize to an absolute path + d = tmp_path / "models" + d.mkdir() + norm_dir = mps._norm_existing_dir(str(d)) + assert norm_dir is not None + assert Path(norm_dir).exists() + assert Path(norm_dir).is_dir() + + # Existing file should normalize as existing path + f = d / "net.pt" + f.write_text("x") + norm_file = mps._norm_existing_path(str(f)) + assert norm_file is not None + assert Path(norm_file).exists() + assert Path(norm_file).is_file() # ----------------------------- @@ -221,19 +235,21 @@ def test_model_path_store_resolve_prefers_config_path_when_valid(tmp_path: Path) assert mps.resolve(str(model)) == str(model) -def test_model_path_store_resolve_falls_back_to_persisted(tmp_path: Path): +def test_model_path_store_resolve_falls_back_to_persisted_tf_dir(tmp_path: Path): settings = InMemoryQSettings() mps = store.ModelPathStore(settings=settings) - persisted = tmp_path / "persisted.pb" - persisted.write_text("x") - settings.setValue("dlc/last_model_path", str(persisted)) + tf_dir = tmp_path / "tf_model" + tf_dir.mkdir() + (tf_dir / "pose_cfg.yaml").write_text("cfg: 1\n") + (tf_dir / "graph.pb").write_text("pb") + + settings.setValue("dlc/last_model_path", str(tf_dir / "graph.pb")) - # invalid config path bad = tmp_path / "notamodel.onnx" bad.write_text("x") - assert mps.resolve(str(bad)) == str(persisted) + assert mps.resolve(str(bad)) == str(tf_dir / "graph.pb") def test_model_path_store_resolve_returns_empty_when_nothing_valid(tmp_path: Path): @@ -289,7 +305,12 @@ def test_model_path_store_suggest_start_dir_falls_back_to_home(tmp_path: Path, m fake_home = tmp_path / "home" fake_home.mkdir() + # Make cwd "invalid" so suggest_start_dir can't use it + fake_cwd = tmp_path / "does_not_exist" + assert not fake_cwd.exists() + monkeypatch.setattr(store.Path, "home", lambda: fake_home) + monkeypatch.setattr(store.Path, "cwd", lambda: fake_cwd) assert mps.suggest_start_dir(fallback_dir=None) == str(fake_home) diff --git a/tests/utils/test_utils.py b/tests/utils/test_utils.py index 70dd628..ebbc954 100644 --- a/tests/utils/test_utils.py +++ b/tests/utils/test_utils.py @@ -1,41 +1,138 @@ +from __future__ import annotations + from pathlib import Path import pytest import dlclivegui.utils.utils as u +from dlclivegui.temp import Engine pytestmark = pytest.mark.unit +# NOTE @C-Achard: These tests are currently in test_utils.py for convenience, +# but we may want to use dlclive.Engine directly +# and possibly move these tests to dlclive's test suite # ----------------------------- -# is_model_file +# Engine.from_model_type # ----------------------------- -@pytest.mark.unit -def test_is_model_file_true_for_supported_extensions(tmp_path: Path): - for ext in [".pt", ".pth", ".pb"]: - p = tmp_path / f"model{ext}" - p.write_text("x") - assert u.is_model_file(p) is True - assert u.is_model_file(str(p)) is True # also accepts str +@pytest.mark.parametrize( + "inp, expected", + [ + ("pytorch", Engine.PYTORCH), + ("PYTORCH", Engine.PYTORCH), + ("tensorflow", Engine.TENSORFLOW), + ("TensorFlow", Engine.TENSORFLOW), + ("base", Engine.TENSORFLOW), + ("tensorrt", Engine.TENSORFLOW), + ("lite", Engine.TENSORFLOW), + ], +) +def test_engine_from_model_type(inp: str, expected: Engine): + assert Engine.from_model_type(inp) == expected + + +def test_engine_from_model_type_unknown(): + with pytest.raises(ValueError): + Engine.from_model_type("onnx") - # case-insensitive - p2 = tmp_path / "MODEL.PT" - p2.write_text("x") - assert u.is_model_file(p2) is True +# ----------------------------- +# Engine.is_pytorch_model_path +# ----------------------------- +@pytest.mark.parametrize("ext", [".pt", ".pth"]) +def test_engine_is_pytorch_model_path_true(tmp_path: Path, ext: str): + p = tmp_path / f"model{ext}" + p.write_text("x") + assert Engine.is_pytorch_model_path(p) is True + assert Engine.is_pytorch_model_path(str(p)) is True -@pytest.mark.unit -def test_is_model_file_false_for_missing_or_dir(tmp_path: Path): - missing = tmp_path / "missing.pt" - assert u.is_model_file(missing) is False +def test_engine_is_pytorch_model_path_false_for_missing(tmp_path: Path): + p = tmp_path / "missing.pt" + assert Engine.is_pytorch_model_path(p) is False + + +def test_engine_is_pytorch_model_path_false_for_dir(tmp_path: Path): d = tmp_path / "model.pt" d.mkdir() - assert u.is_model_file(d) is False + assert Engine.is_pytorch_model_path(d) is False + + +def test_engine_is_pytorch_model_path_case_insensitive(tmp_path: Path): + # only include if you applied the .lower() patch + p = tmp_path / "MODEL.PT" + p.write_text("x") + assert Engine.is_pytorch_model_path(p) is True + + +# ----------------------------- +# Engine.is_tensorflow_model_dir_path +# ----------------------------- +def _make_tf_dir(tmp_path: Path, *, with_cfg: bool = True, with_pb: bool = True, pb_name: str = "graph.pb") -> Path: + d = tmp_path / "tf_model" + d.mkdir() + if with_cfg: + (d / "pose_cfg.yaml").write_text("cfg: 1\n") + if with_pb: + (d / pb_name).write_text("pbdata") + return d + + +def test_engine_is_tensorflow_model_dir_path_true(tmp_path: Path): + d = _make_tf_dir(tmp_path, with_cfg=True, with_pb=True) + assert Engine.is_tensorflow_model_dir_path(d) is True + assert Engine.is_tensorflow_model_dir_path(str(d)) is True + + +def test_engine_is_tensorflow_model_dir_path_false_missing_cfg(tmp_path: Path): + d = _make_tf_dir(tmp_path, with_cfg=False, with_pb=True) + assert Engine.is_tensorflow_model_dir_path(d) is False + + +def test_engine_is_tensorflow_model_dir_path_false_missing_pb(tmp_path: Path): + d = _make_tf_dir(tmp_path, with_cfg=True, with_pb=False) + assert Engine.is_tensorflow_model_dir_path(d) is False + + +def test_engine_is_tensorflow_model_dir_path_case_insensitive_pb(tmp_path: Path): + # only include if you applied the .lower() patch for pb suffix + d = _make_tf_dir(tmp_path, with_cfg=True, with_pb=True, pb_name="GRAPH.PB") + assert Engine.is_tensorflow_model_dir_path(d) is True + + +# ----------------------------- +# Engine.from_model_path +# ----------------------------- +def test_engine_from_model_path_missing_raises(tmp_path: Path): + missing = tmp_path / "does_not_exist.pt" + with pytest.raises(FileNotFoundError): + Engine.from_model_path(missing) + + +def test_engine_from_model_path_pytorch_file(tmp_path: Path): + p = tmp_path / "net.pth" + p.write_text("x") + assert Engine.from_model_path(p) == Engine.PYTORCH + + +def test_engine_from_model_path_tensorflow_dir(tmp_path: Path): + d = _make_tf_dir(tmp_path, with_cfg=True, with_pb=True) + assert Engine.from_model_path(d) == Engine.TENSORFLOW + + +def test_engine_from_model_path_dir_not_tf_raises(tmp_path: Path): + d = tmp_path / "some_dir" + d.mkdir() + with pytest.raises(ValueError): + Engine.from_model_path(d) + - bad = tmp_path / "model.onnx" - bad.write_text("x") - assert u.is_model_file(bad) is False +def test_engine_from_model_path_file_not_pytorch_raises(tmp_path: Path): + p = tmp_path / "model.pb" + p.write_text("x") # PB file alone is not a TF dir + with pytest.raises(ValueError): + Engine.from_model_path(p) # ----------------------------- diff --git a/tox.ini b/tox.ini index 3ac6e62..3344818 100644 --- a/tox.ini +++ b/tox.ini @@ -20,6 +20,7 @@ commands = setenv = PYTHONWARNINGS = default QT_QPA_PLATFORM = offscreen + QT_OPENGL = software # Can help avoid some Windows/OpenCV capture backend flakiness when tests touch video I/O: OPENCV_VIDEOIO_PRIORITY_MSMF = 0 @@ -31,19 +32,19 @@ passenv = WAYLAND_DISPLAY XDG_RUNTIME_DIR -[testenv:lint] -description = Ruff linting/format checks (matches pyproject.toml config) -skip_install = true -deps = - ruff -commands = - ruff check . - ruff format --check . +; Linting already covered by pre-commit hooks and format.yml workflow +; [testenv:lint] +; description = Ruff linting/format checks (matches pyproject.toml config) +; skip_install = true +; deps = +; ruff +; commands = +; ruff check . +; ruff format --check . -# Optional helper if you use tox-gh-actions to map GitHub's python-version to tox envs. -# Requires: pip install tox-gh-actions [gh-actions] python = 3.10: py310 3.11: py311 - 3.12: py312, lint + 3.12: py312 + ; , lint