New logic #66
Open
New logic #66
Conversation
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
Introduces a comprehensive exception hierarchy for application errors, refactors player management to use factory methods and standardized data adapters, and updates Dutch Swiss pairing logic to use explicit color constants (BLACK, WHITE) instead of ambiguous variables. Also moves print utilities to a new location, improves player editing and validation, and enhances logging and UI state management throughout the tournament and player tabs.
…dular view classes under the gui/views directory, improving maintainability and separation of concerns. Adds a new PrintOptionsDialog for flexible print selection, enhances the print output for pairings and standings with improved formatting and layout, and introduces gui_utils for shared GUI helpers. Updates mainwindow to use the new views, improves toolbar and placeholder handling, and updates resource paths and icons. Minor code cleanups and docstring adjustments are included.
Add GitHub Actions workflow for Python package publishing
…update version to PEP 440 compliant format
There was a problem hiding this comment.
Pull request overview
This pull request introduces a comprehensive refactoring of the Gambit Pairing codebase, focusing on improved architecture, exception handling, validation, and code organization. The changes modernize the codebase with factory patterns, better separation of concerns, and enhanced maintainability.
Changes:
- Introduced a structured exception hierarchy with
GambitPairingExceptionbase class and specific exception types - Added validation utilities module with reusable validation functions for email, phone, rating, FIDE ID, etc.
- Refactored tournament management into a package structure with specialized managers (RoundManager, ResultRecorder, TiebreakCalculator)
- Implemented factory pattern for Player creation with proper validation
- Updated type hints from
W/BtoWHITE/BLACKstring constants throughout the codebase - Added API adapter utilities for converting external API responses to Player objects
- Enhanced UI styling and added new SVG icons
- Improved logging to prevent duplicate log path messages
Reviewed changes
Copilot reviewed 52 out of 69 changed files in this pull request and generated 24 comments.
Show a summary per file
| File | Description |
|---|---|
src/gambitpairing/utils/validation.py |
New validation utilities with consistent error handling and ValidationResult pattern |
src/gambitpairing/utils/api_adapters.py |
New API adapters for FIDE/CFC data conversion to Player objects |
src/gambitpairing/utils/command_runner.py |
New command execution utilities with consistent error handling |
src/gambitpairing/tournament/tournament.py |
Refactored Tournament class using manager pattern |
src/gambitpairing/tournament/models.py |
New data models (TournamentConfig, RoundData, MatchResult, PairingHistory) |
src/gambitpairing/tournament/round_manager.py |
Round management logic separated into dedicated manager |
src/gambitpairing/tournament/result_recorder.py |
Result recording logic separated into dedicated manager |
src/gambitpairing/tournament/tiebreak_calculator.py |
Tiebreak calculation logic separated into dedicated calculator |
src/gambitpairing/player/factory.py |
New PlayerFactory implementing factory pattern with validation |
src/gambitpairing/player/base_player.py |
Enhanced Player class with improved validation and documentation |
src/gambitpairing/player/fide_player.py |
Updated FidePlayer with better date handling and validation |
src/gambitpairing/type_hints.py |
Added WHITE/BLACK string constants alongside type literals |
src/gambitpairing/pairing/dutch_swiss.py |
Updated to use WHITE/BLACK constants instead of W/B |
src/gambitpairing/resources/styles.qss |
Updated styling with new button classes and focus removal |
src/gambitpairing/resources/icons/*.svg |
New SVG icons for UI elements |
Comments suppressed due to low confidence (2)
src/gambitpairing/gui/dialogs/manual_pairing_dialog.py:1419
- This assignment to '_auto_pair_selected_player' is unnecessary as it is redefined before this value is used.
def _auto_pair_selected_player(self, item):
src/gambitpairing/gui/views/standings/standings_view.py:35
- Import of 'create_print_button' is not used.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not isinstance(result_selector, ResultSelector): | ||
| return | ||
|
|
||
| white_id = result_selector.property("white_id") |
There was a problem hiding this comment.
Variable white_id is not used.
| return | ||
|
|
||
| white_id = result_selector.property("white_id") | ||
| black_id = result_selector.property("black_id") |
There was a problem hiding this comment.
Variable black_id is not used.
| return self.pairings_table.get_results() | ||
|
|
||
| def log_results_details(self, results_data, round_index_recorded): | ||
| bye_id = self.tournament.rounds_byes_ids[round_index_recorded] |
| @@ -0,0 +1,91 @@ | |||
| from PyQt6 import QtCore, QtGui, QtWidgets | |||
There was a problem hiding this comment.
Import of 'QtCore' is not used.
Import of 'QtGui' is not used.
| from PyQt6 import QtCore, QtGui, QtWidgets | ||
| from PyQt6.QtCore import Qt, pyqtSignal | ||
|
|
||
| from gambitpairing.gui.gui_utils import set_svg_icon |
There was a problem hiding this comment.
Import of 'set_svg_icon' is not used.
Suggested change
| from gambitpairing.gui.gui_utils import set_svg_icon |
Comment on lines
+45
to
+53
| from gambitpairing.constants import ( | ||
| BYE_SCORE, | ||
| DRAW_SCORE, | ||
| LOSS_SCORE, | ||
| RESULT_BLACK_WIN, | ||
| RESULT_DRAW, | ||
| RESULT_WHITE_WIN, | ||
| WIN_SCORE, | ||
| ) |
There was a problem hiding this comment.
Import of 'LOSS_SCORE' is not used.
Import of 'DRAW_SCORE' is not used.
Import of 'RESULT_BLACK_WIN' is not used.
Import of 'RESULT_DRAW' is not used.
Import of 'RESULT_WHITE_WIN' is not used.
| WIN_SCORE, | ||
| ) | ||
| from gambitpairing.gui.dialogs import ManualPairingDialog | ||
| from gambitpairing.gui.gui_utils import get_colored_icon, set_svg_icon |
There was a problem hiding this comment.
Import of 'set_svg_icon' is not used.
Import of 'get_colored_icon' is not used.
| from gambitpairing.gui.views.tournament.utils.pairings_printer import PairingsPrinter | ||
| from gambitpairing.gui.widgets.header import TabHeader | ||
| from gambitpairing.player import Player | ||
| from gambitpairing.resources.resource_utils import get_resource_path |
There was a problem hiding this comment.
Import of 'get_resource_path' is not used.
| painter.end() | ||
| label.setPixmap(pixmap) | ||
| label.setText("") | ||
| except Exception: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| painter.fillRect(pixmap.rect(), QtGui.QColor(color)) | ||
| painter.end() | ||
| return QtGui.QIcon(pixmap) | ||
| except Exception: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
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.
This pull request introduces several improvements and refactorings across the codebase, focusing on exception handling, workflow automation, and user interface enhancements. The most significant changes include a comprehensive overhaul of the application's exception hierarchy, the addition of a GitHub Actions workflow for publishing to PyPI, and improvements to dialogs and player data import logic.
Exception Handling Improvements:
src/gambitpairing/exceptions.pyto introduce a new, structured exception hierarchy. All custom exceptions now inherit from a new base classGambitPairingException, with specific subclasses for pairing, tournament, player, result, validation, resource, API, and configuration errors. This enables more granular and unified error handling throughout the application.Workflow Automation:
.github/workflows/python-publish.ymlthat builds and publishes the Python package to PyPI automatically when a release is created. This streamlines the release process and ensures consistent package deployment.User Interface and Dialog Enhancements:
ManualPairingDialogby disabling floating for the player pool dock widget to prevent detachment issues and adding an_auto_pair_selected_playermethod, allowing users to auto-pair a selected player with the best available opponent based on rating. [1] [2]PlayerManagementDialogto use API adapter functions for importing player data, standardizing field population, improving gender and date-of-birth handling, and providing clearer user notifications. Also improved the copy-to-clipboard functionality and fallback icon handling for the copy button. [1] [2] [3] [4] [5]PrintOptionsDialogto the list of available dialogs, making it accessible in the GUI. [1] [2]Project Metadata and Configuration:
pyproject.tomlto reflect the new repository location. Also added dynamic versioning using thegambitpairing.__version__attribute for setuptools compatibility. [1] [2]pyproject.toml.src/gambitpairing/__init__.pyto remove the "(alpha)" label, reflecting a stable release.These changes collectively enhance code maintainability, user experience, and the project's release process.