From 850ba8732a3cc396d5a2b9cfcf05f76d14731964 Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Mon, 4 May 2026 23:46:36 +0200 Subject: [PATCH 01/13] Enhance hover labels in Plotly for clearer view --- .../display/plotters/plotly.py | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/easydiffraction/display/plotters/plotly.py b/src/easydiffraction/display/plotters/plotly.py index 969761528..61cc77abd 100644 --- a/src/easydiffraction/display/plotters/plotly.py +++ b/src/easydiffraction/display/plotters/plotly.py @@ -385,6 +385,7 @@ def _get_powder_trace( line=line, mode=mode, name=name, + hovertemplate=f'{name}
x: %{{x}}
y: %{{y}}', ) @staticmethod @@ -643,16 +644,14 @@ def _get_bragg_tick_trace( y = np.full(tick_set.x.shape, row_y, dtype=float) hover_text = [] for idx, x_value in enumerate(tick_set.x): - hkl_text = ( - f'hkl: ({int(tick_set.h[idx])} ' - f'{int(tick_set.k[idx])} {int(tick_set.ell[idx])})
' - ) hover_text.append( - f'phase_id: {tick_set.phase_id}
' - f'{hkl_text}' + f'Bragg peaks: {tick_set.phase_id}
' + f'hkl: ({int(tick_set.h[idx])} {int(tick_set.k[idx])} ' + f'{int(tick_set.ell[idx])})
' f'x: {float(x_value):.6g}
' - f'f_squared_calc: {float(tick_set.f_squared_calc[idx]):.6g}
' - f'f_calc: {float(tick_set.f_calc[idx]):.6g}' + # f'F²cal:{float(tick_set.f_squared_calc[idx]):.6g}
' + # f'Fcalc:{float(tick_set.f_calc[idx]):.6g}' + '' ) return go.Scatter( @@ -662,13 +661,16 @@ def _get_bragg_tick_trace( marker={ 'symbol': 'line-ns-open', 'size': 12, - 'line': {'color': color, 'width': 1}, + 'line': {'width': 1}, 'color': color, }, - name=f'Bragg ({tick_set.phase_id})', + name=f'Bragg peaks ({tick_set.phase_id})', text=hover_text, + hoverlabel={ + 'font': {'color': 'white'}, + 'bordercolor': 'white', + }, hovertemplate='%{text}', - showlegend=False, ) @staticmethod From ad74bb1eecc6ef205fac0525e87cf6b7a1264b80 Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 00:20:59 +0200 Subject: [PATCH 02/13] Refactor Plotly plotter to improve layout management and hover label clarity --- .../display/plotters/plotly.py | 136 ++++++++++++------ .../display/plotters/test_plotly.py | 28 ++-- 2 files changed, 112 insertions(+), 52 deletions(-) diff --git a/src/easydiffraction/display/plotters/plotly.py b/src/easydiffraction/display/plotters/plotly.py index 61cc77abd..cc2b9800f 100644 --- a/src/easydiffraction/display/plotters/plotly.py +++ b/src/easydiffraction/display/plotters/plotly.py @@ -51,6 +51,13 @@ NICE_AXIS_FRACTIONS = (1.0, 2.0, 5.0, 10.0) DISPLAY_TICK_FRACTIONS = (1.0, 2.0, 2.5, 4.0, 5.0, 7.5, 10.0) PLOTLY_HEIGHT_PER_UNIT = 24 +BRAGG_TICK_MARKER_SIZE = 12 +BRAGG_TICK_MARKER_LINE_WIDTH = 1 +BRAGG_TICK_SYMBOL_HEIGHT_SCALE = 1.4 +COMPOSITE_VERTICAL_SPACING = 0.03 +COMPOSITE_MARGIN_RIGHT = 30 +COMPOSITE_MARGIN_TOP = 40 +COMPOSITE_MARGIN_BOTTOM = 45 @dataclass(frozen=True) @@ -58,11 +65,9 @@ class PowderCompositeRows: """Resolved row layout for the composite powder figure.""" row_count: int - normalized_heights: list[float] + row_heights: list[float] bragg_row: int | None residual_row: int | None - total_weight: float - baseline_weight: float class PlotlyPlotter(PlotterBase): @@ -646,7 +651,7 @@ def _get_bragg_tick_trace( for idx, x_value in enumerate(tick_set.x): hover_text.append( f'Bragg peaks: {tick_set.phase_id}
' - f'hkl: ({int(tick_set.h[idx])} {int(tick_set.k[idx])} ' + f'Miller indices: ({int(tick_set.h[idx])} {int(tick_set.k[idx])} ' f'{int(tick_set.ell[idx])})
' f'x: {float(x_value):.6g}
' # f'F²cal:{float(tick_set.f_squared_calc[idx]):.6g}
' @@ -660,11 +665,11 @@ def _get_bragg_tick_trace( mode='markers', marker={ 'symbol': 'line-ns-open', - 'size': 12, - 'line': {'width': 1}, + 'size': BRAGG_TICK_MARKER_SIZE, + 'line': {'width': BRAGG_TICK_MARKER_LINE_WIDTH}, 'color': color, }, - name=f'Bragg peaks ({tick_set.phase_id})', + name=f'Bragg peaks: {tick_set.phase_id}', text=hover_text, hoverlabel={ 'font': {'color': 'white'}, @@ -704,23 +709,73 @@ def _get_display_tick_limit(raw_limit: float) -> float: return DISPLAY_TICK_FRACTIONS[0] * base @staticmethod - def _scaled_bragg_row_height(plot_spec: PowderMeasVsCalcSpec) -> float: - """ - Return Bragg-row weight for the current number of phases. - """ - phase_count = len(plot_spec.bragg_tick_sets) - if phase_count == 0: - return 0.0 + def _base_composite_height_pixels(plot_spec: PowderMeasVsCalcSpec) -> float: + """Return the baseline figure height for a single-phase plot.""" + if plot_spec.height is None: + return float(DEFAULT_HEIGHT * PLOTLY_HEIGHT_PER_UNIT) + return float(plot_spec.height) - return plot_spec.bragg_peaks_height_fraction * phase_count + @staticmethod + def _composite_plot_area_height(full_height: float) -> float: + """Return the drawable plot area height after vertical margins.""" + return max(full_height - COMPOSITE_MARGIN_TOP - COMPOSITE_MARGIN_BOTTOM, 1.0) + + @staticmethod + def _subplot_available_height_fraction(row_count: int) -> float: + """Return the fraction of plot height available for subplot rows.""" + return 1.0 - COMPOSITE_VERTICAL_SPACING * max(row_count - 1, 0) + + @staticmethod + def _bragg_tick_symbol_height_pixels() -> float: + """Return rendered pixel height for one Bragg tick marker.""" + return ( + BRAGG_TICK_MARKER_SIZE * BRAGG_TICK_SYMBOL_HEIGHT_SCALE + + BRAGG_TICK_MARKER_LINE_WIDTH + ) + + @staticmethod + def _bragg_row_height_pixels(plot_spec: PowderMeasVsCalcSpec) -> float: + """Return the exact Bragg-row pixel height for the current phases.""" + return float( + len(plot_spec.bragg_tick_sets) + * PlotlyPlotter._bragg_tick_symbol_height_pixels() + ) + + @classmethod + def _baseline_non_bragg_row_heights( + cls, + plot_spec: PowderMeasVsCalcSpec, + row_count: int, + has_bragg_ticks: bool, + has_residual: bool, + ) -> tuple[float, float | None]: + """Return baseline main and residual row heights in pixels.""" + baseline_height = cls._base_composite_height_pixels(plot_spec) + plot_area_height = cls._composite_plot_area_height(baseline_height) + available_row_pixels = plot_area_height * cls._subplot_available_height_fraction(row_count) + baseline_bragg_pixels = float(cls._bragg_tick_symbol_height_pixels() if has_bragg_ticks else 0) + non_bragg_pixels = max(available_row_pixels - baseline_bragg_pixels, 1.0) + + if not has_residual: + return non_bragg_pixels, None + + main_pixels = non_bragg_pixels / (1.0 + plot_spec.residual_height_fraction) + residual_pixels = main_pixels * plot_spec.residual_height_fraction + return main_pixels, residual_pixels @staticmethod def _get_powder_composite_rows(plot_spec: PowderMeasVsCalcSpec) -> PowderCompositeRows: """Resolve subplot rows for the composite powder figure.""" has_bragg_ticks = bool(plot_spec.bragg_tick_sets) has_residual = plot_spec.y_resid is not None - row_heights = [1.0] - baseline_weight = 1.0 + row_count = 1 + int(has_bragg_ticks) + int(has_residual) + main_row_height, residual_row_height = PlotlyPlotter._baseline_non_bragg_row_heights( + plot_spec=plot_spec, + row_count=row_count, + has_bragg_ticks=has_bragg_ticks, + has_residual=has_residual, + ) + row_heights = [main_row_height] bragg_row = None residual_row = None next_row = 2 @@ -728,36 +783,35 @@ def _get_powder_composite_rows(plot_spec: PowderMeasVsCalcSpec) -> PowderComposi if has_bragg_ticks: bragg_row = next_row next_row += 1 - row_heights.append(PlotlyPlotter._scaled_bragg_row_height(plot_spec)) - baseline_weight += plot_spec.bragg_peaks_height_fraction + row_heights.append(PlotlyPlotter._bragg_row_height_pixels(plot_spec)) if has_residual: residual_row = next_row - row_heights.append(plot_spec.residual_height_fraction) - baseline_weight += plot_spec.residual_height_fraction + row_heights.append(residual_row_height if residual_row_height is not None else 1.0) - total_height = sum(row_heights) - normalized_heights = [row_height / total_height for row_height in row_heights] return PowderCompositeRows( - row_count=1 + int(has_bragg_ticks) + int(has_residual), - normalized_heights=normalized_heights, + row_count=row_count, + row_heights=row_heights, bragg_row=bragg_row, residual_row=residual_row, - total_weight=total_height, - baseline_weight=baseline_weight, ) - @staticmethod + @classmethod def _composite_figure_height( + cls, plot_spec: PowderMeasVsCalcSpec, layout: PowderCompositeRows, - ) -> int: - """Return figure height scaled by Bragg-row growth.""" - if plot_spec.height is None: - base_pixels = DEFAULT_HEIGHT * PLOTLY_HEIGHT_PER_UNIT - else: - base_pixels = plot_spec.height - scaled_pixels = np.ceil(base_pixels * layout.total_weight / layout.baseline_weight) - return int(scaled_pixels) + ) -> float: + """Return figure height with Bragg growth from a single-phase baseline.""" + base_pixels = cls._base_composite_height_pixels(plot_spec) + phase_count = len(plot_spec.bragg_tick_sets) + if phase_count <= 1: + return base_pixels + + added_bragg_pixels = float( + (phase_count - 1) * cls._bragg_tick_symbol_height_pixels() + ) + growth_pixels = added_bragg_pixels / cls._subplot_available_height_fraction(layout.row_count) + return base_pixels + growth_pixels @classmethod def _get_residual_limit(cls, plot_spec: PowderMeasVsCalcSpec) -> float: @@ -801,8 +855,8 @@ def plot_powder_meas_vs_calc( rows=layout.row_count, cols=1, shared_xaxes=True, - vertical_spacing=0.04, - row_heights=layout.normalized_heights, + vertical_spacing=COMPOSITE_VERTICAL_SPACING, + row_heights=layout.row_heights, ) fig.add_trace(self._get_powder_trace(plot_spec.x, plot_spec.y_meas, 'meas'), row=1, col=1) @@ -833,9 +887,9 @@ def plot_powder_meas_vs_calc( height=self._composite_figure_height(plot_spec, layout), margin={ 'autoexpand': True, - 'r': 30, - 't': 40, - 'b': 45, + 'r': COMPOSITE_MARGIN_RIGHT, + 't': COMPOSITE_MARGIN_TOP, + 'b': COMPOSITE_MARGIN_BOTTOM, }, title={'text': plot_spec.title}, legend={ diff --git a/tests/unit/easydiffraction/display/plotters/test_plotly.py b/tests/unit/easydiffraction/display/plotters/test_plotly.py index 7db809f08..6a206654a 100644 --- a/tests/unit/easydiffraction/display/plotters/test_plotly.py +++ b/tests/unit/easydiffraction/display/plotters/test_plotly.py @@ -297,16 +297,20 @@ def fake_show_figure(self, fig): assert fig.layout.xaxis2.matches == 'x' assert fig.layout.xaxis3.matches == 'x' + plot_area_height = fig.layout.height - fig.layout.margin.t - fig.layout.margin.b main_height = fig.layout.yaxis.domain[1] - fig.layout.yaxis.domain[0] bragg_height = fig.layout.yaxis2.domain[1] - fig.layout.yaxis2.domain[0] residual_height = fig.layout.yaxis3.domain[1] - fig.layout.yaxis3.domain[0] assert residual_height == pytest.approx(main_height * 0.25) - assert bragg_height == pytest.approx( - main_height * pp.PlotlyPlotter._scaled_bragg_row_height(plot_spec) + assert plot_area_height * bragg_height == pytest.approx( + 2 * pp.PlotlyPlotter._bragg_tick_symbol_height_pixels() ) bragg_traces = [trace for trace in fig.data if trace.name.startswith('Bragg')] - assert [trace.name for trace in bragg_traces] == ['Bragg (phase-a)', 'Bragg (phase-b)'] + assert [trace.name for trace in bragg_traces] == [ + 'Bragg peaks: phase-a', + 'Bragg peaks: phase-b', + ] assert list(bragg_traces[0].y) == [1.0] assert list(bragg_traces[1].y) == [2.0] assert list(fig.layout.yaxis2.ticktext) == ['phase-a', 'phase-b'] @@ -315,11 +319,11 @@ def fake_show_figure(self, fig): assert fig.layout.yaxis3.title.text is None assert fig.layout.yaxis3.zeroline is False assert fig.layout.xaxis3.title.text == '2θ (degree)' - assert 'hkl: (1 0 1)' in bragg_traces[0].text[0] - assert 'f_squared_calc: 100' in bragg_traces[0].text[0] + assert 'Miller indices: (1 0 1)' in bragg_traces[0].text[0] + assert 'Bragg peaks: phase-a' in bragg_traces[0].text[0] -def test_scaled_bragg_row_height_scales_linearly_with_phase_count(): +def test_bragg_row_height_pixels_scale_linearly_with_phase_count(): from easydiffraction.display.plotters.base import BraggTickSet from easydiffraction.display.plotters.base import PowderMeasVsCalcSpec from easydiffraction.display.plotters.plotly import PlotlyPlotter @@ -370,10 +374,11 @@ def test_scaled_bragg_row_height_scales_linearly_with_phase_count(): height=single_phase.height, ) - single_height = PlotlyPlotter._scaled_bragg_row_height(single_phase) - two_phase_height = PlotlyPlotter._scaled_bragg_row_height(two_phase) - assert single_height == pytest.approx(0.10) - assert two_phase_height == pytest.approx(0.20) + symbol_height = PlotlyPlotter._bragg_tick_symbol_height_pixels() + single_height = PlotlyPlotter._bragg_row_height_pixels(single_phase) + two_phase_height = PlotlyPlotter._bragg_row_height_pixels(two_phase) + assert single_height == pytest.approx(symbol_height) + assert two_phase_height == pytest.approx(2 * symbol_height) def test_plot_powder_meas_vs_calc_grows_total_height_for_many_phases(monkeypatch): @@ -423,7 +428,8 @@ def plot_spec(phase_count: int) -> PowderMeasVsCalcSpec: def row_height_pixels(fig, axis_name: str) -> float: axis = getattr(fig.layout, axis_name) - return fig.layout.height * (axis.domain[1] - axis.domain[0]) + plot_area_height = fig.layout.height - fig.layout.margin.t - fig.layout.margin.b + return plot_area_height * (axis.domain[1] - axis.domain[0]) assert multi_fig.layout.height > single_fig.layout.height assert row_height_pixels(multi_fig, 'yaxis') == pytest.approx( From f35b1ba457d7f17bbce38972f28285a93106220d Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 00:40:10 +0200 Subject: [PATCH 03/13] Fix Plotly powder scaling and residual axis sync --- .../display/plotters/plotly.py | 29 +++++++++++++++++-- .../display/plotters/test_plotly.py | 16 ++++++++-- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/easydiffraction/display/plotters/plotly.py b/src/easydiffraction/display/plotters/plotly.py index cc2b9800f..f96138dd0 100644 --- a/src/easydiffraction/display/plotters/plotly.py +++ b/src/easydiffraction/display/plotters/plotly.py @@ -813,6 +813,21 @@ def _composite_figure_height( growth_pixels = added_bragg_pixels / cls._subplot_available_height_fraction(layout.row_count) return base_pixels + growth_pixels + @classmethod + def _get_main_intensity_range(cls, plot_spec: PowderMeasVsCalcSpec) -> tuple[float, float]: + """Return an explicit y-range for the main powder intensity row.""" + y_meas = np.asarray(plot_spec.y_meas) + y_calc = np.asarray(plot_spec.y_calc) + if min(y_meas.size, y_calc.size) == 0: + return 0.0, 1.0 + + main_y_min = float(min(np.min(y_meas), np.min(y_calc))) + main_y_max = float(max(np.max(y_meas), np.max(y_calc))) + lower_limit = 0.0 if main_y_min >= 0.0 else main_y_min + if main_y_max <= lower_limit: + return lower_limit - 1.0, lower_limit + 1.0 + return lower_limit, main_y_max + @classmethod def _get_residual_limit(cls, plot_spec: PowderMeasVsCalcSpec) -> float: """Return a symmetric residual limit matched to the main row.""" @@ -825,8 +840,7 @@ def _get_residual_limit(cls, plot_spec: PowderMeasVsCalcSpec) -> float: if min(y_meas.size, y_calc.size, y_resid.size) == 0: return 1.0 - main_y_min = float(min(np.min(y_meas), np.min(y_calc))) - main_y_max = float(max(np.max(y_meas), np.max(y_calc))) + main_y_min, main_y_max = cls._get_main_intensity_range(plot_spec) main_y_range = max(main_y_max - main_y_min, 0.0) scale_matched_half_range = 0.5 * main_y_range * plot_spec.residual_height_fraction if scale_matched_half_range > 0.0: @@ -850,6 +864,8 @@ def plot_powder_meas_vs_calc( has_x_values = x_values.size > 0 x_min = float(np.min(x_values)) if has_x_values else None x_max = float(np.max(x_values)) if has_x_values else None + main_y_min, main_y_max = self._get_main_intensity_range(plot_spec) + residual_limit = None fig = make_subplots( rows=layout.row_count, @@ -923,7 +939,12 @@ def plot_powder_meas_vs_calc( ) fig.update_xaxes(showticklabels=(layout.row_count == 1), row=1, col=1) - fig.update_yaxes(title_text=plot_spec.axes_labels[1], row=1, col=1) + fig.update_yaxes( + title_text=plot_spec.axes_labels[1], + range=[main_y_min, main_y_max], + row=1, + col=1, + ) if layout.bragg_row is not None: fig.update_yaxes( @@ -949,6 +970,8 @@ def plot_powder_meas_vs_calc( range=[-residual_limit, residual_limit], tickmode='array', tickvals=[-residual_tick_limit, 0.0, residual_tick_limit], + scaleanchor='y', + scaleratio=1, zeroline=False, row=layout.residual_row, col=1, diff --git a/tests/unit/easydiffraction/display/plotters/test_plotly.py b/tests/unit/easydiffraction/display/plotters/test_plotly.py index 6a206654a..17109a4c2 100644 --- a/tests/unit/easydiffraction/display/plotters/test_plotly.py +++ b/tests/unit/easydiffraction/display/plotters/test_plotly.py @@ -296,6 +296,8 @@ def fake_show_figure(self, fig): assert fig.layout.xaxis.matches == 'x' assert fig.layout.xaxis2.matches == 'x' assert fig.layout.xaxis3.matches == 'x' + assert fig.layout.yaxis3.scaleanchor == 'y' + assert fig.layout.yaxis3.scaleratio == pytest.approx(1.0) plot_area_height = fig.layout.height - fig.layout.margin.t - fig.layout.margin.b main_height = fig.layout.yaxis.domain[1] - fig.layout.yaxis.domain[0] @@ -555,9 +557,19 @@ def fake_show_figure(self, fig): ) fig = captured['fig'] - expected_limit = 0.5 * (3600.0 - 180.0) * 0.25 + expected_limit = 0.5 * (3600.0 - 0.0) * 0.25 + assert fig.layout.yaxis2.scaleanchor == 'y' + assert fig.layout.yaxis2.scaleratio == pytest.approx(1.0) + assert fig.layout.yaxis.range[0] == pytest.approx(0.0) + assert fig.layout.yaxis.range[1] == pytest.approx(3600.0) assert fig.layout.yaxis2.range[0] == pytest.approx(-expected_limit) assert fig.layout.yaxis2.range[1] == pytest.approx(expected_limit) + plot_area_height = fig.layout.height - fig.layout.margin.t - fig.layout.margin.b + main_pixels = plot_area_height * (fig.layout.yaxis.domain[1] - fig.layout.yaxis.domain[0]) + residual_pixels = plot_area_height * (fig.layout.yaxis2.domain[1] - fig.layout.yaxis2.domain[0]) + main_units_per_pixel = (fig.layout.yaxis.range[1] - fig.layout.yaxis.range[0]) / main_pixels + residual_units_per_pixel = (fig.layout.yaxis2.range[1] - fig.layout.yaxis2.range[0]) / residual_pixels + assert residual_units_per_pixel == pytest.approx(main_units_per_pixel) assert list(fig.layout.yaxis2.tickvals) == pytest.approx([-400.0, 0.0, 400.0]) @@ -590,7 +602,7 @@ def fake_show_figure(self, fig): ) fig = captured['fig'] - expected_limit = 0.5 * (3600.0 - 180.0) * 0.25 + expected_limit = 0.5 * (3600.0 - 0.0) * 0.25 assert fig.layout.yaxis2.range[0] == pytest.approx(-expected_limit) assert fig.layout.yaxis2.range[1] == pytest.approx(expected_limit) assert list(fig.layout.yaxis2.tickvals) == pytest.approx([-400.0, 0.0, 400.0]) From 0315448e45676c31bbc6fc306bb6ad1c11996a2a Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 00:42:02 +0200 Subject: [PATCH 04/13] Refactor docstrings and improve code readability in Plotly --- .../display/plotters/plotly.py | 39 ++++++++++++------- .../display/plotters/test_plotly.py | 8 +++- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/easydiffraction/display/plotters/plotly.py b/src/easydiffraction/display/plotters/plotly.py index f96138dd0..4d171210d 100644 --- a/src/easydiffraction/display/plotters/plotly.py +++ b/src/easydiffraction/display/plotters/plotly.py @@ -717,28 +717,32 @@ def _base_composite_height_pixels(plot_spec: PowderMeasVsCalcSpec) -> float: @staticmethod def _composite_plot_area_height(full_height: float) -> float: - """Return the drawable plot area height after vertical margins.""" + """ + Return the drawable plot area height after vertical margins. + """ return max(full_height - COMPOSITE_MARGIN_TOP - COMPOSITE_MARGIN_BOTTOM, 1.0) @staticmethod def _subplot_available_height_fraction(row_count: int) -> float: - """Return the fraction of plot height available for subplot rows.""" + """ + Return the fraction of plot height available for subplot rows. + """ return 1.0 - COMPOSITE_VERTICAL_SPACING * max(row_count - 1, 0) @staticmethod def _bragg_tick_symbol_height_pixels() -> float: """Return rendered pixel height for one Bragg tick marker.""" return ( - BRAGG_TICK_MARKER_SIZE * BRAGG_TICK_SYMBOL_HEIGHT_SCALE - + BRAGG_TICK_MARKER_LINE_WIDTH + BRAGG_TICK_MARKER_SIZE * BRAGG_TICK_SYMBOL_HEIGHT_SCALE + BRAGG_TICK_MARKER_LINE_WIDTH ) @staticmethod def _bragg_row_height_pixels(plot_spec: PowderMeasVsCalcSpec) -> float: - """Return the exact Bragg-row pixel height for the current phases.""" + """ + Return the exact Bragg-row pixel height for the current phases. + """ return float( - len(plot_spec.bragg_tick_sets) - * PlotlyPlotter._bragg_tick_symbol_height_pixels() + len(plot_spec.bragg_tick_sets) * PlotlyPlotter._bragg_tick_symbol_height_pixels() ) @classmethod @@ -753,7 +757,9 @@ def _baseline_non_bragg_row_heights( baseline_height = cls._base_composite_height_pixels(plot_spec) plot_area_height = cls._composite_plot_area_height(baseline_height) available_row_pixels = plot_area_height * cls._subplot_available_height_fraction(row_count) - baseline_bragg_pixels = float(cls._bragg_tick_symbol_height_pixels() if has_bragg_ticks else 0) + baseline_bragg_pixels = float( + cls._bragg_tick_symbol_height_pixels() if has_bragg_ticks else 0 + ) non_bragg_pixels = max(available_row_pixels - baseline_bragg_pixels, 1.0) if not has_residual: @@ -801,21 +807,26 @@ def _composite_figure_height( plot_spec: PowderMeasVsCalcSpec, layout: PowderCompositeRows, ) -> float: - """Return figure height with Bragg growth from a single-phase baseline.""" + """ + Return figure height with Bragg growth from a single-phase + baseline. + """ base_pixels = cls._base_composite_height_pixels(plot_spec) phase_count = len(plot_spec.bragg_tick_sets) if phase_count <= 1: return base_pixels - added_bragg_pixels = float( - (phase_count - 1) * cls._bragg_tick_symbol_height_pixels() + added_bragg_pixels = float((phase_count - 1) * cls._bragg_tick_symbol_height_pixels()) + growth_pixels = added_bragg_pixels / cls._subplot_available_height_fraction( + layout.row_count ) - growth_pixels = added_bragg_pixels / cls._subplot_available_height_fraction(layout.row_count) return base_pixels + growth_pixels @classmethod def _get_main_intensity_range(cls, plot_spec: PowderMeasVsCalcSpec) -> tuple[float, float]: - """Return an explicit y-range for the main powder intensity row.""" + """ + Return an explicit y-range for the main powder intensity row. + """ y_meas = np.asarray(plot_spec.y_meas) y_calc = np.asarray(plot_spec.y_calc) if min(y_meas.size, y_calc.size) == 0: @@ -823,7 +834,7 @@ def _get_main_intensity_range(cls, plot_spec: PowderMeasVsCalcSpec) -> tuple[flo main_y_min = float(min(np.min(y_meas), np.min(y_calc))) main_y_max = float(max(np.max(y_meas), np.max(y_calc))) - lower_limit = 0.0 if main_y_min >= 0.0 else main_y_min + lower_limit = min(0.0, main_y_min) if main_y_max <= lower_limit: return lower_limit - 1.0, lower_limit + 1.0 return lower_limit, main_y_max diff --git a/tests/unit/easydiffraction/display/plotters/test_plotly.py b/tests/unit/easydiffraction/display/plotters/test_plotly.py index 17109a4c2..a20ee7288 100644 --- a/tests/unit/easydiffraction/display/plotters/test_plotly.py +++ b/tests/unit/easydiffraction/display/plotters/test_plotly.py @@ -566,9 +566,13 @@ def fake_show_figure(self, fig): assert fig.layout.yaxis2.range[1] == pytest.approx(expected_limit) plot_area_height = fig.layout.height - fig.layout.margin.t - fig.layout.margin.b main_pixels = plot_area_height * (fig.layout.yaxis.domain[1] - fig.layout.yaxis.domain[0]) - residual_pixels = plot_area_height * (fig.layout.yaxis2.domain[1] - fig.layout.yaxis2.domain[0]) + residual_pixels = plot_area_height * ( + fig.layout.yaxis2.domain[1] - fig.layout.yaxis2.domain[0] + ) main_units_per_pixel = (fig.layout.yaxis.range[1] - fig.layout.yaxis.range[0]) / main_pixels - residual_units_per_pixel = (fig.layout.yaxis2.range[1] - fig.layout.yaxis2.range[0]) / residual_pixels + residual_units_per_pixel = ( + fig.layout.yaxis2.range[1] - fig.layout.yaxis2.range[0] + ) / residual_pixels assert residual_units_per_pixel == pytest.approx(main_units_per_pixel) assert list(fig.layout.yaxis2.tickvals) == pytest.approx([-400.0, 0.0, 400.0]) From 270adc8eb3f9b56ee6710f2eaa642db9786c19f7 Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 00:46:54 +0200 Subject: [PATCH 05/13] Fix Plotly plotter check regressions --- .github/copilot-instructions.md | 3 ++- src/easydiffraction/display/plotters/plotly.py | 6 ++---- tests/unit/easydiffraction/display/plotters/test_plotly.py | 7 +++---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 45504df50..da38b9445 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -153,7 +153,8 @@ update those artifacts. - Keep commits atomic, single-purpose, and aligned with plan steps. Use imperative commit messages, no type prefix, and keep the subject line - at or below 72 characters. + at or below 72 characters. Do not add "Co-authored-by: Copilot" to the + commit message. - Before each commit, inspect the worktree and avoid staging unrelated user changes. If unrelated dirty files exist, leave them untouched and mention them only when relevant. diff --git a/src/easydiffraction/display/plotters/plotly.py b/src/easydiffraction/display/plotters/plotly.py index 4d171210d..c7908e907 100644 --- a/src/easydiffraction/display/plotters/plotly.py +++ b/src/easydiffraction/display/plotters/plotly.py @@ -750,6 +750,7 @@ def _baseline_non_bragg_row_heights( cls, plot_spec: PowderMeasVsCalcSpec, row_count: int, + *, has_bragg_ticks: bool, has_residual: bool, ) -> tuple[float, float | None]: @@ -807,10 +808,7 @@ def _composite_figure_height( plot_spec: PowderMeasVsCalcSpec, layout: PowderCompositeRows, ) -> float: - """ - Return figure height with Bragg growth from a single-phase - baseline. - """ + """Return figure height for Bragg row growth.""" base_pixels = cls._base_composite_height_pixels(plot_spec) phase_count = len(plot_spec.bragg_tick_sets) if phase_count <= 1: diff --git a/tests/unit/easydiffraction/display/plotters/test_plotly.py b/tests/unit/easydiffraction/display/plotters/test_plotly.py index a20ee7288..96859c374 100644 --- a/tests/unit/easydiffraction/display/plotters/test_plotly.py +++ b/tests/unit/easydiffraction/display/plotters/test_plotly.py @@ -237,10 +237,9 @@ def test_get_bragg_tick_trace_includes_peak_metadata(): assert trace.mode == 'markers' assert trace.marker.symbol == 'line-ns-open' assert trace.hovertemplate == '%{text}' - assert 'phase_id: phase-a' in trace.text[0] - assert 'hkl: (1 0 1)' in trace.text[0] - assert 'f_squared_calc: 100' in trace.text[0] - assert 'f_calc: 10' in trace.text[0] + assert 'Bragg peaks: phase-a' in trace.text[0] + assert 'Miller indices: (1 0 1)' in trace.text[0] + assert 'x: 1.5' in trace.text[0] def test_plot_powder_meas_vs_calc_creates_synced_three_panel_figure(monkeypatch): From 955ef0e69771f6645f02bcedd22fdbb0cd8a88d0 Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 00:49:50 +0200 Subject: [PATCH 06/13] Tighten and restructure copilot instructions --- .github/copilot-instructions.md | 238 ++++++++++++++++---------------- 1 file changed, 116 insertions(+), 122 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index da38b9445..d8d521bfc 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -4,18 +4,18 @@ - Python library for crystallographic diffraction analysis (refining structural models against experimental data). -- Axes: `sample_form` (powder, single crystal), `beam_mode` (time-of- - flight, constant wavelength), `radiation_probe` (neutron, x-ray), - `scattering_type` (bragg, total). -- Calculation backends: `cryspy` and `crysfml` (Bragg), `pdffit2` (total - scattering). -- Follow CIF naming conventions; deviate only for clearly better API. -- CIF concepts map to: `DatablockItem` / `DatablockCollection` and - `CategoryItem` / `CategoryCollection` (loops). +- Domain axes: `sample_form` (powder, single crystal), `beam_mode` + (time-of-flight, constant wavelength), `radiation_probe` (neutron, + x-ray), `scattering_type` (bragg, total). +- Calculation backends: `cryspy` and `crysfml` (Bragg), `pdffit2` + (total scattering). +- CIF maps to `DatablockItem`/`DatablockCollection` and + `CategoryItem`/`CategoryCollection` (loops). Follow CIF naming; + deviate only for a clearly better API. - Metadata via frozen dataclasses: `TypeInfo`, `Compatibility`, `CalculatorSupport`. -- Audience: scientists, often non-programmers. Prioritize - discoverability, clear errors, safe defaults over developer +- Audience is scientists, often non-programmers: prioritize + discoverability, clear errors, and safe defaults over developer ergonomics. - Critical-software rigor: every code path tested, edge cases handled explicitly, no silent failures. @@ -24,62 +24,55 @@ - snake_case (functions/vars), PascalCase (classes), UPPER_SNAKE_CASE (constants). -- `from __future__ import annotations` in every module. -- Type-annotate all public signatures. -- Numpy-style docstrings on all public classes/methods, with Parameters - / Returns / Raises where applicable. Summary is one line ≤72 chars +- `from __future__ import annotations` in every module. Type-annotate + all public signatures. +- Numpy-style docstrings on all public classes/methods (Parameters / + Returns / Raises where applicable). Summary is one line ≤72 chars (`max-doc-length`); shorten wording rather than wrap. -- Flat over nested, explicit over clever. No defensive checks for - unlikely edge cases. Composition over deep inheritance. +- Flat over nested, explicit over clever, composition over deep + inheritance. No defensive checks for unlikely edge cases. - One class per file when substantial; group small related classes. - No `**kwargs` — use explicit keyword arguments. - No string-based dispatch (e.g. `getattr(self, f'_{name}')`); write - explicit named methods (`_set_sample_form`, `_set_beam_mode`). -- Public attrs are either editable (getter+setter property) or read-only - (getter only). For internal mutation of read-only props, add a private - `_set_` method, not a public setter. + named methods (`_set_sample_form`, `_set_beam_mode`). +- Public attrs are either editable (getter+setter property) or + read-only (getter only). For internal mutation of read-only props, + use a private `_set_` method, not a public setter. - Lint complexity thresholds in `pyproject.toml` (`max-args`, `max-branches`, `max-statements`, `max-locals`, `max-nested-blocks`, …) are guardrails. A violation means refactor (extract helpers, - parameter objects, flatten). Do not raise thresholds, add `# noqa`, or - otherwise silence them. For complex refactors touching many lines or - public API, propose a plan and wait for approval. + parameter objects, flatten) — do not raise thresholds, add `# noqa`, + or otherwise silence them. For complex refactors touching many lines + or public API, propose a plan and wait for approval. ## Architecture - Eager top-of-module imports by default. Lazy imports only to break - circular deps or keep `core/` free of heavy imports on rarely-called - paths (e.g. `help()`). -- No `pkgutil` / `importlib` auto-discovery, no background threads, no + circular deps or to keep `core/` free of heavy imports on rarely- + called paths (e.g. `help()`). +- No `pkgutil`/`importlib` auto-discovery, no background threads, no monkey-patching or runtime class mutation. - No `__all__`; control public API via explicit `__init__.py` imports. -- No redundant `import X as X` aliases — use plain - `from module import X`. -- Concrete classes use `@Factory.register`. Each package's `__init__.py` - must explicitly import every concrete class to trigger registration. - Always add new concrete classes to the corresponding `__init__.py`. -- Switchable categories (factory-swappable at runtime) follow this fixed - API on the owner (experiment / structure / analysis): `` - (read-only), `_type` (getter+setter), - `show_supported__types()`, `show_current__type()`. - The owner owns the type setter and show methods; show methods delegate - to `Factory.show_supported(...)`. Required even if only one - implementation exists. -- Categories are flat siblings within their owner. Never nest a category - as a child of another category of a different type; cross-reference - via IDs instead. + No redundant `import X as X` aliases. +- Concrete classes use `@Factory.register`. Each package's + `__init__.py` must explicitly import every concrete class to trigger + registration — always update it when adding a class. +- Switchable categories (factory-swappable at runtime) follow this + fixed API on the owner (experiment / structure / analysis): + `` (read-only), `_type` (getter+setter), + `show_supported__types()`, + `show_current__type()`. The owner owns the type setter and + show methods; show methods delegate to `Factory.show_supported(...)`. + Required even if only one implementation exists. +- Categories are flat siblings within their owner. Never nest a + category as a child of another category of a different type; + cross-reference via IDs instead. - Every finite, closed set of values (factory tags, axes, enumerated - descriptors) uses a `(str, Enum)` class; compare against members, not - raw strings. + descriptors) is a `(str, Enum)`; compare against members, not raw + strings. - Keep `core/` free of domain logic (base classes and utilities only). - Don't introduce abstractions before a concrete second use case. -- Don't add dependencies without asking. - -## Tutorials - -- Notebooks in `docs/docs/tutorials/*.ipynb` are generated artifacts. - Edit only the corresponding `*.py`, then run - `pixi run notebook-prepare`. + Don't add dependencies without asking. ## Testing @@ -88,10 +81,9 @@ - Unit tests mirror the source tree: `src/easydiffraction//.py` → `tests/unit/easydiffraction//test_.py`. Verify with - `pixi run test-structure-check`. -- Category packages with only `default.py`/`factory.py` may use one - parent-level `test_.py`. -- Supplementary tests: `test__coverage.py`. + `pixi run test-structure-check`. Supplementary tests: + `test__coverage.py`. Category packages with only + `default.py`/`factory.py` may use one parent-level `test_.py`. - Tests expecting `log.error()` to raise must `monkeypatch` Logger to RAISE mode (another test may have leaked WARN mode). - `@typechecked` setters raise `typeguard.TypeCheckError`, not @@ -99,88 +91,90 @@ - No test-ordering dependence, no network, no sleeping, no real calculation engines in unit tests. -## Changes +## Tutorials + +- Notebooks in `docs/docs/tutorials/*.ipynb` are generated artifacts. + Edit only the corresponding `*.py`, then run + `pixi run notebook-prepare`. + +## Change Discipline - Before any structural/design change (new categories, factories, switchable-category wiring, datablocks, CIF serialisation), read `docs/architecture/architecture.md` and follow documented patterns. Localised bug fixes or test updates need only this file. -- Project is in beta: no legacy shims, no deprecation warnings — update - tests and tutorials to current API. -- Minimal diffs; don't reformat working code. +- Project is in beta: no legacy shims, no deprecation warnings — + update tests and tutorials to the current API. +- Minimal diffs; don't reformat working code. Fix only what's asked; + flag adjacent issues as comments. Don't add features or refactor + unless asked. Don't remove TODOs or comments unless the change fully + resolves them. - Never remove or replace existing functionality without explicit - confirmation. Highlight every removal and wait for approval. -- Fix only what's asked; flag adjacent issues as comments. -- Don't add features or refactor unless asked. Don't remove TODOs or - comments unless the change fully resolves them. -- When renaming, grep the entire project (code, tests, tutorials, docs). -- Each change is atomic, single-commit-sized. Make one change, suggest - the commit message, then stop and wait for confirmation. + confirmation — highlight every removal and wait for approval. +- When renaming, grep the entire project (code, tests, tutorials, + docs). +- Each change is atomic and single-commit-sized: make one change, + suggest the commit message, then stop and wait for confirmation. - When in doubt, ask. +## Commits + +- Suggest a commit message after each change: code block, ≤72 chars, + imperative mood, no type prefix, no `Co-authored-by: Copilot`. + Examples: + - Add ChebyshevPolynomialBackground class + - Implement background_type setter on Experiment + - Standardize switchable-category naming convention +- Stage only the files modified for the step, using explicit paths + where practical. Do not include data, project, CIF, or other + generated artifacts produced by integration/script/notebook tests + unless the user explicitly asked to update them. +- Before each commit, inspect the worktree and avoid staging unrelated + user changes. If unrelated dirty files exist, leave them untouched + and mention them only when relevant. + ## Workflow -### Planning Workflow +Non-trivial changes use a two-phase workflow: -- When asked to create a plan, first gather enough repository context to - make the plan concrete. Ask all ambiguous, potentially ambiguous, or - unclear questions in one concise batch, and record unresolved - questions in the plan if the user wants the plan saved before - answering them. -- Save plans as Markdown files in `docs/dev` with the filename pattern - `plan_.md`. The `` part uses lowercase - words separated by dashes, for example - `docs/dev/plan_background-refactor.md`. -- Use the same `` to create the implementation branch, - normally `feature/`. Do not push the branch unless the - user explicitly asks. -- Each plan must include a status checklist with `[ ]` items. Mark each - item as `[x]` as it is completed during implementation. -- Plans for non-trivial work must separate the work into two phases: - - **Phase 1 — Implementation:** the agent works independently through - all implementation steps, updating the plan checklist as it goes. Do - not create tests or run tests in this phase unless the user - explicitly asks. When Phase 1 is complete, stop and ask the user to - review the implementation. - - **Phase 2 — Verification:** after user approval, add/update tests, - run formatting, linting, unit tests, integration tests, and script - or notebook checks requested by the plan. -- Every completed implementation step must end with a local commit. - Stage only the files modified for that step, using explicit paths - where practical. Do not include data files, project files, CIF files, - or other generated artifacts created by integration tests, script - tests, or notebook execution unless the user explicitly asked to - update those artifacts. -- Keep commits atomic, single-purpose, and aligned with plan steps. Use - imperative commit messages, no type prefix, and keep the subject line - at or below 72 characters. Do not add "Co-authored-by: Copilot" to the - commit message. -- Before each commit, inspect the worktree and avoid staging unrelated - user changes. If unrelated dirty files exist, leave them untouched and - mention them only when relevant. -- The plan should be easy to maintain while working: include concrete - files likely to change, decisions already made, open questions, - verification commands for Phase 2, and a short suggested commit - message or branch name when useful. +- **Phase 1 — Implementation.** Code, docs, and architecture updates + only. Do not create or run tests unless the user explicitly asks. + When done, present for review and iterate until approved. +- **Phase 2 — Verification.** Add/update tests, then run + `pixi run fix`, `pixi run check`, `pixi run unit-tests`, + `pixi run integration-tests`, `pixi run script-tests`. -### General Workflow +Notes: -- Two-phase workflow for non-trivial changes: - - **Phase 1 — Implementation:** code, docs, architecture updates. Do - not create new tests or run existing tests. Present for review and - iterate until approved. - - **Phase 2 — Verification:** add/update tests, then run - `pixi run fix`, `pixi run check`, `pixi run unit-tests`, - `pixi run integration-tests`, `pixi run script-tests`. +- `pixi run fix` regenerates `docs/architecture/package-structure-*.md` + automatically — never edit those by hand. Don't review auto-fixes; + accept and move on. Then `pixi run check` until clean. - Open issues / design questions / planned improvements live in `docs/architecture/issues_open.md` (priority-ordered). On resolution, move to `docs/architecture/issues_closed.md` and update `architecture.md` if affected. -- `pixi run fix` regenerates `docs/architecture/package-structure-*.md` - automatically — never edit those by hand. Don't review auto-fixes; - accept and move on. Then `pixi run check` until clean. -- Suggest a commit message (code block, ≤72 chars, imperative mood, no - type prefix) after each change. E.g.: - - Add ChebyshevPolynomialBackground class - - Implement background_type setter on Experiment - - Standardize switchable-category naming convention + +### Planning + +When asked to create a plan: + +- First gather enough repository context to make the plan concrete. + Ask all ambiguous or unclear questions in one concise batch; record + unresolved questions in the plan if the user wants it saved before + answering them. +- Save plans as `docs/dev/plan_.md` (lowercase, + dash-separated, e.g. `plan_background-refactor.md`). Use the same + `` for the implementation branch + (`feature/`). Do not push the branch unless asked. +- Include a status checklist with `[ ]` items; mark `[x]` as completed + during implementation. +- Apply the two-phase workflow (Phase 1 implementation, Phase 2 + verification) to non-trivial plans. Stop after Phase 1 and ask the + user to review before starting Phase 2. +- Every completed implementation step ends with a local commit + following the rules in **Commits**. Keep commits atomic, + single-purpose, and aligned with plan steps. +- The plan should be easy to maintain while working: include concrete + files likely to change, decisions already made, open questions, + verification commands for Phase 2, and a short suggested commit + message or branch name when useful. From ef1850c9dcfb927d01bff16bec344d7e254ef676 Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 01:16:45 +0200 Subject: [PATCH 07/13] Remove layout fraction overrides from plot_meas_vs_calc --- src/easydiffraction/display/plotting.py | 16 ++-------------- .../easydiffraction/display/test_plotting.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/easydiffraction/display/plotting.py b/src/easydiffraction/display/plotting.py index 6ef6ef6c2..782b0e435 100644 --- a/src/easydiffraction/display/plotting.py +++ b/src/easydiffraction/display/plotting.py @@ -78,8 +78,6 @@ class _MeasVsCalcPlotOptions: x_min: float | None = None x_max: float | None = None show_residual: bool | None = None - residual_height_fraction: float = DEFAULT_RESID_HEIGHT - bragg_peaks_height_fraction: float = DEFAULT_BRAGG_ROW x: object | None = None @@ -467,8 +465,6 @@ def plot_meas_vs_calc( x_max: float | None = None, *, show_residual: bool | None = None, - residual_height_fraction: float = DEFAULT_RESID_HEIGHT, - bragg_peaks_height_fraction: float = DEFAULT_BRAGG_ROW, x: object | None = None, ) -> None: """ @@ -486,12 +482,6 @@ def plot_meas_vs_calc( When ``None``, powder Bragg plots include the residual by default while other measured-vs-calculated plots keep the historical no-residual default. - residual_height_fraction : float, default=DEFAULT_RESID_HEIGHT - Optional. Defaults to 0.25. Residual-row height relative to - the main intensity row. - bragg_peaks_height_fraction : float, default=DEFAULT_BRAGG_ROW - Optional. Defaults to 0.15. Bragg-tick-row height relative - to the main intensity row. x : object | None, default=None Optional explicit x-axis data to override stored values. """ @@ -501,8 +491,6 @@ def plot_meas_vs_calc( x_min=x_min, x_max=x_max, show_residual=show_residual, - residual_height_fraction=residual_height_fraction, - bragg_peaks_height_fraction=bragg_peaks_height_fraction, x=x, ) self._plot_meas_vs_calc_data( @@ -1284,8 +1272,8 @@ def _plot_powder_bragg_meas_vs_calc( bragg_tick_sets=bragg_tick_sets, axes_labels=ctx['axes_labels'], title=title, - residual_height_fraction=plot_options.residual_height_fraction, - bragg_peaks_height_fraction=plot_options.bragg_peaks_height_fraction, + residual_height_fraction=DEFAULT_RESID_HEIGHT, + bragg_peaks_height_fraction=DEFAULT_BRAGG_ROW, height=self._composite_plot_height(), ) self._backend.plot_powder_meas_vs_calc(plot_spec=plot_spec) diff --git a/tests/unit/easydiffraction/display/test_plotting.py b/tests/unit/easydiffraction/display/test_plotting.py index e1cde85ab..656976f48 100644 --- a/tests/unit/easydiffraction/display/test_plotting.py +++ b/tests/unit/easydiffraction/display/test_plotting.py @@ -492,6 +492,18 @@ class Experiment: assert call.bragg_tick_sets == () +def test_plot_meas_vs_calc_does_not_accept_layout_fraction_overrides(): + from easydiffraction.display.plotting import Plotter + + plotter = Plotter() + + with pytest.raises(TypeError, match='residual_height_fraction'): + plotter.plot_meas_vs_calc('E1', residual_height_fraction=0.20) + + with pytest.raises(TypeError, match='bragg_peaks_height_fraction'): + plotter.plot_meas_vs_calc('E1', bragg_peaks_height_fraction=0.20) + + def test_plot_meas_vs_calc_keeps_single_crystal_routing(): import numpy as np From 0c957fa0c03d8f8306eeb26f180b3f8b9cfcf1f1 Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 01:16:53 +0200 Subject: [PATCH 08/13] Apply formatting --- .github/copilot-instructions.md | 82 ++++++++++++++++----------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d8d521bfc..93b84d2eb 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -7,8 +7,8 @@ - Domain axes: `sample_form` (powder, single crystal), `beam_mode` (time-of-flight, constant wavelength), `radiation_probe` (neutron, x-ray), `scattering_type` (bragg, total). -- Calculation backends: `cryspy` and `crysfml` (Bragg), `pdffit2` - (total scattering). +- Calculation backends: `cryspy` and `crysfml` (Bragg), `pdffit2` (total + scattering). - CIF maps to `DatablockItem`/`DatablockCollection` and `CategoryItem`/`CategoryCollection` (loops). Follow CIF naming; deviate only for a clearly better API. @@ -35,9 +35,9 @@ - No `**kwargs` — use explicit keyword arguments. - No string-based dispatch (e.g. `getattr(self, f'_{name}')`); write named methods (`_set_sample_form`, `_set_beam_mode`). -- Public attrs are either editable (getter+setter property) or - read-only (getter only). For internal mutation of read-only props, - use a private `_set_` method, not a public setter. +- Public attrs are either editable (getter+setter property) or read-only + (getter only). For internal mutation of read-only props, use a private + `_set_` method, not a public setter. - Lint complexity thresholds in `pyproject.toml` (`max-args`, `max-branches`, `max-statements`, `max-locals`, `max-nested-blocks`, …) are guardrails. A violation means refactor (extract helpers, @@ -54,25 +54,25 @@ monkey-patching or runtime class mutation. - No `__all__`; control public API via explicit `__init__.py` imports. No redundant `import X as X` aliases. -- Concrete classes use `@Factory.register`. Each package's - `__init__.py` must explicitly import every concrete class to trigger - registration — always update it when adding a class. -- Switchable categories (factory-swappable at runtime) follow this - fixed API on the owner (experiment / structure / analysis): - `` (read-only), `_type` (getter+setter), - `show_supported__types()`, - `show_current__type()`. The owner owns the type setter and - show methods; show methods delegate to `Factory.show_supported(...)`. - Required even if only one implementation exists. -- Categories are flat siblings within their owner. Never nest a - category as a child of another category of a different type; - cross-reference via IDs instead. +- Concrete classes use `@Factory.register`. Each package's `__init__.py` + must explicitly import every concrete class to trigger registration — + always update it when adding a class. +- Switchable categories (factory-swappable at runtime) follow this fixed + API on the owner (experiment / structure / analysis): `` + (read-only), `_type` (getter+setter), + `show_supported__types()`, `show_current__type()`. + The owner owns the type setter and show methods; show methods delegate + to `Factory.show_supported(...)`. Required even if only one + implementation exists. +- Categories are flat siblings within their owner. Never nest a category + as a child of another category of a different type; cross-reference + via IDs instead. - Every finite, closed set of values (factory tags, axes, enumerated descriptors) is a `(str, Enum)`; compare against members, not raw strings. - Keep `core/` free of domain logic (base classes and utilities only). -- Don't introduce abstractions before a concrete second use case. - Don't add dependencies without asking. +- Don't introduce abstractions before a concrete second use case. Don't + add dependencies without asking. ## Testing @@ -83,7 +83,8 @@ `tests/unit/easydiffraction//test_.py`. Verify with `pixi run test-structure-check`. Supplementary tests: `test__coverage.py`. Category packages with only - `default.py`/`factory.py` may use one parent-level `test_.py`. + `default.py`/`factory.py` may use one parent-level + `test_.py`. - Tests expecting `log.error()` to raise must `monkeypatch` Logger to RAISE mode (another test may have leaked WARN mode). - `@typechecked` setters raise `typeguard.TypeCheckError`, not @@ -103,16 +104,15 @@ switchable-category wiring, datablocks, CIF serialisation), read `docs/architecture/architecture.md` and follow documented patterns. Localised bug fixes or test updates need only this file. -- Project is in beta: no legacy shims, no deprecation warnings — - update tests and tutorials to the current API. +- Project is in beta: no legacy shims, no deprecation warnings — update + tests and tutorials to the current API. - Minimal diffs; don't reformat working code. Fix only what's asked; flag adjacent issues as comments. Don't add features or refactor unless asked. Don't remove TODOs or comments unless the change fully resolves them. - Never remove or replace existing functionality without explicit confirmation — highlight every removal and wait for approval. -- When renaming, grep the entire project (code, tests, tutorials, - docs). +- When renaming, grep the entire project (code, tests, tutorials, docs). - Each change is atomic and single-commit-sized: make one change, suggest the commit message, then stop and wait for confirmation. - When in doubt, ask. @@ -125,24 +125,24 @@ - Add ChebyshevPolynomialBackground class - Implement background_type setter on Experiment - Standardize switchable-category naming convention -- Stage only the files modified for the step, using explicit paths - where practical. Do not include data, project, CIF, or other - generated artifacts produced by integration/script/notebook tests - unless the user explicitly asked to update them. +- Stage only the files modified for the step, using explicit paths where + practical. Do not include data, project, CIF, or other generated + artifacts produced by integration/script/notebook tests unless the + user explicitly asked to update them. - Before each commit, inspect the worktree and avoid staging unrelated - user changes. If unrelated dirty files exist, leave them untouched - and mention them only when relevant. + user changes. If unrelated dirty files exist, leave them untouched and + mention them only when relevant. ## Workflow Non-trivial changes use a two-phase workflow: - **Phase 1 — Implementation.** Code, docs, and architecture updates - only. Do not create or run tests unless the user explicitly asks. - When done, present for review and iterate until approved. -- **Phase 2 — Verification.** Add/update tests, then run - `pixi run fix`, `pixi run check`, `pixi run unit-tests`, - `pixi run integration-tests`, `pixi run script-tests`. + only. Do not create or run tests unless the user explicitly asks. When + done, present for review and iterate until approved. +- **Phase 2 — Verification.** Add/update tests, then run `pixi run fix`, + `pixi run check`, `pixi run unit-tests`, `pixi run integration-tests`, + `pixi run script-tests`. Notes: @@ -158,8 +158,8 @@ Notes: When asked to create a plan: -- First gather enough repository context to make the plan concrete. - Ask all ambiguous or unclear questions in one concise batch; record +- First gather enough repository context to make the plan concrete. Ask + all ambiguous or unclear questions in one concise batch; record unresolved questions in the plan if the user wants it saved before answering them. - Save plans as `docs/dev/plan_.md` (lowercase, @@ -171,9 +171,9 @@ When asked to create a plan: - Apply the two-phase workflow (Phase 1 implementation, Phase 2 verification) to non-trivial plans. Stop after Phase 1 and ask the user to review before starting Phase 2. -- Every completed implementation step ends with a local commit - following the rules in **Commits**. Keep commits atomic, - single-purpose, and aligned with plan steps. +- Every completed implementation step ends with a local commit following + the rules in **Commits**. Keep commits atomic, single-purpose, and + aligned with plan steps. - The plan should be easy to maintain while working: include concrete files likely to change, decisions already made, open questions, verification commands for Phase 2, and a short suggested commit From f0e9b8b6d5abda88db9c79b3379c064553181092 Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 08:25:58 +0200 Subject: [PATCH 09/13] Use shared hover labels for composite powder traces --- .../display/plotters/plotly.py | 88 +++++++++++++++++-- .../display/plotters/test_plotly.py | 17 ++++ 2 files changed, 96 insertions(+), 9 deletions(-) diff --git a/src/easydiffraction/display/plotters/plotly.py b/src/easydiffraction/display/plotters/plotly.py index c7908e907..3661fd55e 100644 --- a/src/easydiffraction/display/plotters/plotly.py +++ b/src/easydiffraction/display/plotters/plotly.py @@ -361,6 +361,9 @@ def _get_powder_trace( x: object, y: object, label: str, + *, + customdata: object | None = None, + hovertemplate: str | None = None, ) -> object: """ Create a Plotly trace for powder diffraction data. @@ -373,6 +376,11 @@ def _get_powder_trace( 1D array- like of y-axis values. label : str Series identifier (``'meas'``, ``'calc'``, or ``'resid'``). + customdata : object | None, default=None + Optional per-point payload used by the hover template. + hovertemplate : str | None, default=None + Optional hover template overriding the default per-trace + one. Returns ------- @@ -390,7 +398,39 @@ def _get_powder_trace( line=line, mode=mode, name=name, - hovertemplate=f'{name}
x: %{{x}}
y: %{{y}}', + customdata=customdata, + hovertemplate=( + hovertemplate + if hovertemplate is not None + else f'{name}
x: %{{x}}
y: %{{y}}' + ), + ) + + @staticmethod + def _powder_meas_vs_calc_hover_data(plot_spec: PowderMeasVsCalcSpec) -> np.ndarray: + """Return shared hover values for composite powder traces.""" + residual_values = ( + np.asarray(plot_spec.y_resid) + if plot_spec.y_resid is not None + else np.asarray(plot_spec.y_meas) - np.asarray(plot_spec.y_calc) + ) + return np.column_stack(( + np.asarray(plot_spec.y_meas), + np.asarray(plot_spec.y_calc), + residual_values, + )) + + @staticmethod + def _powder_meas_vs_calc_hover_template() -> str: + """ + Return a shared hover template for composite powder traces. + """ + return ( + 'x: %{x:,.2f}
' + 'Imeas: %{customdata[0]:,.2f}
' + 'Icalc: %{customdata[1]:,.2f}
' + 'Imeas - Icalc: %{customdata[2]:,.2f}' + '' ) @staticmethod @@ -649,11 +689,13 @@ def _get_bragg_tick_trace( y = np.full(tick_set.x.shape, row_y, dtype=float) hover_text = [] for idx, x_value in enumerate(tick_set.x): + index_h = int(tick_set.h[idx]) + index_k = int(tick_set.k[idx]) + index_l = int(tick_set.ell[idx]) hover_text.append( - f'Bragg peaks: {tick_set.phase_id}
' - f'Miller indices: ({int(tick_set.h[idx])} {int(tick_set.k[idx])} ' - f'{int(tick_set.ell[idx])})
' - f'x: {float(x_value):.6g}
' + f'{tick_set.phase_id}
' + f'x: {float(x_value):,.2f}
' + f'Miller indices: ({index_h} {index_k} {index_l})
' # f'F²cal:{float(tick_set.f_squared_calc[idx]):.6g}
' # f'Fcalc:{float(tick_set.f_calc[idx]):.6g}' '' @@ -875,6 +917,8 @@ def plot_powder_meas_vs_calc( x_max = float(np.max(x_values)) if has_x_values else None main_y_min, main_y_max = self._get_main_intensity_range(plot_spec) residual_limit = None + hover_data = self._powder_meas_vs_calc_hover_data(plot_spec) + hover_template = self._powder_meas_vs_calc_hover_template() fig = make_subplots( rows=layout.row_count, @@ -884,8 +928,28 @@ def plot_powder_meas_vs_calc( row_heights=layout.row_heights, ) - fig.add_trace(self._get_powder_trace(plot_spec.x, plot_spec.y_meas, 'meas'), row=1, col=1) - fig.add_trace(self._get_powder_trace(plot_spec.x, plot_spec.y_calc, 'calc'), row=1, col=1) + fig.add_trace( + self._get_powder_trace( + plot_spec.x, + plot_spec.y_meas, + 'meas', + customdata=hover_data, + hovertemplate=hover_template, + ), + row=1, + col=1, + ) + fig.add_trace( + self._get_powder_trace( + plot_spec.x, + plot_spec.y_calc, + 'calc', + customdata=hover_data, + hovertemplate=hover_template, + ), + row=1, + col=1, + ) if layout.bragg_row is not None: for idx, tick_set in enumerate(plot_spec.bragg_tick_sets): @@ -903,7 +967,13 @@ def plot_powder_meas_vs_calc( if layout.residual_row is not None and plot_spec.y_resid is not None: residual_limit = self._get_residual_limit(plot_spec) fig.add_trace( - self._get_powder_trace(plot_spec.x, plot_spec.y_resid, 'resid'), + self._get_powder_trace( + plot_spec.x, + plot_spec.y_resid, + 'resid', + customdata=hover_data, + hovertemplate=hover_template, + ), row=layout.residual_row, col=1, ) @@ -1074,7 +1144,7 @@ def plot_scatter( 'array': sy, 'visible': True, }, - hovertemplate='x: %{x}
y: %{y}
', + hovertemplate='x: %{x:,.2f}
y: %{y:,.2f}
', ) layout = self._get_layout( diff --git a/tests/unit/easydiffraction/display/plotters/test_plotly.py b/tests/unit/easydiffraction/display/plotters/test_plotly.py index 96859c374..339b6bf90 100644 --- a/tests/unit/easydiffraction/display/plotters/test_plotly.py +++ b/tests/unit/easydiffraction/display/plotters/test_plotly.py @@ -307,6 +307,23 @@ def fake_show_figure(self, fig): 2 * pp.PlotlyPlotter._bragg_tick_symbol_height_pixels() ) + expected_hovertemplate = ( + 'x: %{x}
' + 'Imeas: %{customdata[0]}
' + 'Icalc: %{customdata[1]}
' + 'Imeas - Icalc: %{customdata[2]}' + '' + ) + meas_trace = next(trace for trace in fig.data if trace.name == 'Measured (Imeas)') + calc_trace = next(trace for trace in fig.data if trace.name == 'Total calculated (Icalc)') + residual_trace = next(trace for trace in fig.data if trace.name == 'Residual (Imeas - Icalc)') + assert meas_trace.hovertemplate == expected_hovertemplate + assert calc_trace.hovertemplate == expected_hovertemplate + assert residual_trace.hovertemplate == expected_hovertemplate + assert list(meas_trace.customdata[0]) == pytest.approx([10.0, 9.0, 1.0]) + assert list(calc_trace.customdata[0]) == pytest.approx([10.0, 9.0, 1.0]) + assert list(residual_trace.customdata[0]) == pytest.approx([10.0, 9.0, 1.0]) + bragg_traces = [trace for trace in fig.data if trace.name.startswith('Bragg')] assert [trace.name for trace in bragg_traces] == [ 'Bragg peaks: phase-a', From 080bd5b20ce5a0b421f1dae5328f9b83dc6f61c2 Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 08:37:50 +0200 Subject: [PATCH 10/13] Add powder background curve to composite plot --- src/easydiffraction/display/plotters/base.py | 11 ++- .../display/plotters/plotly.py | 81 +++++++++++++------ src/easydiffraction/display/plotting.py | 9 +++ .../display/plotters/test_plotly.py | 70 ++++++++++++++-- .../easydiffraction/display/test_plotting.py | 2 + 5 files changed, 137 insertions(+), 36 deletions(-) diff --git a/src/easydiffraction/display/plotters/base.py b/src/easydiffraction/display/plotters/base.py index ec5053951..3c7d25693 100644 --- a/src/easydiffraction/display/plotters/base.py +++ b/src/easydiffraction/display/plotters/base.py @@ -44,9 +44,9 @@ class PowderMeasVsCalcSpec: """ Specification for one composite powder plot. - The plotting facade assembles the measured, calculated, residual, - and Bragg-tick data into this display-specific object before - delegating to a backend. + The plotting facade assembles the measured, background, + calculated, residual, and Bragg-tick data into this + display-specific object before delegating to a backend. """ x: np.ndarray @@ -59,6 +59,7 @@ class PowderMeasVsCalcSpec: residual_height_fraction: float bragg_peaks_height_fraction: float height: int | None = None + y_bkg: np.ndarray | None = None class XAxisType(StrEnum): @@ -184,6 +185,10 @@ class XAxisType(StrEnum): 'mode': 'lines', 'name': 'Total calculated (Icalc)', }, + 'bkg': { + 'mode': 'lines', + 'name': 'Background (Ibkg)', + }, 'meas': { 'mode': 'lines+markers', 'name': 'Measured (Imeas)', diff --git a/src/easydiffraction/display/plotters/plotly.py b/src/easydiffraction/display/plotters/plotly.py index 3661fd55e..874b3c4db 100644 --- a/src/easydiffraction/display/plotters/plotly.py +++ b/src/easydiffraction/display/plotters/plotly.py @@ -36,6 +36,7 @@ DEFAULT_COLORS = { 'meas': 'rgb(31, 119, 180)', + 'bkg': 'rgb(140, 140, 140)', 'calc': 'rgb(214, 39, 40)', 'resid': 'rgb(44, 160, 44)', } @@ -375,7 +376,8 @@ def _get_powder_trace( y : object 1D array- like of y-axis values. label : str - Series identifier (``'meas'``, ``'calc'``, or ``'resid'``). + Series identifier (``'meas'``, ``'bkg'``, ``'calc'``, or + ``'resid'``). customdata : object | None, default=None Optional per-point payload used by the hover template. hovertemplate : str | None, default=None @@ -391,6 +393,12 @@ def _get_powder_trace( name = SERIES_CONFIG[label]['name'] color = DEFAULT_COLORS[label] line = {'color': color} + legend_rank = { + 'meas': 10, + 'bkg': 20, + 'calc': 30, + 'resid': 40, + }[label] return go.Scatter( x=x, @@ -398,6 +406,7 @@ def _get_powder_trace( line=line, mode=mode, name=name, + legendrank=legend_rank, customdata=customdata, hovertemplate=( hovertemplate @@ -414,22 +423,40 @@ def _powder_meas_vs_calc_hover_data(plot_spec: PowderMeasVsCalcSpec) -> np.ndarr if plot_spec.y_resid is not None else np.asarray(plot_spec.y_meas) - np.asarray(plot_spec.y_calc) ) + if plot_spec.y_bkg is None: + return np.column_stack(( + np.asarray(plot_spec.y_meas), + np.asarray(plot_spec.y_calc), + residual_values, + )) + return np.column_stack(( np.asarray(plot_spec.y_meas), + np.asarray(plot_spec.y_bkg), np.asarray(plot_spec.y_calc), residual_values, )) @staticmethod - def _powder_meas_vs_calc_hover_template() -> str: + def _powder_meas_vs_calc_hover_template(plot_spec: PowderMeasVsCalcSpec) -> str: """ Return a shared hover template for composite powder traces. """ + if plot_spec.y_bkg is None: + return ( + 'x: %{x:,.2f}
' + 'Imeas: %{customdata[0]:,.2f}
' + 'Icalc: %{customdata[1]:,.2f}
' + 'Imeas - Icalc: %{customdata[2]:,.2f}' + '' + ) + return ( 'x: %{x:,.2f}
' 'Imeas: %{customdata[0]:,.2f}
' - 'Icalc: %{customdata[1]:,.2f}
' - 'Imeas - Icalc: %{customdata[2]:,.2f}' + 'Ibkg: %{customdata[1]:,.2f}
' + 'Icalc: %{customdata[2]:,.2f}
' + 'Imeas - Icalc: %{customdata[3]:,.2f}' '' ) @@ -918,7 +945,7 @@ def plot_powder_meas_vs_calc( main_y_min, main_y_max = self._get_main_intensity_range(plot_spec) residual_limit = None hover_data = self._powder_meas_vs_calc_hover_data(plot_spec) - hover_template = self._powder_meas_vs_calc_hover_template() + hover_template = self._powder_meas_vs_calc_hover_template(plot_spec) fig = make_subplots( rows=layout.row_count, @@ -928,28 +955,30 @@ def plot_powder_meas_vs_calc( row_heights=layout.row_heights, ) - fig.add_trace( - self._get_powder_trace( - plot_spec.x, - plot_spec.y_meas, - 'meas', - customdata=hover_data, - hovertemplate=hover_template, - ), - row=1, - col=1, - ) - fig.add_trace( - self._get_powder_trace( - plot_spec.x, - plot_spec.y_calc, - 'calc', - customdata=hover_data, - hovertemplate=hover_template, - ), - row=1, - col=1, + main_traces = ( + ( + ('bkg', plot_spec.y_bkg), + ('calc', plot_spec.y_calc), + ('meas', plot_spec.y_meas), + ) + if plot_spec.y_bkg is not None + else ( + ('meas', plot_spec.y_meas), + ('calc', plot_spec.y_calc), + ) ) + for label, y_values in main_traces: + fig.add_trace( + self._get_powder_trace( + plot_spec.x, + y_values, + label, + customdata=hover_data, + hovertemplate=hover_template, + ), + row=1, + col=1, + ) if layout.bragg_row is not None: for idx, tick_set in enumerate(plot_spec.bragg_tick_sets): diff --git a/src/easydiffraction/display/plotting.py b/src/easydiffraction/display/plotting.py index 782b0e435..49efc5d1c 100644 --- a/src/easydiffraction/display/plotting.py +++ b/src/easydiffraction/display/plotting.py @@ -1188,6 +1188,12 @@ def _plot_meas_vs_calc_data( y_calc = self._filtered_y_array( pattern.intensity_calc, ctx['x_array'], ctx['x_min'], ctx['x_max'] ) + y_bkg_raw = getattr(pattern, 'intensity_bkg', None) + y_bkg = ( + self._filtered_y_array(y_bkg_raw, ctx['x_array'], ctx['x_min'], ctx['x_max']) + if y_bkg_raw is not None + else None + ) if sample_form == SampleFormEnum.POWDER and scattering_type == ScatteringTypeEnum.BRAGG: self._plot_powder_bragg_meas_vs_calc( @@ -1195,6 +1201,7 @@ def _plot_meas_vs_calc_data( expt_name=expt_name, ctx=ctx, y_meas=y_meas, + y_bkg=y_bkg, y_calc=y_calc, plot_options=plot_options, title=title, @@ -1245,6 +1252,7 @@ def _plot_powder_bragg_meas_vs_calc( expt_name: str, ctx: dict[str, object], y_meas: np.ndarray, + y_bkg: np.ndarray | None, y_calc: np.ndarray, plot_options: _MeasVsCalcPlotOptions, title: str, @@ -1275,6 +1283,7 @@ def _plot_powder_bragg_meas_vs_calc( residual_height_fraction=DEFAULT_RESID_HEIGHT, bragg_peaks_height_fraction=DEFAULT_BRAGG_ROW, height=self._composite_plot_height(), + y_bkg=y_bkg, ) self._backend.plot_powder_meas_vs_calc(plot_spec=plot_spec) diff --git a/tests/unit/easydiffraction/display/plotters/test_plotly.py b/tests/unit/easydiffraction/display/plotters/test_plotly.py index 339b6bf90..b2fc16884 100644 --- a/tests/unit/easydiffraction/display/plotters/test_plotly.py +++ b/tests/unit/easydiffraction/display/plotters/test_plotly.py @@ -237,9 +237,9 @@ def test_get_bragg_tick_trace_includes_peak_metadata(): assert trace.mode == 'markers' assert trace.marker.symbol == 'line-ns-open' assert trace.hovertemplate == '%{text}' - assert 'Bragg peaks: phase-a' in trace.text[0] + assert 'phase-a' in trace.text[0] assert 'Miller indices: (1 0 1)' in trace.text[0] - assert 'x: 1.5' in trace.text[0] + assert 'x: 1.50' in trace.text[0] def test_plot_powder_meas_vs_calc_creates_synced_three_panel_figure(monkeypatch): @@ -308,10 +308,10 @@ def fake_show_figure(self, fig): ) expected_hovertemplate = ( - 'x: %{x}
' - 'Imeas: %{customdata[0]}
' - 'Icalc: %{customdata[1]}
' - 'Imeas - Icalc: %{customdata[2]}' + 'x: %{x:,.2f}
' + 'Imeas: %{customdata[0]:,.2f}
' + 'Icalc: %{customdata[1]:,.2f}
' + 'Imeas - Icalc: %{customdata[2]:,.2f}' '' ) meas_trace = next(trace for trace in fig.data if trace.name == 'Measured (Imeas)') @@ -338,7 +338,63 @@ def fake_show_figure(self, fig): assert fig.layout.yaxis3.zeroline is False assert fig.layout.xaxis3.title.text == '2θ (degree)' assert 'Miller indices: (1 0 1)' in bragg_traces[0].text[0] - assert 'Bragg peaks: phase-a' in bragg_traces[0].text[0] + assert 'phase-a' in bragg_traces[0].text[0] + + +def test_plot_powder_meas_vs_calc_adds_background_curve(monkeypatch): + import easydiffraction.display.plotters.plotly as pp + + from easydiffraction.display.plotters.base import BraggTickSet + from easydiffraction.display.plotters.base import PowderMeasVsCalcSpec + + captured = {} + + def fake_show_figure(self, fig): + captured['fig'] = fig + + monkeypatch.setattr(pp.PlotlyPlotter, '_show_figure', fake_show_figure) + + plot_spec = PowderMeasVsCalcSpec( + x=np.array([1.0, 2.0, 3.0]), + y_meas=np.array([10.0, 12.0, 11.0]), + y_calc=np.array([9.0, 11.0, 10.5]), + y_resid=np.array([1.0, 1.0, 0.5]), + bragg_tick_sets=( + BraggTickSet( + phase_id='phase-a', + x=np.array([1.5]), + h=np.array([1]), + k=np.array([0]), + ell=np.array([1]), + f_squared_calc=np.array([100.0]), + f_calc=np.array([10.0]), + ), + ), + axes_labels=['2θ (degree)', 'Intensity (arb. units)'], + title='Powder', + residual_height_fraction=0.25, + bragg_peaks_height_fraction=0.10, + height=None, + y_bkg=np.array([1.5, 1.5, 1.5]), + ) + + plotter = pp.PlotlyPlotter() + plotter.plot_powder_meas_vs_calc(plot_spec=plot_spec) + + fig = captured['fig'] + assert len(fig.data) == 5 + background_trace = next(trace for trace in fig.data if trace.name == 'Background (Ibkg)') + meas_trace = next(trace for trace in fig.data if trace.name == 'Measured (Imeas)') + calc_trace = next(trace for trace in fig.data if trace.name == 'Total calculated (Icalc)') + residual_trace = next(trace for trace in fig.data if trace.name == 'Residual (Imeas - Icalc)') + assert list(background_trace.y) == pytest.approx([1.5, 1.5, 1.5]) + assert background_trace.mode == 'lines' + assert background_trace.line.color == pp.DEFAULT_COLORS['bkg'] + assert meas_trace.legendrank < background_trace.legendrank < calc_trace.legendrank + assert residual_trace.legendrank > calc_trace.legendrank + for trace in (meas_trace, background_trace, calc_trace, residual_trace): + assert 'Ibkg: %{customdata[1]' in trace.hovertemplate + assert list(trace.customdata[0]) == pytest.approx([10.0, 1.5, 9.0, 1.0]) def test_bragg_row_height_pixels_scale_linearly_with_phase_count(): diff --git a/tests/unit/easydiffraction/display/test_plotting.py b/tests/unit/easydiffraction/display/test_plotting.py index 656976f48..5e1cc4daf 100644 --- a/tests/unit/easydiffraction/display/test_plotting.py +++ b/tests/unit/easydiffraction/display/test_plotting.py @@ -290,6 +290,7 @@ class Pattern: two_theta = np.array([0.0, 1.0, 2.0, 3.0]) d_spacing = two_theta intensity_meas = np.array([10.0, 20.0, 30.0, 40.0]) + intensity_bkg = np.array([1.0, 2.0, 3.0, 4.0]) intensity_calc = np.array([9.0, 18.0, 27.0, 39.0]) class Refln: @@ -324,6 +325,7 @@ class Experiment: call = captured['powder_meas_vs_calc'] assert np.allclose(call.x, np.array([1.0, 2.0])) assert np.allclose(call.y_meas, np.array([20.0, 30.0])) + assert np.allclose(call.y_bkg, np.array([2.0, 3.0])) assert np.allclose(call.y_calc, np.array([18.0, 27.0])) assert np.allclose(call.y_resid, np.array([2.0, 3.0])) assert [tick_set.phase_id for tick_set in call.bragg_tick_sets] == [ From 1073ff454b73ba8270ccaefd725cd15c48750cc6 Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 08:44:41 +0200 Subject: [PATCH 11/13] Adjust powder trace stacking order --- src/easydiffraction/display/plotters/plotly.py | 2 +- tests/unit/easydiffraction/display/plotters/test_plotly.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/easydiffraction/display/plotters/plotly.py b/src/easydiffraction/display/plotters/plotly.py index 874b3c4db..7fbfaf7fc 100644 --- a/src/easydiffraction/display/plotters/plotly.py +++ b/src/easydiffraction/display/plotters/plotly.py @@ -957,9 +957,9 @@ def plot_powder_meas_vs_calc( main_traces = ( ( + ('meas', plot_spec.y_meas), ('bkg', plot_spec.y_bkg), ('calc', plot_spec.y_calc), - ('meas', plot_spec.y_meas), ) if plot_spec.y_bkg is not None else ( diff --git a/tests/unit/easydiffraction/display/plotters/test_plotly.py b/tests/unit/easydiffraction/display/plotters/test_plotly.py index b2fc16884..2f1f9ce98 100644 --- a/tests/unit/easydiffraction/display/plotters/test_plotly.py +++ b/tests/unit/easydiffraction/display/plotters/test_plotly.py @@ -383,6 +383,11 @@ def fake_show_figure(self, fig): fig = captured['fig'] assert len(fig.data) == 5 + assert [trace.name for trace in fig.data[:3]] == [ + 'Measured (Imeas)', + 'Background (Ibkg)', + 'Total calculated (Icalc)', + ] background_trace = next(trace for trace in fig.data if trace.name == 'Background (Ibkg)') meas_trace = next(trace for trace in fig.data if trace.name == 'Measured (Imeas)') calc_trace = next(trace for trace in fig.data if trace.name == 'Total calculated (Icalc)') From 17f6db2c644e8db03df5c550696dbf451cf94055 Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 08:47:22 +0200 Subject: [PATCH 12/13] Add explicit Plotly powder line width constants --- src/easydiffraction/display/plotters/base.py | 6 +++--- src/easydiffraction/display/plotters/plotly.py | 13 ++++++++++++- .../easydiffraction/display/plotters/test_plotly.py | 5 +++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/easydiffraction/display/plotters/base.py b/src/easydiffraction/display/plotters/base.py index 3c7d25693..b1a19a274 100644 --- a/src/easydiffraction/display/plotters/base.py +++ b/src/easydiffraction/display/plotters/base.py @@ -44,9 +44,9 @@ class PowderMeasVsCalcSpec: """ Specification for one composite powder plot. - The plotting facade assembles the measured, background, - calculated, residual, and Bragg-tick data into this - display-specific object before delegating to a backend. + The plotting facade assembles the measured, background, calculated, + residual, and Bragg-tick data into this display-specific object + before delegating to a backend. """ x: np.ndarray diff --git a/src/easydiffraction/display/plotters/plotly.py b/src/easydiffraction/display/plotters/plotly.py index 7fbfaf7fc..8b9490fa7 100644 --- a/src/easydiffraction/display/plotters/plotly.py +++ b/src/easydiffraction/display/plotters/plotly.py @@ -41,6 +41,11 @@ 'resid': 'rgb(44, 160, 44)', } +MEASURED_LINE_WIDTH = 2.0 +BACKGROUND_LINE_WIDTH = 1.0 +CALCULATED_LINE_WIDTH = 2.0 +RESIDUAL_LINE_WIDTH = 2.0 + BRAGG_TICK_COLORS = ( 'rgb(255, 127, 14)', 'rgb(23, 190, 207)', @@ -392,7 +397,13 @@ def _get_powder_trace( mode = SERIES_CONFIG[label]['mode'] name = SERIES_CONFIG[label]['name'] color = DEFAULT_COLORS[label] - line = {'color': color} + line_width = { + 'meas': MEASURED_LINE_WIDTH, + 'bkg': BACKGROUND_LINE_WIDTH, + 'calc': CALCULATED_LINE_WIDTH, + 'resid': RESIDUAL_LINE_WIDTH, + }[label] + line = {'color': color, 'width': line_width} legend_rank = { 'meas': 10, 'bkg': 20, diff --git a/tests/unit/easydiffraction/display/plotters/test_plotly.py b/tests/unit/easydiffraction/display/plotters/test_plotly.py index 2f1f9ce98..6f43d2221 100644 --- a/tests/unit/easydiffraction/display/plotters/test_plotly.py +++ b/tests/unit/easydiffraction/display/plotters/test_plotly.py @@ -113,6 +113,7 @@ def __init__(self, html): assert hasattr(trace, 'kwargs') assert trace.kwargs['x'] == x assert trace.kwargs['y'] == y + assert trace.kwargs['line']['width'] == pp.CALCULATED_LINE_WIDTH # Exercise plot_powder (non-PyCharm, display path) plotter.plot_powder( @@ -320,6 +321,9 @@ def fake_show_figure(self, fig): assert meas_trace.hovertemplate == expected_hovertemplate assert calc_trace.hovertemplate == expected_hovertemplate assert residual_trace.hovertemplate == expected_hovertemplate + assert meas_trace.line.width == pp.MEASURED_LINE_WIDTH + assert calc_trace.line.width == pp.CALCULATED_LINE_WIDTH + assert residual_trace.line.width == pp.RESIDUAL_LINE_WIDTH assert list(meas_trace.customdata[0]) == pytest.approx([10.0, 9.0, 1.0]) assert list(calc_trace.customdata[0]) == pytest.approx([10.0, 9.0, 1.0]) assert list(residual_trace.customdata[0]) == pytest.approx([10.0, 9.0, 1.0]) @@ -395,6 +399,7 @@ def fake_show_figure(self, fig): assert list(background_trace.y) == pytest.approx([1.5, 1.5, 1.5]) assert background_trace.mode == 'lines' assert background_trace.line.color == pp.DEFAULT_COLORS['bkg'] + assert background_trace.line.width == pp.BACKGROUND_LINE_WIDTH assert meas_trace.legendrank < background_trace.legendrank < calc_trace.legendrank assert residual_trace.legendrank > calc_trace.legendrank for trace in (meas_trace, background_trace, calc_trace, residual_trace): From 1e2d686460abdfcdacd767fda1390f4b981b0c0f Mon Sep 17 00:00:00 2001 From: Andrew Sazonov Date: Tue, 5 May 2026 08:51:12 +0200 Subject: [PATCH 13/13] Pass repo check for powder plot updates --- docs/architecture/package-structure-full.md | 1 + .../display/plotters/plotly.py | 14 ++++++--- src/easydiffraction/display/plotting.py | 31 +++++++++++++------ 3 files changed, 31 insertions(+), 15 deletions(-) diff --git a/docs/architecture/package-structure-full.md b/docs/architecture/package-structure-full.md index f3647b63f..be35bb076 100644 --- a/docs/architecture/package-structure-full.md +++ b/docs/architecture/package-structure-full.md @@ -365,6 +365,7 @@ │ ├── 📄 plotting.py │ │ ├── 🏷️ class PlotterEngineEnum │ │ ├── 🏷️ class _MeasVsCalcPlotOptions +│ │ ├── 🏷️ class _PowderMeasVsCalcSeries │ │ ├── 🏷️ class Plotter │ │ └── 🏷️ class PlotterFactory │ ├── 📄 tables.py diff --git a/src/easydiffraction/display/plotters/plotly.py b/src/easydiffraction/display/plotters/plotly.py index 8b9490fa7..3072b142d 100644 --- a/src/easydiffraction/display/plotters/plotly.py +++ b/src/easydiffraction/display/plotters/plotly.py @@ -937,6 +937,13 @@ def _get_residual_limit(cls, plot_spec: PowderMeasVsCalcSpec) -> float: return cls._nice_axis_limit(float(np.max(np.abs(y_resid)))) + @staticmethod + def _composite_x_range(x_values: np.ndarray) -> tuple[float | None, float | None]: + """Return the explicit x-range for the composite powder plot.""" + if x_values.size == 0: + return None, None + return float(np.min(x_values)), float(np.max(x_values)) + def plot_powder_meas_vs_calc( self, plot_spec: PowderMeasVsCalcSpec, @@ -949,10 +956,7 @@ def plot_powder_meas_vs_calc( residual row is added only when residual data is requested. """ layout = self._get_powder_composite_rows(plot_spec) - x_values = np.asarray(plot_spec.x) - has_x_values = x_values.size > 0 - x_min = float(np.min(x_values)) if has_x_values else None - x_max = float(np.max(x_values)) if has_x_values else None + x_min, x_max = self._composite_x_range(np.asarray(plot_spec.x)) main_y_min, main_y_max = self._get_main_intensity_range(plot_spec) residual_limit = None hover_data = self._powder_meas_vs_calc_hover_data(plot_spec) @@ -1044,7 +1048,7 @@ def plot_powder_meas_vs_calc( 'tickformat': ',.6~g', 'separatethousands': True, } - if has_x_values: + if x_min is not None and x_max is not None: x_axis_kwargs['range'] = [x_min, x_max] fig.update_xaxes(row=row_idx, col=1, **x_axis_kwargs) fig.update_yaxes( diff --git a/src/easydiffraction/display/plotting.py b/src/easydiffraction/display/plotting.py index 49efc5d1c..61e93f683 100644 --- a/src/easydiffraction/display/plotting.py +++ b/src/easydiffraction/display/plotting.py @@ -81,6 +81,15 @@ class _MeasVsCalcPlotOptions: x: object | None = None +@dataclass(frozen=True) +class _PowderMeasVsCalcSeries: + """Filtered y-series for a composite powder plot.""" + + y_meas: np.ndarray + y_calc: np.ndarray + y_bkg: np.ndarray | None = None + + class Plotter(RendererBase): """User-facing plotting facade backed by concrete plotters.""" @@ -1195,14 +1204,18 @@ def _plot_meas_vs_calc_data( else None ) + powder_series = _PowderMeasVsCalcSeries( + y_meas=y_meas, + y_calc=y_calc, + y_bkg=y_bkg, + ) + if sample_form == SampleFormEnum.POWDER and scattering_type == ScatteringTypeEnum.BRAGG: self._plot_powder_bragg_meas_vs_calc( experiment=experiment, expt_name=expt_name, ctx=ctx, - y_meas=y_meas, - y_bkg=y_bkg, - y_calc=y_calc, + series=powder_series, plot_options=plot_options, title=title, ) @@ -1251,9 +1264,7 @@ def _plot_powder_bragg_meas_vs_calc( experiment: object, expt_name: str, ctx: dict[str, object], - y_meas: np.ndarray, - y_bkg: np.ndarray | None, - y_calc: np.ndarray, + series: _PowderMeasVsCalcSeries, plot_options: _MeasVsCalcPlotOptions, title: str, ) -> None: @@ -1261,7 +1272,7 @@ def _plot_powder_bragg_meas_vs_calc( Render the composite powder Bragg measured-vs-calculated plot. """ show_residual = True if plot_options.show_residual is None else plot_options.show_residual - y_resid = y_meas - y_calc if show_residual else None + y_resid = series.y_meas - series.y_calc if show_residual else None if np.asarray(ctx['x_filtered']).size == 0: bragg_tick_sets = () else: @@ -1274,8 +1285,8 @@ def _plot_powder_bragg_meas_vs_calc( ) plot_spec = PowderMeasVsCalcSpec( x=ctx['x_filtered'], - y_meas=y_meas, - y_calc=y_calc, + y_meas=series.y_meas, + y_calc=series.y_calc, y_resid=y_resid, bragg_tick_sets=bragg_tick_sets, axes_labels=ctx['axes_labels'], @@ -1283,7 +1294,7 @@ def _plot_powder_bragg_meas_vs_calc( residual_height_fraction=DEFAULT_RESID_HEIGHT, bragg_peaks_height_fraction=DEFAULT_BRAGG_ROW, height=self._composite_plot_height(), - y_bkg=y_bkg, + y_bkg=series.y_bkg, ) self._backend.plot_powder_meas_vs_calc(plot_spec=plot_spec)