mirror of
https://github.com/bec-project/bec_widgets.git
synced 2026-10-27 00:01:27 +01:00
fix(curve_settings): edit curve copies and drop removed curves
This commit is contained in:
1 parent
a6181c81f5
commit
00f7f566b2
4 files changed
+162
-5
No files matched your search
@@ -737,7 +737,11 @@ class CurveTree(BECWidget, QWidget):
|
||||
self.tree.collapseItem(item)
|
||||
|
||||
def refresh_from_waveform(self):
|
||||
"""Clear the tree and rebuild from the waveform's existing curves if any, else add sample rows."""
|
||||
"""Clear the tree and rebuild from the waveform's existing curves if any, else add sample rows.
|
||||
|
||||
Each row works on a deep copy of its curve's config, so edits stay staged in the tree
|
||||
and only reach the waveform through :meth:`send_curve_json`.
|
||||
"""
|
||||
if self.waveform is None:
|
||||
return
|
||||
self.tree.clear()
|
||||
@@ -746,10 +750,20 @@ class CurveTree(BECWidget, QWidget):
|
||||
top_curves = [c for c in self.waveform.curves if c.config.source in ("device", "history")]
|
||||
dap_curves = [c for c in self.waveform.curves if c.config.source == "dap"]
|
||||
for dev in top_curves:
|
||||
dr = CurveRow(self.tree, parent_item=None, config=dev.config, device_manager=self.dev)
|
||||
dr = CurveRow(
|
||||
self.tree,
|
||||
parent_item=None,
|
||||
config=dev.config.model_copy(deep=True),
|
||||
device_manager=self.dev,
|
||||
)
|
||||
for dap in dap_curves:
|
||||
if dap.config.parent_label == dev.config.label:
|
||||
CurveRow(self.tree, parent_item=dr, config=dap.config, device_manager=self.dev)
|
||||
CurveRow(
|
||||
self.tree,
|
||||
parent_item=dr,
|
||||
config=dap.config.model_copy(deep=True),
|
||||
device_manager=self.dev,
|
||||
)
|
||||
|
||||
def cleanup(self):
|
||||
"""Cleanup the widget."""
|
||||
|
||||
@@ -1346,7 +1346,14 @@ class Waveform(PlotBase):
|
||||
return curve
|
||||
|
||||
def _refresh_history_curves(self):
|
||||
for curve in self._history_curves:
|
||||
"""
|
||||
Reload the data of all history curves from their scans, e.g. after an x-axis change.
|
||||
|
||||
Curves whose source is no longer 'history' are dropped from the history tracking
|
||||
instead of being filled with scan data they do not accept.
|
||||
"""
|
||||
self._history_curves = [c for c in self._history_curves if c.config.source == "history"]
|
||||
for curve in list(self._history_curves):
|
||||
scan_item = self.get_history_scan_item(
|
||||
scan_id=curve.config.scan_id, scan_index=curve.config.scan_number
|
||||
)
|
||||
@@ -1432,6 +1439,7 @@ class Waveform(PlotBase):
|
||||
"""
|
||||
curve_list = self.curves
|
||||
self._dap_curves = []
|
||||
self._history_curves = []
|
||||
self._sync_curves = []
|
||||
self._async_curves = []
|
||||
for curve in curve_list:
|
||||
@@ -1518,6 +1526,12 @@ class Waveform(PlotBase):
|
||||
)
|
||||
curve.rpc_register.remove_rpc(curve)
|
||||
|
||||
# Stop tracking the removed curve, so history refreshes and DAP requests skip it
|
||||
if curve in self._history_curves:
|
||||
self._history_curves.remove(curve)
|
||||
if curve in self._dap_curves:
|
||||
self._dap_curves.remove(curve)
|
||||
|
||||
# Remove itself from the DAP summary only for side panels
|
||||
if (
|
||||
curve.config.source == "dap"
|
||||
|
||||
@@ -5,7 +5,12 @@ import pytest
|
||||
from bec_lib.scan_history import ScanHistory
|
||||
from qtpy.QtWidgets import QComboBox, QVBoxLayout
|
||||
|
||||
from bec_widgets.tests.client_mocks import dap_plugin_message, mocked_client, mocked_client_with_dap
|
||||
from bec_widgets.tests.client_mocks import (
|
||||
dap_plugin_message,
|
||||
inject_scan_history,
|
||||
mocked_client,
|
||||
mocked_client_with_dap,
|
||||
)
|
||||
from bec_widgets.widgets.plots.waveform.settings.curve_settings.curve_setting import CurveSetting
|
||||
from bec_widgets.widgets.plots.waveform.settings.curve_settings.curve_tree import CurveTree
|
||||
from bec_widgets.widgets.plots.waveform.waveform import Waveform
|
||||
@@ -470,3 +475,70 @@ def test_export_data_history_curve(curve_tree_fixture, scan_history_factory):
|
||||
assert exported["scan_number"] == 2
|
||||
assert exported["scan_id"] == "hid2"
|
||||
assert exported["label"] == "bpm4i-bpm4i-scan-2"
|
||||
|
||||
|
||||
##################################################
|
||||
# Staging of edits (issue #1314)
|
||||
##################################################
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def history_curve_setting(qtbot, mocked_client, scan_history_factory):
|
||||
"""
|
||||
A Waveform showing one history curve of scan 1 (two scans in history) and a CurveSetting
|
||||
targeting it.
|
||||
"""
|
||||
wf = create_widget(qtbot, Waveform, client=mocked_client)
|
||||
history_msgs = inject_scan_history(wf, scan_history_factory, ("hist1", 1), ("hist2", 2))
|
||||
# The scan index combobox lists scans by number, so register those as well.
|
||||
for msg in history_msgs:
|
||||
wf.client.history._scan_numbers.append(msg.scan_number)
|
||||
wf.x_mode = "index"
|
||||
history_curve = wf.plot(device_y="bpm4i", signal_y="bpm4i", scan_id="hist1")
|
||||
curve_setting = create_widget(qtbot, CurveSetting, parent=None, target_widget=wf)
|
||||
curve_setting.mode_combo.setCurrentText("index")
|
||||
return curve_setting, wf, history_curve
|
||||
|
||||
|
||||
def test_switch_history_curve_to_live_keeps_history_refresh_working(history_curve_setting):
|
||||
"""
|
||||
Regression for #1314: switching a history curve to 'live' and applying must not leave a
|
||||
device-source curve behind in the Waveform's history tracking. Previously the settings row
|
||||
edited the live curve's config in place, so the removed history curve turned into a
|
||||
device curve that the next history refresh tried to fill, raising
|
||||
"Source device do not allow custom data setting."
|
||||
"""
|
||||
curve_setting, wf, history_curve = history_curve_setting
|
||||
row = curve_setting.curve_manager.tree.topLevelItem(0)
|
||||
assert row.scan_index_combo.scan_id == "hist1"
|
||||
|
||||
row.scan_index_combo.set_scan_id(None) # select 'live'
|
||||
curve_setting.accept_changes()
|
||||
|
||||
# The next history refresh (x-axis change, Apply again, ...) must not raise.
|
||||
wf._refresh_history_curves()
|
||||
|
||||
assert [curve.config.source for curve in wf.curves] == ["device"]
|
||||
assert wf._history_curves == []
|
||||
# The removed history curve kept its own configuration.
|
||||
assert history_curve.config.source == "history"
|
||||
assert history_curve.config.scan_id == "hist1"
|
||||
|
||||
|
||||
def test_curve_settings_edits_do_not_touch_live_curve_config(history_curve_setting):
|
||||
"""
|
||||
Edits in the curve tree are staged on a copy of each curve's config: changing the color,
|
||||
renormalizing colors or exporting the rows must leave the plotted curve untouched until
|
||||
the Waveform applies the exported configuration (also #1304, Cancel keeps color edits).
|
||||
"""
|
||||
curve_setting, wf, history_curve = history_curve_setting
|
||||
curve_manager = curve_setting.curve_manager
|
||||
original_config = history_curve.config.model_dump()
|
||||
|
||||
row = curve_manager.tree.topLevelItem(0)
|
||||
curve_manager.renormalize_colors()
|
||||
row.color_button.set_color("#123456")
|
||||
row.scan_index_combo.set_scan_id(None) # select 'live'
|
||||
curve_manager.export_all_curves()
|
||||
|
||||
assert history_curve.config.model_dump() == original_config
|
||||
@@ -1892,6 +1892,63 @@ def test_history_curve_x_modes_post_plot(qtbot, mocked_client, scan_history_fact
|
||||
assert c.config.current_x_mode == mode
|
||||
|
||||
|
||||
@pytest.mark.parametrize("removal", ["remove_curve", "clear_all"])
|
||||
def test_removed_history_curve_leaves_history_tracking(
|
||||
qtbot, mocked_client, scan_history_factory, removal
|
||||
):
|
||||
"""
|
||||
Regression for #1314: a removed history curve must not stay in the list of curves
|
||||
refreshed from the scan history.
|
||||
"""
|
||||
wf = create_widget(qtbot, Waveform, client=mocked_client)
|
||||
inject_scan_history(wf, scan_history_factory, ("hist1", 1))
|
||||
wf.x_mode = "index"
|
||||
c = wf.plot(device_y="bpm4i", signal_y="bpm4i", scan_id="hist1")
|
||||
assert wf._history_curves == [c]
|
||||
|
||||
if removal == "remove_curve":
|
||||
wf.remove_curve(c.name())
|
||||
else:
|
||||
wf.clear_all()
|
||||
|
||||
assert wf._history_curves == []
|
||||
|
||||
|
||||
def test_history_refresh_skips_curve_that_is_no_longer_history(
|
||||
qtbot, mocked_client, scan_history_factory
|
||||
):
|
||||
"""
|
||||
Regression for #1314: a curve whose source is no longer 'history' is dropped from the
|
||||
history refresh instead of raising "Source device do not allow custom data setting."
|
||||
"""
|
||||
wf = create_widget(qtbot, Waveform, client=mocked_client)
|
||||
inject_scan_history(wf, scan_history_factory, ("hist1", 1))
|
||||
wf.x_mode = "index"
|
||||
c = wf.plot(device_y="bpm4i", signal_y="bpm4i", scan_id="hist1")
|
||||
c.config.source = "device"
|
||||
c.config.scan_id = None
|
||||
c.config.scan_number = None
|
||||
|
||||
wf._refresh_history_curves()
|
||||
|
||||
assert wf._history_curves == []
|
||||
|
||||
|
||||
def test_removed_dap_curve_leaves_dap_tracking(qtbot, mocked_client_with_dap):
|
||||
"""
|
||||
Removing a device curve also removes its DAP child; neither may stay tracked as a DAP curve.
|
||||
"""
|
||||
wf = create_widget(qtbot, Waveform, client=mocked_client_with_dap)
|
||||
wf.plot(arg1="bpm4i", label="bpm4i-bpm4i")
|
||||
dap_curve = wf.add_dap_curve(device_label="bpm4i-bpm4i", dap_name="GaussianModel")
|
||||
assert wf._dap_curves == [dap_curve]
|
||||
|
||||
wf.remove_curve("bpm4i-bpm4i")
|
||||
|
||||
assert wf.curves == []
|
||||
assert wf._dap_curves == []
|
||||
|
||||
|
||||
def test_history_curve_incompatible_x_mode_hides_curve(qtbot, mocked_client, scan_history_factory):
|
||||
"""
|
||||
Test that setting an x_mode not present in stored data hides the history curve.
|
||||
|
||||
Reference in new issue
Block a user