-
Notifications
You must be signed in to change notification settings - Fork 0
Decouple fitting from results and add show_fit_results API
#118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
show_fit_results API
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #118 +/- ##
===========================================
- Coverage 83.15% 82.93% -0.22%
===========================================
Files 89 89
Lines 5455 5461 +6
Branches 452 453 +1
===========================================
- Hits 4536 4529 -7
- Misses 753 761 +8
- Partials 166 171 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this 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 decouples the display of fitting results from the fitting process itself by introducing a new show_fit_results() API method. Previously, fit results were automatically displayed immediately after calling fit(). Now, users must explicitly call show_fit_results() to see the output, providing more flexibility in how and when results are displayed.
Key changes:
- Added
Analysis.show_fit_results()method to explicitly display fit results after fitting - Modified
Fitter.fit()to no longer automatically call_process_fit_results() - Updated all tutorial files to include explicit
show_fit_results()calls afterfit()
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/easydiffraction/analysis/analysis.py | Adds new show_fit_results() method that delegates to fitter's _process_fit_results() and updates fit() docstring to document the new workflow |
| src/easydiffraction/analysis/fitting.py | Removes automatic call to _process_fit_results() from fit() and updates documentation to indicate results display is now separate |
| tests/unit/easydiffraction/analysis/test_fitting.py | Adds test to verify Fitter.fit() no longer automatically calls _process_fit_results() |
| tests/unit/easydiffraction/analysis/test_analysis.py | Adds tests for the new show_fit_results() method including warning when no results exist and delegation to fitter |
| tutorials/ed-1.py through ed-13.py | Updates all tutorial files to include explicit show_fit_results() calls after fitting |
| tests/unit/easydiffraction/utils/test_utils.py | Adds pytest warning filter to suppress expected UserWarning in test |
| src/easydiffraction/utils/logging.py | Changes Unicode characters for section and chapter separators from box drawing characters to em dashes |
| .github/workflows/release-pr.yaml | Updates to use GitHub App authentication instead of personal access token |
| .github/workflows/docs.yaml | Updates to use GitHub App authentication for gh-pages deployment |
| .github/workflows/dashboard.yaml | Updates to use GitHub App authentication for dashboard deployment |
| .github/workflows/backmerge.yaml | Updates to use unified GitHub App credentials and bot name |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| full_title = f'{title.upper()}' | ||
| line = '━' * len(full_title) | ||
| formatted = f'[bold green]\n{line}\n{full_title}\n{line}[/bold green]' | ||
| line = '—' * len(full_title) |
Copilot
AI
Dec 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The character changed from box drawing character '━' (U+2501, BOX DRAWINGS HEAVY HORIZONTAL) to em dash '—' (U+2014, EM DASH). Box drawing characters are specifically designed for creating visual lines and borders in terminal output and typically render more consistently across terminals. The em dash is a punctuation mark and may not create visually continuous lines. Consider whether this change was intentional, as it may affect the visual appearance of section headers in console output.
| """ | ||
| width = ConsoleManager._detect_width() | ||
| symbol = '─' | ||
| symbol = '—' |
Copilot
AI
Dec 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The character changed from box drawing character '─' (U+2500, BOX DRAWINGS LIGHT HORIZONTAL) to em dash '—' (U+2014, EM DASH). Box drawing characters are specifically designed for creating visual lines and borders in terminal output and typically render more consistently across terminals. The em dash is a punctuation mark and may not create visually continuous lines. Consider whether this change was intentional, as it may affect the visual appearance of chapter headers in console output.
| # Create a mock project with sample_models and experiments | ||
| class MockProject: | ||
| sample_models = object() | ||
| experiments = object() |
Copilot
AI
Dec 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment to 'experiments' is unnecessary as it is redefined before this value is used.
| experiments = object() |
No description provided.