From d9be8e723b1ca58c59211ce896251ce46dae1355 Mon Sep 17 00:00:00 2001 From: Haley Date: Wed, 21 Jan 2026 13:26:20 -0800 Subject: [PATCH 1/3] Refactor plots to use shared tooltip utility --- .../microphonics/plots/base_plot.py | 66 +++---------------- .../microphonics/plots/fft_plot.py | 15 +---- .../microphonics/plots/histogram_plot.py | 15 +---- .../microphonics/plots/spectrogram_plot.py | 23 +++++-- .../microphonics/plots/time_series_plot.py | 6 +- src/sc_linac_physics/utils/plot_tooltip.py | 2 +- 6 files changed, 34 insertions(+), 93 deletions(-) diff --git a/src/sc_linac_physics/applications/microphonics/plots/base_plot.py b/src/sc_linac_physics/applications/microphonics/plots/base_plot.py index 2ac27807..feb5517e 100644 --- a/src/sc_linac_physics/applications/microphonics/plots/base_plot.py +++ b/src/sc_linac_physics/applications/microphonics/plots/base_plot.py @@ -11,7 +11,7 @@ def autoRangeEnabled(self): pg.PlotWidget.autoRangeEnabled = autoRangeEnabled from typing import Tuple, Optional - +from sc_linac_physics.utils.plot_tooltip import PlotTooltip import numpy as np import pyqtgraph as pg from PyQt5.QtCore import Qt @@ -34,7 +34,6 @@ def __init__(self, parent=None, plot_type=None, config=None): self.plot_type = plot_type self.config = config or {} self.plot_curves = {} - self.tooltips = {} # Setup UI components common to all plots self.setup_ui() @@ -117,13 +116,14 @@ def _setup_plot_widget(self, plot_type, config): if "y_range" in config: widget.setYRange(*config["y_range"]) - # Connect signal for tooltips - widget.scene().sigMouseMoved.connect( - lambda ev: self._show_tooltip(plot_type, ev) - ) + self.tooltip = PlotTooltip(self.plot_widget, self.get_formatter()) return widget + def get_formatter(self): + """Override in subclasses for custom tooltip formatting.""" + return lambda x, y: f"X: {x:.2f}, Y: {y:.2f}" + def _get_cavity_pen(self, cavity_num): """ Create a pen for the specified cavity w/ appropriate styling @@ -153,56 +153,6 @@ def _get_cavity_pen(self, cavity_num): return pg.mkPen(qcolor, width=2, style=style) - def _show_tooltip(self, plot_type, ev): - """ - Show tooltip w/ data values on hover - - Args: - plot_type: Type of plot - ev: Mouse event - """ - try: - plot = self.plot_widget - view = plot.plotItem.vb - if plot.sceneBoundingRect().contains(ev): - mouse_point = view.mapSceneToView(ev) - x, y = mouse_point.x(), mouse_point.y() - - # Format tooltip based on plot type - tooltip = self._format_tooltip(plot_type, x, y) - if not tooltip: - return - - # Update/create tooltip label - if plot_type not in self.tooltips: - self.tooltips[plot_type] = pg.TextItem( - text=tooltip, - color=(255, 255, 255), - border="k", - fill=(0, 0, 0, 180), - ) - plot.addItem(self.tooltips[plot_type]) - - self.tooltips[plot_type].setText(tooltip) - self.tooltips[plot_type].setPos(x, y) - self.tooltips[plot_type].show() - except Exception as e: - print(f"Tooltip error: {str(e)}") - - def _format_tooltip(self, plot_type, x, y): - """ - Format tooltip text based on plot type - - Args: - plot_type: Type of plot - x: X coordinate - y: Y coordinate - - Returns: - str: Tooltip text - """ - return f"X: {x:.2f}, Y: {y:.2f}" - def _preprocess_data( self, cavity_channel_data: dict, channel_type: str = "DF" ) -> Tuple[Optional[np.ndarray], bool]: @@ -263,8 +213,8 @@ def clear_plot(self): self.plot_widget.clear() self.plot_curves = {} # Hide tooltip if it exists - if self.plot_type in self.tooltips: - self.tooltips[self.plot_type].hide() + if self.tooltip: + self.tooltip.hide() def update_plot(self, cavity_num, data): """ diff --git a/src/sc_linac_physics/applications/microphonics/plots/fft_plot.py b/src/sc_linac_physics/applications/microphonics/plots/fft_plot.py index 299db793..69371d83 100644 --- a/src/sc_linac_physics/applications/microphonics/plots/fft_plot.py +++ b/src/sc_linac_physics/applications/microphonics/plots/fft_plot.py @@ -67,18 +67,9 @@ def set_plot_config(self, panel_wide_config): default_y_range = self.config.get("y_range", (0, 1.5)) self.plot_widget.setYRange(*default_y_range) - def _format_tooltip(self, plot_type, x, y): - """Format tooltip text for FFT plot - - Args: - plot_type: Type of plot (unused in this implementation) - x: X coordinate (frequency in Hz) - y: Y coordinate (amplitude) - - Returns: - str: Formatted tooltip text - """ - return f"Frequency: {x:.1f} Hz\nAmplitude: {y:.3f}" + def get_formatter(self): + """Tooltip formatter for FFT plot.""" + return lambda x, y: f"Frequency: {x:.1f} Hz\nAmplitude: {y:.3f}" def update_plot(self, cavity_num, cavity_channel_data): """Update FFT plot w/ new data diff --git a/src/sc_linac_physics/applications/microphonics/plots/histogram_plot.py b/src/sc_linac_physics/applications/microphonics/plots/histogram_plot.py index baf84aac..90fbb2d1 100644 --- a/src/sc_linac_physics/applications/microphonics/plots/histogram_plot.py +++ b/src/sc_linac_physics/applications/microphonics/plots/histogram_plot.py @@ -26,18 +26,9 @@ def __init__(self, parent=None): self.num_bins = 140 # Default number of bins super().__init__(parent, plot_type="histogram", config=config) - def _format_tooltip(self, plot_type, x, y): - """Format tooltip text specifically for histogram plot - - Args: - plot_type: Type of plot (unused in this implementation) - x: X coordinate (detuning in Hz) - y: Y coordinate (count) - - Returns: - str: Formatted tooltip text - """ - return f"Detuning: {x:.1f} Hz\nCount: {int(max(1, y))}" + def get_formatter(self): + """Tooltip formatter for histogram plot.""" + return lambda x, y: f"Detuning: {x:.1f} Hz\nCount: {int(max(1, y))}" def _update_data_range(self, df_data): """Update data range based on current data.""" diff --git a/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py b/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py index b798a08d..2e63d0b5 100644 --- a/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py +++ b/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py @@ -9,6 +9,7 @@ from sc_linac_physics.applications.microphonics.utils.data_processing import ( calculate_spectrogram, ) +from sc_linac_physics.utils.plot_tooltip import PlotTooltip class SpectrogramPlot(BasePlot): @@ -32,6 +33,7 @@ def __init__(self, parent=None): # Colorbar self.colormap = pg.colormap.get("viridis") self.colorbar = None + self.plot_tooltips = {} # Grid controls self.grid_controls_widget = None @@ -86,6 +88,9 @@ def setup_plot(self): def _refresh_grid_layout(self): """Central method to refresh entire grid layout""" + for tooltip in self.plot_tooltips.values(): + tooltip.cleanup() + self.plot_tooltips.clear() # Clear all self.graphics_layout.clear() self.plot_items.clear() @@ -151,14 +156,18 @@ def _refresh_grid_layout(self): plot_item.setXRange(t[0], t[-1], padding=0) plot_item.setYRange(f[0], f[-1], padding=0) + self.plot_tooltips[cavity_num] = PlotTooltip( + plot_item, + lambda x, y: f"Time: {x:.3f} s\nFrequency: {y:.1f} Hz", + ) # Add colorbar if visible_cavities: self._add_colorbar(num_rows) - def _format_tooltip(self, plot_type, x, y): - """Format tooltip text specifically for spectrogram plot""" - return f"Time: {x:.3f} s\nFrequency: {y:.1f} Hz" + def _show_tooltip(self, plot_type, ev): + """Override tooltip behavior""" + pass def update_plot(self, cavity_num, cavity_channel_data): df_data, is_valid = self._preprocess_data( @@ -270,14 +279,14 @@ def set_plot_config(self, config): elif self.cavity_data_cache: self._refresh_grid_layout() - def _show_tooltip(self, plot_type, ev): - """Override tooltip behavior""" - pass - def clear_plot(self): """Clear all plot data""" self.cavity_data_cache.clear() self.cavity_order.clear() self.cavity_is_visible_flags.clear() + for tooltip in self.plot_tooltips.values(): + tooltip.cleanup() + self.plot_tooltips.clear() + self._refresh_grid_layout() diff --git a/src/sc_linac_physics/applications/microphonics/plots/time_series_plot.py b/src/sc_linac_physics/applications/microphonics/plots/time_series_plot.py index e7666f3a..d28c3988 100644 --- a/src/sc_linac_physics/applications/microphonics/plots/time_series_plot.py +++ b/src/sc_linac_physics/applications/microphonics/plots/time_series_plot.py @@ -39,9 +39,9 @@ def __init__(self, parent=None): vb.setLimits(xMin=0) # Prevent negative time vb.sigRangeChangedManually.connect(self._on_range_changed) - def _format_tooltip(self, plot_type, x, y): - """Override base tooltip formatting""" - return f"Time: {x:.3f} s\nDetuning: {y:.2f} Hz" + def get_formatter(self): + """Tooltip formatter for time series plot.""" + return lambda x, y: f"Time: {x:.3f} s\nDetuning: {y:.2f} Hz" def _decimate_data(self, times, values, target_points): """Decimation that still preserves important features""" diff --git a/src/sc_linac_physics/utils/plot_tooltip.py b/src/sc_linac_physics/utils/plot_tooltip.py index 98bf57d4..0a0ed4f2 100644 --- a/src/sc_linac_physics/utils/plot_tooltip.py +++ b/src/sc_linac_physics/utils/plot_tooltip.py @@ -36,7 +36,7 @@ def __init__( self._container = plot else: self._plot_item = plot - self._container = plot + self._container = plot.getViewBox() self.formatter = formatter or format_default self.enabled = True From 7294e6286000a0de4480652033ab1ba92cfa7e77 Mon Sep 17 00:00:00 2001 From: Haley Date: Thu, 5 Feb 2026 17:06:06 -0800 Subject: [PATCH 2/3] Add make_formatter helper per review feedback --- .../microphonics/plots/fft_plot.py | 3 ++- .../microphonics/plots/histogram_plot.py | 3 ++- .../microphonics/plots/spectrogram_plot.py | 18 +++--------------- .../microphonics/plots/time_series_plot.py | 3 ++- src/sc_linac_physics/utils/plot_tooltip.py | 7 +++++++ 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/sc_linac_physics/applications/microphonics/plots/fft_plot.py b/src/sc_linac_physics/applications/microphonics/plots/fft_plot.py index 69371d83..2abb655a 100644 --- a/src/sc_linac_physics/applications/microphonics/plots/fft_plot.py +++ b/src/sc_linac_physics/applications/microphonics/plots/fft_plot.py @@ -7,6 +7,7 @@ from sc_linac_physics.applications.microphonics.utils.data_processing import ( calculate_fft, ) +from sc_linac_physics.utils.plot_tooltip import PlotTooltip class FFTPlot(BasePlot): @@ -69,7 +70,7 @@ def set_plot_config(self, panel_wide_config): def get_formatter(self): """Tooltip formatter for FFT plot.""" - return lambda x, y: f"Frequency: {x:.1f} Hz\nAmplitude: {y:.3f}" + return PlotTooltip.make_formatter("Frequency (Hz)", "Amplitude") def update_plot(self, cavity_num, cavity_channel_data): """Update FFT plot w/ new data diff --git a/src/sc_linac_physics/applications/microphonics/plots/histogram_plot.py b/src/sc_linac_physics/applications/microphonics/plots/histogram_plot.py index 90fbb2d1..c4800849 100644 --- a/src/sc_linac_physics/applications/microphonics/plots/histogram_plot.py +++ b/src/sc_linac_physics/applications/microphonics/plots/histogram_plot.py @@ -4,6 +4,7 @@ from sc_linac_physics.applications.microphonics.utils.data_processing import ( calculate_histogram, ) +from sc_linac_physics.utils.plot_tooltip import PlotTooltip class HistogramPlot(BasePlot): @@ -28,7 +29,7 @@ def __init__(self, parent=None): def get_formatter(self): """Tooltip formatter for histogram plot.""" - return lambda x, y: f"Detuning: {x:.1f} Hz\nCount: {int(max(1, y))}" + return PlotTooltip.make_formatter("Detuning (Hz)", "Count") def _update_data_range(self, df_data): """Update data range based on current data.""" diff --git a/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py b/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py index 2e63d0b5..d2d95f45 100644 --- a/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py +++ b/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py @@ -33,7 +33,6 @@ def __init__(self, parent=None): # Colorbar self.colormap = pg.colormap.get("viridis") self.colorbar = None - self.plot_tooltips = {} # Grid controls self.grid_controls_widget = None @@ -88,9 +87,6 @@ def setup_plot(self): def _refresh_grid_layout(self): """Central method to refresh entire grid layout""" - for tooltip in self.plot_tooltips.values(): - tooltip.cleanup() - self.plot_tooltips.clear() # Clear all self.graphics_layout.clear() self.plot_items.clear() @@ -156,18 +152,14 @@ def _refresh_grid_layout(self): plot_item.setXRange(t[0], t[-1], padding=0) plot_item.setYRange(f[0], f[-1], padding=0) - self.plot_tooltips[cavity_num] = PlotTooltip( - plot_item, - lambda x, y: f"Time: {x:.3f} s\nFrequency: {y:.1f} Hz", - ) # Add colorbar if visible_cavities: self._add_colorbar(num_rows) - def _show_tooltip(self, plot_type, ev): - """Override tooltip behavior""" - pass + def get_formatter(self): + """Tooltip formatter for spectrogram plot.""" + return PlotTooltip.make_formatter("Time (s)", "Frequency (Hz)") def update_plot(self, cavity_num, cavity_channel_data): df_data, is_valid = self._preprocess_data( @@ -285,8 +277,4 @@ def clear_plot(self): self.cavity_order.clear() self.cavity_is_visible_flags.clear() - for tooltip in self.plot_tooltips.values(): - tooltip.cleanup() - self.plot_tooltips.clear() - self._refresh_grid_layout() diff --git a/src/sc_linac_physics/applications/microphonics/plots/time_series_plot.py b/src/sc_linac_physics/applications/microphonics/plots/time_series_plot.py index d28c3988..a303b845 100644 --- a/src/sc_linac_physics/applications/microphonics/plots/time_series_plot.py +++ b/src/sc_linac_physics/applications/microphonics/plots/time_series_plot.py @@ -7,6 +7,7 @@ from sc_linac_physics.applications.microphonics.utils.constants import ( BASE_HARDWARE_SAMPLE_RATE, ) +from sc_linac_physics.utils.plot_tooltip import PlotTooltip class TimeSeriesPlot(BasePlot): @@ -41,7 +42,7 @@ def __init__(self, parent=None): def get_formatter(self): """Tooltip formatter for time series plot.""" - return lambda x, y: f"Time: {x:.3f} s\nDetuning: {y:.2f} Hz" + return PlotTooltip.make_formatter("Time (s)", "Detuning (Hz)") def _decimate_data(self, times, values, target_points): """Decimation that still preserves important features""" diff --git a/src/sc_linac_physics/utils/plot_tooltip.py b/src/sc_linac_physics/utils/plot_tooltip.py index 0a0ed4f2..38b80a71 100644 --- a/src/sc_linac_physics/utils/plot_tooltip.py +++ b/src/sc_linac_physics/utils/plot_tooltip.py @@ -98,3 +98,10 @@ def cleanup(self): self._plot_item.removeItem(self._tooltip) except (RuntimeError, AttributeError): pass + + @staticmethod + def make_formatter( + x_label: str, y_label: str + ) -> Callable[[float, float], str]: + """Create a formatter from axis labels.""" + return lambda x, y: f"{x_label}: {x:.2f}\n{y_label}: {y:.2f}" From 4d2f9ad773caeafa2176463eb0d22ceba63fa45b Mon Sep 17 00:00:00 2001 From: Haley Date: Sun, 10 May 2026 19:45:24 -0700 Subject: [PATCH 3/3] Address review feedback: move default formatter into PlotTooltip and fix tooltip init bug --- .../applications/microphonics/plots/base_plot.py | 4 ++-- .../applications/microphonics/plots/spectrogram_plot.py | 5 ----- src/sc_linac_physics/utils/plot_tooltip.py | 9 ++++++--- .../microphonics/plots/test_time_series_plot.py | 9 +++++---- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/sc_linac_physics/applications/microphonics/plots/base_plot.py b/src/sc_linac_physics/applications/microphonics/plots/base_plot.py index feb5517e..1c0d0cc7 100644 --- a/src/sc_linac_physics/applications/microphonics/plots/base_plot.py +++ b/src/sc_linac_physics/applications/microphonics/plots/base_plot.py @@ -116,13 +116,13 @@ def _setup_plot_widget(self, plot_type, config): if "y_range" in config: widget.setYRange(*config["y_range"]) - self.tooltip = PlotTooltip(self.plot_widget, self.get_formatter()) + self.tooltip = PlotTooltip(widget, self.get_formatter()) return widget def get_formatter(self): """Override in subclasses for custom tooltip formatting.""" - return lambda x, y: f"X: {x:.2f}, Y: {y:.2f}" + return None def _get_cavity_pen(self, cavity_num): """ diff --git a/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py b/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py index d2d95f45..682022c7 100644 --- a/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py +++ b/src/sc_linac_physics/applications/microphonics/plots/spectrogram_plot.py @@ -9,7 +9,6 @@ from sc_linac_physics.applications.microphonics.utils.data_processing import ( calculate_spectrogram, ) -from sc_linac_physics.utils.plot_tooltip import PlotTooltip class SpectrogramPlot(BasePlot): @@ -157,10 +156,6 @@ def _refresh_grid_layout(self): if visible_cavities: self._add_colorbar(num_rows) - def get_formatter(self): - """Tooltip formatter for spectrogram plot.""" - return PlotTooltip.make_formatter("Time (s)", "Frequency (Hz)") - def update_plot(self, cavity_num, cavity_channel_data): df_data, is_valid = self._preprocess_data( cavity_channel_data, channel_type="DF" diff --git a/src/sc_linac_physics/utils/plot_tooltip.py b/src/sc_linac_physics/utils/plot_tooltip.py index 38b80a71..82178047 100644 --- a/src/sc_linac_physics/utils/plot_tooltip.py +++ b/src/sc_linac_physics/utils/plot_tooltip.py @@ -101,7 +101,10 @@ def cleanup(self): @staticmethod def make_formatter( - x_label: str, y_label: str + x_label: str, + y_label: str, + x_fmt: str = ".2f", + y_fmt: str = ".2f", ) -> Callable[[float, float], str]: - """Create a formatter from axis labels.""" - return lambda x, y: f"{x_label}: {x:.2f}\n{y_label}: {y:.2f}" + """Create a formatter from axis labels and optional format specs.""" + return lambda x, y: f"{x_label}: {x:{x_fmt}}\n{y_label}: {y:{y_fmt}}" diff --git a/tests/applications/microphonics/plots/test_time_series_plot.py b/tests/applications/microphonics/plots/test_time_series_plot.py index d8f7bff4..dab70591 100644 --- a/tests/applications/microphonics/plots/test_time_series_plot.py +++ b/tests/applications/microphonics/plots/test_time_series_plot.py @@ -428,11 +428,12 @@ def test_end_zoom_resets_flag(self, plot_widget): def test_format_tooltip(self, plot_widget): """Test tooltip formatting""" - tooltip = plot_widget._format_tooltip("time_series", x=5.123, y=123.456) + formatter = plot_widget.get_formatter() + tooltip = formatter(5.123, 123.456) - assert "Time:" in tooltip - assert "5.123" in tooltip - assert "Detuning:" in tooltip + assert "Time" in tooltip + assert "5.12" in tooltip + assert "Detuning" in tooltip assert "123.46" in tooltip # ===== Clear Plot Tests =====