minor follow-up adjustments for config GUI validation - #3399
Merged
deruyter92 merged 14 commits intoJul 20, 2026
Conversation
3 tasks
C-Achard
reviewed
Jul 15, 2026
C-Achard
left a comment
Collaborator
There was a problem hiding this comment.
Amazing follow-up, thank you! Great that you added GUI tests as well, will be very useful.
C-Achard
self-requested a review
July 17, 2026 08:36
- Initialize self._config_monitor = None early in __init__ - Replace getattr(self, "_config_monitor", None) with direct attribute access in config_path setter - Remove unintentional double @Property decorator on project_folder
- Catch BaseException in Worker.run() so SystemExit from CLI functions is marshalled to the UI instead of silently killing the thread - Wrap the full evaluate_network method in a single try/except Exception so runtime errors (GPU OOM, corrupted models, etc.) are consistently reported through show_task_error instead of only catching config errors
- Add named _reload_timer members to AnalyzeVideos and ManageProject tabs so the timer is inspectable in tests and auto-cancels on tab destruction - Wire editor.accepted to timer.start instead of a fire-and-forget lambda
a lightweight hook to force the next cfg access to re-read and validate from disk. Can be called from outside the GUI as well (e.g. after video analysis or training network, in case they manipulate te config)
…d timer - Add test_task_error.py: verify generic vs config-error dialog rendering and that the 'Open configuration' button only appears for config errors - Test invalidate_config_cache drops the cached config for the next access and is safe to call with no project loaded - Test that the named _reload_timer (replacing QTimer.singleShot) reliably fires the reload callback
C-Achard
reviewed
Jul 17, 2026
C-Achard
left a comment
Collaborator
There was a problem hiding this comment.
Here are a few additional comments; thanks again!
| comparisonbodyparts=bodyparts_to_use, | ||
| ) | ||
| except CONFIG_LOAD_ERRORS as error: | ||
| self.root.show_task_error(error, self.root.pose_cfg_path) |
Collaborator
There was a problem hiding this comment.
Should this always force the pose_cfg ? Since the config is read too by evaluate_network
Collaborator
Author
There was a problem hiding this comment.
Not sure what you mean here, but we could tackle it in a follow-up if you want
deruyter92
force-pushed
the
jaap/config-gui-additions
branch
from
July 17, 2026 15:40
7b949d7 to
ddce16f
Compare
Connect to the directoryChanged signal and add the parent directory to the watched paths in set_path(). Some editors atomically replace files (delete + rename), which silently drops a file-only watch in QFileSystemWatcher. Watching the parent catches those changes and re-adds the file path in _check_for_change() as before.
The error signal sets it to True. _show_success_message checks it and skips the dialog entirely if there was an error.
…fig-related in show_task_error CONFIG_LOAD_ERRORS includes FileNotFoundError, PermissionError, OSError, TypeError, and ValueError — all of which can originate from non-config sources (e.g. missing checkpoint, disk full, wrong type from a GUI callback) when raised by worker threads. Showing a config-themed dialog with an "Open configuration" button for those errors was misleading. Now show_task_error only trusts ValidationError (which is always from config parsing) to decide on the config dialog. The config-loading recovery loop continues to use the full CONFIG_LOAD_ERRORS tuple where the context is known. Restore TypeError and ValueError to CONFIG_LOAD_ERRORS so the recovery loop properly catches those as user-repairable config errors.
Collaborator
Author
|
@C-Achard, I've addressed all your comments I think. See the unresolved ones for open questions. I think it is in a good state, but will do another iteration on Monday before merging it in your PR. |
deruyter92
marked this pull request as ready for review
July 17, 2026 16:24
C-Achard
approved these changes
Jul 17, 2026
deruyter92
added a commit
that referenced
this pull request
Jul 21, 2026
* Add GUI formatter for config load errors Introduce a new `config_errors` dialog helper that builds user-facing configuration error reports via `ConfigErrorReport`. It formats Pydantic validation issues into readable field paths, customizes messages for missing/unsupported settings, includes truncated invalid input values, and adds dedicated handling for missing-file and permission errors with technical details preserved for diagnostics. * Handle GUI config load errors with recovery Refactors project loading in the main GUI window to consistently validate config files and recover from failures. It adds a structured error dialog with Retry/Edit/Cancel actions, opens the config editor for repair, and reloads after edits. The flow now routes recent-project opens through the same validation path, clears partial tabs before retrying, and updates project state/load methods to return success status so invalid or unreadable configs no longer leave stale UI state. * Use system app for config error recovery Replaces the in-app config repair editor flow with an "Open configuration" action that launches the file in the OS default application, then guides users to save and reload. The config error dialog now supports repeated open/reload attempts in-place, and both project UI initialization and `load_config` were refactored to consistently handle validation and file-read errors with clearer logging and retry behavior. * Improve shuffle config error label display Refines `SelectedShuffleDisplay` error rendering when `pose_cfg.yaml` is missing by formatting the message with line breaks and full path visibility. The label is now configured for plain text, word wrapping, expandable sizing, and mouse/keyboard text selection, making long file paths easier to read and copy. * Fix GUI window maximize behavior Move window sizing constants to module scope and stop forcing the main window max size to screen dimensions. This allows the maximize button to work properly while preserving the initial resize factor and minimum window size defaults. * Make DefaultTab content scrollable Wrap `DefaultTab`’s main content in a `QScrollArea` with a dedicated content widget and zero outer margins so long tab content can be scrolled instead of being clipped. Also updates the `parent` type hint to `QtWidgets.QWidget | None` for consistency with modern typing style. * Wrap selected videos label in GUI widget Update `VideoSelectionWidget` so the selected-videos status label can wrap text and expand horizontally. This improves readability when many videos are selected and prevents the text from being clipped in the GUI layout. * Normalize GUI config paths to absolute Add a shared `absolute_path` helper in `gui.utils` and use it in `MainWindow` when selecting, opening recent, and reloading projects so config paths are consistently expanded and absolute. Also treat closing the config error dialog as a cancel action to avoid falling through without an explicit choice. * minor follow-up adjustments for config GUI validation (#3399) * squashed updates for GUI config validation error handling * replace getattr guard with explicit attr - Initialize self._config_monitor = None early in __init__ - Replace getattr(self, "_config_monitor", None) with direct attribute access in config_path setter - Remove unintentional double @Property decorator on project_folder * fix: widen worker and evaluate-network error handling - Catch BaseException in Worker.run() so SystemExit from CLI functions is marshalled to the UI instead of silently killing the thread - Wrap the full evaluate_network method in a single try/except Exception so runtime errors (GPU OOM, corrupted models, etc.) are consistently reported through show_task_error instead of only catching config errors * refactor: replace QTimer.singleShot with explicit named reload timers - Add named _reload_timer members to AnalyzeVideos and ManageProject tabs so the timer is inspectable in tests and auto-cancels on tab destruction - Wire editor.accepted to timer.start instead of a fire-and-forget lambda * Add `MainWindow.invalidate_config_cache()` a lightweight hook to force the next cfg access to re-read and validate from disk. Can be called from outside the GUI as well (e.g. after video analysis or training network, in case they manipulate te config) * test: add coverage for show_task_error, cache invalidation, and reload timer - Add test_task_error.py: verify generic vs config-error dialog rendering and that the 'Open configuration' button only appears for config errors - Test invalidate_config_cache drops the cached config for the next access and is safe to call with no project loaded - Test that the named _reload_timer (replacing QTimer.singleShot) reliably fires the reload callback * widen train_network error handling, similar to 4472fd3 * deliberately narrow down config errors. (exclude TypeError and ValueError) * watch config file parent directory to detect atomic replacements Connect to the directoryChanged signal and add the parent directory to the watched paths in set_path(). Some editors atomically replace files (delete + rename), which silently drops a file-only watch in QFileSystemWatcher. Watching the parent catches those changes and re-adds the file path in _check_for_change() as before. * guard pose_cfg.get("method") string-type * Add a self._extract_error flag. The error signal sets it to True. _show_success_message checks it and skips the dialog entirely if there was an error. * update tests with narrow error types * Revert narrowing of CONFIG_ERRORS - treat only ValidationError as config-related in show_task_error CONFIG_LOAD_ERRORS includes FileNotFoundError, PermissionError, OSError, TypeError, and ValueError — all of which can originate from non-config sources (e.g. missing checkpoint, disk full, wrong type from a GUI callback) when raised by worker threads. Showing a config-themed dialog with an "Open configuration" button for those errors was misleading. Now show_task_error only trusts ValidationError (which is always from config parsing) to decide on the config dialog. The config-loading recovery loop continues to use the full CONFIG_LOAD_ERRORS tuple where the context is known. Restore TypeError and ValueError to CONFIG_LOAD_ERRORS so the recovery loop properly catches those as user-repairable config errors. * fix MainWindow init _config_monitor * improve error handling * update pyproject.toml and uv.lock * copilot: hardening of PoseConfig-error instance check (Pydantic v2) Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Jaap de Ruyter van Steveninck <32810691+deruyter92@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Some add-on follow-up commits to merge/cherry-pick into #3397
83bf20d8) and deduplicate error handling (35eb0eb6).ProjectConfigthroughout the GUI (a37f5ef6,e4832ed1).d649f6df,2f82e030).83200b18,12d07b2a).1cb4d940).compat.py(4f9cf4af).991d9f77).Tests
Some basic tests for the new behavior are added here: