Skip to content

Commit 828a54a

Browse files
Decouple fitting from results and add show_fit_results API (#118)
* Decouples fit from results; adds show_fit_results * Updates tutorials for split fit/results API * Standardizes console separators to em dashes * Migrates workflows to Easyscience App auth * Grants app token access to dashboard repo
1 parent daf8566 commit 828a54a

23 files changed

Lines changed: 250 additions & 22 deletions

File tree

.github/workflows/backmerge.yaml

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
# Required repo config:
1010
# https://github.com/organizations/easyscience/settings/secrets/actions
1111
# https://github.com/organizations/easyscience/settings/variables/actions
12-
# - Actions secret: BACKMERGE_PRIVATE_KEY (GitHub App private key PEM)
13-
# - Actions variable: BACKMERGE_APP_ID (GitHub App ID)
12+
# - Actions secret: EASYSCIENCE_APP_KEY (GitHub App private key PEM)
13+
# - Actions variable: EASYSCIENCE_APP_ID (GitHub App ID)
1414
# The GitHub App must be added to the develop branch ruleset bypass list.
1515

1616
name: Backmerge (master -> develop)
@@ -31,19 +31,19 @@ jobs:
3131
id: app-token
3232
uses: actions/create-github-app-token@v2
3333
with:
34-
app-id: ${{ vars.BACKMERGE_APP_ID }}
35-
private-key: ${{ secrets.BACKMERGE_PRIVATE_KEY }}
34+
app-id: ${{ vars.EASYSCIENCE_APP_ID }}
35+
private-key: ${{ secrets.EASYSCIENCE_APP_KEY }}
3636

3737
- name: Checkout repository
3838
uses: actions/checkout@v5
3939
with:
4040
fetch-depth: 0
4141
token: ${{ steps.app-token.outputs.token }}
4242

43-
- name: Configure git
43+
- name: Configure git for pushing
4444
run: |
45-
git config user.name "es-backmerge[bot]"
46-
git config user.email "${{ vars.BACKMERGE_APP_ID }}+es-backmerge[bot]@users.noreply.github.com"
45+
git config user.name "easyscience[bot]"
46+
git config user.email "${{ vars.EASYSCIENCE_APP_ID }}+easyscience[bot]@users.noreply.github.com"
4747
4848
- name: Merge master into develop
4949
run: |

.github/workflows/dashboard.yaml

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,19 @@ jobs:
2424
runs-on: ubuntu-latest
2525

2626
steps:
27+
# Create GitHub App token for pushing to external dashboard repo.
28+
# The 'repositories' parameter is required to grant access to repos
29+
# other than the one where the workflow is running.
30+
- name: Create GitHub App installation token
31+
id: app-token
32+
uses: actions/create-github-app-token@v2
33+
with:
34+
app-id: ${{ vars.EASYSCIENCE_APP_ID }}
35+
private-key: ${{ secrets.EASYSCIENCE_APP_KEY }}
36+
repositories: |
37+
${{ github.event.repository.name }}
38+
dashboard
39+
2740
- name: Checkout repository
2841
uses: actions/checkout@v5
2942
with:
@@ -80,7 +93,7 @@ jobs:
8093
with:
8194
external_repository: ${{ env.REPO_OWNER }}/dashboard
8295
publish_branch: ${{ env.DEFAULT_BRANCH }}
83-
personal_token: ${{ secrets.GH_API_PERSONAL_ACCESS_TOKEN }}
96+
personal_token: ${{ steps.app-token.outputs.token }}
8497
publish_dir: ./_dashboard_publish
8598
keep_files: true
8699

.github/workflows/docs.yaml

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,20 @@ jobs:
8585
echo "DOCS_VERSION=${DOCS_VERSION}" >> "$GITHUB_ENV"
8686
echo "DEPLOYMENT_URL=https://easyscience.github.io/${{ github.event.repository.name }}/${DOCS_VERSION}" >> "$GITHUB_ENV"
8787
88+
# Create GitHub App token for pushing to gh-pages as easyscience[bot].
89+
- name: Create GitHub App installation token
90+
id: app-token
91+
uses: actions/create-github-app-token@v2
92+
with:
93+
app-id: ${{ vars.EASYSCIENCE_APP_ID }}
94+
private-key: ${{ secrets.EASYSCIENCE_APP_KEY }}
95+
8896
# Check out the repository source code.
8997
# Note: The gh-pages branch is fetched separately later for mike deployment.
9098
- name: Check-out repository
9199
uses: actions/checkout@v5
100+
with:
101+
token: ${{ steps.app-token.outputs.token }}
92102

93103
# Activate dark mode to create documentation with Plotly charts in dark mode
94104
# Need a better solution to automatically switch the chart colour theme based on the mkdocs material switcher
@@ -179,8 +189,8 @@ jobs:
179189
# Configure git user for mike to commit and push to gh-pages branch.
180190
- name: Configure git for pushing
181191
run: |
182-
git config user.name "github-actions[bot]"
183-
git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
192+
git config user.name "easyscience[bot]"
193+
git config user.email "${{ vars.EASYSCIENCE_APP_ID }}+easyscience[bot]@users.noreply.github.com"
184194
185195
# Fetch the gh-pages branch to ensure mike has the latest remote state.
186196
# This is required because the checkout step only fetches the source branch,

.github/workflows/release-pr.yaml

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
1-
# This workflow creates an automated release PR from `develop` into `main`.
1+
# This workflow creates an automated release PR from `develop` into `master`.
22
#
33
# Usage:
44
# - Triggered manually via workflow_dispatch.
55
# - Creates a PR titled "Release: merge develop into master".
66
# - Adds the label "[maintainer] auto-pull-request" so it is excluded from changelogs.
77
# - The PR body makes clear that this is automation only (no review needed).
8+
#
9+
# Required repo config:
10+
# https://github.com/organizations/easyscience/settings/secrets/actions
11+
# https://github.com/organizations/easyscience/settings/variables/actions
12+
# - Actions secret: EASYSCIENCE_APP_KEY (GitHub App private key PEM)
13+
# - Actions variable: EASYSCIENCE_APP_ID (GitHub App ID)
814

915
name: Release PR (develop -> master)
1016

@@ -13,7 +19,7 @@ on:
1319
workflow_dispatch:
1420

1521
permissions:
16-
contents: write
22+
contents: read
1723
pull-requests: write
1824

1925
# Set the environment variables to be used in all jobs defined in this workflow
@@ -24,10 +30,18 @@ jobs:
2430
create-pull-request:
2531
runs-on: ubuntu-latest
2632
steps:
33+
- name: Create GitHub App installation token
34+
id: app-token
35+
uses: actions/create-github-app-token@v2
36+
with:
37+
app-id: ${{ vars.EASYSCIENCE_APP_ID }}
38+
private-key: ${{ secrets.EASYSCIENCE_APP_KEY }}
39+
2740
- name: Checkout develop branch
2841
uses: actions/checkout@v5
2942
with:
3043
ref: develop
44+
token: ${{ steps.app-token.outputs.token }}
3145

3246
- name: Create PR from develop to ${{ env.DEFAULT_BRANCH }}
3347
run: |
@@ -36,8 +50,8 @@ jobs:
3650
--head develop \
3751
--title "Release: merge develop into ${{ env.DEFAULT_BRANCH }}" \
3852
--label "[maintainer] auto-pull-request" \
39-
--body "⚠️ This PR is created automatically to trigger the release pipeline. It merges the accumulated changes from \`develop\` into \`${{ env.DEFAULT_BRANCH }}\`.
53+
--body "This PR is created automatically to trigger the release pipeline. It merges the accumulated changes from \`develop\` into \`${{ env.DEFAULT_BRANCH }}\`.
4054
4155
It is labeled \`[maintainer] auto-pull-request\` and is excluded from release notes and version bump logic."
4256
env:
43-
GITHUB_TOKEN: ${{ secrets.GH_API_PERSONAL_ACCESS_TOKEN }}
57+
GH_TOKEN: ${{ steps.app-token.outputs.token }}

src/easydiffraction/analysis/analysis.py

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,10 +501,23 @@ def fit(self):
501501
"""Execute fitting using the selected mode, calculator and
502502
minimizer.
503503
504+
This method performs the optimization but does not display
505+
results automatically. Call :meth:`show_fit_results` after
506+
fitting to see a summary of the fit quality and parameter
507+
values.
508+
504509
In 'single' mode, fits each experiment independently. In
505510
'joint' mode, performs a simultaneous fit across experiments
506511
with weights.
507-
Sets :attr:`fit_results` on success.
512+
513+
Sets :attr:`fit_results` on success, which can be accessed
514+
programmatically
515+
(e.g., ``analysis.fit_results.reduced_chi_square``).
516+
517+
Example::
518+
519+
project.analysis.fit()
520+
project.analysis.show_fit_results() # Display results
508521
"""
509522
sample_models = self.project.sample_models
510523
if not sample_models:
@@ -554,6 +567,30 @@ def fit(self):
554567
# After fitting, get the results
555568
self.fit_results = self.fitter.results
556569

570+
def show_fit_results(self) -> None:
571+
"""Display a summary of the fit results.
572+
573+
Renders the fit quality metrics (reduced χ², R-factors) and a
574+
table of fitted parameters with their starting values, final
575+
values, and uncertainties.
576+
577+
This method should be called after :meth:`fit` completes. If no
578+
fit has been performed yet, a warning is logged.
579+
580+
Example::
581+
582+
project.analysis.fit()
583+
project.analysis.show_fit_results()
584+
"""
585+
if not hasattr(self, 'fit_results') or self.fit_results is None:
586+
log.warning('No fit results available. Run fit() first.')
587+
return
588+
589+
sample_models = self.project.sample_models
590+
experiments = self.project.experiments
591+
592+
self.fitter._process_fit_results(sample_models, experiments)
593+
557594
def _update_categories(self, called_by_minimizer=False) -> None:
558595
"""Update all categories owned by Analysis.
559596

src/easydiffraction/analysis/fitting.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ def fit(
3737
) -> None:
3838
"""Run the fitting process.
3939
40+
This method performs the optimization but does not display
41+
results. Use :meth:`show_fit_results` on the Analysis object
42+
to display the fit results after fitting is complete.
43+
4044
Args:
4145
sample_models: Collection of sample models.
4246
experiments: Collection of experiments.
@@ -66,15 +70,17 @@ def objective_function(engine_params: Dict[str, Any]) -> np.ndarray:
6670
# Perform fitting
6771
self.results = self.minimizer.fit(params, objective_function)
6872

69-
# Post-fit processing
70-
self._process_fit_results(sample_models, experiments)
71-
7273
def _process_fit_results(
7374
self,
7475
sample_models: SampleModels,
7576
experiments: Experiments,
7677
) -> None:
77-
"""Collect reliability inputs and display results after fitting.
78+
"""Collect reliability inputs and display fit results.
79+
80+
This method is typically called by
81+
:meth:`Analysis.show_fit_results` rather than directly. It
82+
calculates R-factors and other metrics, then renders them to
83+
the console.
7884
7985
Args:
8086
sample_models: Collection of sample models.

src/easydiffraction/utils/logging.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -554,8 +554,8 @@ def paragraph(cls, title: str) -> None:
554554
def section(cls, title: str) -> None:
555555
"""Formats a section header with bold green text."""
556556
full_title = f'{title.upper()}'
557-
line = '' * len(full_title)
558-
formatted = f'[bold green]\n{line}\n{full_title}\n{line}[/bold green]'
557+
line = '' * len(full_title)
558+
formatted = f'[bold green]{line}\n{full_title}\n{line}[/bold green]'
559559
if not in_jupyter():
560560
formatted = f'\n{formatted}'
561561
cls._console.print(formatted)
@@ -566,7 +566,7 @@ def chapter(cls, title: str) -> None:
566566
and padding.
567567
"""
568568
width = ConsoleManager._detect_width()
569-
symbol = ''
569+
symbol = ''
570570
full_title = f' {title.upper()} '
571571
pad_len = (width - len(full_title)) // 2
572572
padding = symbol * pad_len

tests/unit/easydiffraction/analysis/test_analysis.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,56 @@ def test_fit_modes_show_and_switch_to_joint(monkeypatch, capsys):
8282
out2 = capsys.readouterr().out
8383
assert 'Current fit mode changed to' in out2
8484
assert a.fit_mode == 'joint'
85+
86+
87+
def test_show_fit_results_warns_when_no_results(capsys):
88+
"""Test that show_fit_results logs a warning when fit() has not been run."""
89+
from easydiffraction.analysis.analysis import Analysis
90+
91+
a = Analysis(project=_make_project_with_names([]))
92+
93+
# Ensure fit_results is not set
94+
assert not hasattr(a, 'fit_results') or a.fit_results is None
95+
96+
a.show_fit_results()
97+
out = capsys.readouterr().out
98+
assert 'No fit results available' in out
99+
100+
101+
def test_show_fit_results_calls_process_fit_results(monkeypatch):
102+
"""Test that show_fit_results delegates to fitter._process_fit_results."""
103+
from easydiffraction.analysis.analysis import Analysis
104+
105+
# Track if _process_fit_results was called
106+
process_called = {'called': False, 'args': None}
107+
108+
def mock_process_fit_results(sample_models, experiments):
109+
process_called['called'] = True
110+
process_called['args'] = (sample_models, experiments)
111+
112+
# Create a mock project with sample_models and experiments
113+
class MockProject:
114+
sample_models = object()
115+
experiments = object()
116+
_varname = 'proj'
117+
118+
class experiments_cls:
119+
names = []
120+
121+
experiments = experiments_cls()
122+
123+
project = MockProject()
124+
project.sample_models = object()
125+
project.experiments.names = []
126+
127+
a = Analysis(project=project)
128+
129+
# Set up fit_results so show_fit_results doesn't return early
130+
a.fit_results = object()
131+
132+
# Mock the fitter's _process_fit_results method
133+
monkeypatch.setattr(a.fitter, '_process_fit_results', mock_process_fit_results)
134+
135+
a.show_fit_results()
136+
137+
assert process_called['called'], '_process_fit_results should be called'

tests/unit/easydiffraction/analysis/test_fitting.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,59 @@ def fit(self, params, obj):
3434
f.fit(sample_models=DummyCollection(), experiments=DummyCollection())
3535
out = capsys.readouterr().out
3636
assert 'No parameters selected for fitting' in out
37+
38+
39+
def test_fitter_fit_does_not_call_process_fit_results(monkeypatch):
40+
"""Test that Fitter.fit() does not automatically call _process_fit_results.
41+
42+
The display of results is now the responsibility of Analysis.show_fit_results().
43+
"""
44+
from easydiffraction.analysis.fitting import Fitter
45+
46+
process_called = {'called': False}
47+
48+
class DummyParam:
49+
value = 1.0
50+
_fit_start_value = None
51+
52+
class DummyCollection:
53+
free_parameters = [DummyParam()]
54+
55+
def __init__(self):
56+
self._names = ['e1']
57+
58+
@property
59+
def names(self):
60+
return self._names
61+
62+
class MockFitResults:
63+
pass
64+
65+
class DummyMin:
66+
tracker = type('T', (), {'track': staticmethod(lambda a, b: a)})()
67+
68+
def fit(self, params, obj):
69+
return MockFitResults()
70+
71+
def _sync_result_to_parameters(self, params, engine_params):
72+
pass
73+
74+
f = Fitter()
75+
f.minimizer = DummyMin()
76+
77+
# Track if _process_fit_results is called
78+
original_process = f._process_fit_results
79+
80+
def mock_process(*args, **kwargs):
81+
process_called['called'] = True
82+
return original_process(*args, **kwargs)
83+
84+
monkeypatch.setattr(f, '_process_fit_results', mock_process)
85+
86+
f.fit(sample_models=DummyCollection(), experiments=DummyCollection())
87+
88+
assert not process_called['called'], (
89+
'Fitter.fit() should not call _process_fit_results automatically. '
90+
'Use Analysis.show_fit_results() instead.'
91+
)
92+
assert f.results is not None, 'Fitter.fit() should still set results'

tests/unit/easydiffraction/utils/test_utils.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ def test_get_version_for_url_released(monkeypatch):
189189
assert MUT._get_version_for_url() == '0.8.0.post1'
190190

191191

192+
@pytest.mark.filterwarnings('ignore:Failed to fetch tutorials index:UserWarning')
192193
def test_fetch_tutorials_index_returns_empty_on_error(monkeypatch):
193194
import easydiffraction.utils.utils as MUT
194195

0 commit comments

Comments
 (0)