From 97fddf01965ec9979cb77feb619035e6426fe1aa Mon Sep 17 00:00:00 2001 From: David Perl Date: Fri, 5 Sep 2025 12:23:41 +0200 Subject: [PATCH] refactor: genericise config form --- bec_widgets/utils/forms_from_types/items.py | 16 +- .../services/device_browser/device_browser.py | 8 +- .../device_item/device_config_dialog.py | 170 +++++++++--------- .../device_browser/device_item/device_item.py | 4 +- .../test_device_config_form_dialog.py | 6 +- 5 files changed, 106 insertions(+), 98 deletions(-) diff --git a/bec_widgets/utils/forms_from_types/items.py b/bec_widgets/utils/forms_from_types/items.py index e9d92db6..c9d83801 100644 --- a/bec_widgets/utils/forms_from_types/items.py +++ b/bec_widgets/utils/forms_from_types/items.py @@ -442,10 +442,17 @@ class ListFormItem(DynamicFormItem): self._add_list_item(val) self._repop(self._data) + def _item_height(self): + return int(QFontMetrics(self.font()).height() * 1.5) + def _add_list_item(self, val): item = QListWidgetItem(self._main_widget) item.setFlags(item.flags() | QtCore.Qt.ItemIsEditable | QtCore.Qt.ItemIsEditable) item_widget = self._types.widget(parent=self) + item_widget.setMinimumHeight(self._item_height()) + self._main_widget.setGridSize(QSize(0, self._item_height())) + if (layout := item_widget.layout()) is not None: + layout.setContentsMargins(0, 0, 0, 0) WidgetIO.set_value(item_widget, val) self._main_widget.setItemWidget(item, item_widget) self._main_widget.addItem(item) @@ -482,14 +489,11 @@ class ListFormItem(DynamicFormItem): self._data = list(value) self._repop(self._data) - def _line_height(self): - return QFontMetrics(self._main_widget.font()).height() - def set_max_height_in_lines(self, lines: int): outer_inc = 1 if self._spec.pretty_display else 3 - self._main_widget.setFixedHeight(self._line_height() * max(lines, self._min_lines)) - self._button_holder.setFixedHeight(self._line_height() * (max(lines, self._min_lines) + 1)) - self.setFixedHeight(self._line_height() * (max(lines, self._min_lines) + outer_inc)) + self._main_widget.setFixedHeight(self._item_height() * max(lines, self._min_lines)) + self._button_holder.setFixedHeight(self._item_height() * (max(lines, self._min_lines) + 1)) + self.setFixedHeight(self._item_height() * (max(lines, self._min_lines) + outer_inc)) def scale_to_data(self, *_): self.set_max_height_in_lines(self._main_widget.count() + 1) diff --git a/bec_widgets/widgets/services/device_browser/device_browser.py b/bec_widgets/widgets/services/device_browser/device_browser.py index 3bf106ac..2a75217a 100644 --- a/bec_widgets/widgets/services/device_browser/device_browser.py +++ b/bec_widgets/widgets/services/device_browser/device_browser.py @@ -1,6 +1,4 @@ import os -import re -from functools import partial from typing import Callable import bec_lib @@ -21,7 +19,7 @@ from bec_widgets.utils.list_of_expandable_frames import ListOfExpandableFrames from bec_widgets.utils.ui_loader import UILoader from bec_widgets.widgets.services.device_browser.device_item import DeviceItem from bec_widgets.widgets.services.device_browser.device_item.device_config_dialog import ( - DeviceConfigDialog, + DirectUpdateDeviceConfigDialog, ) from bec_widgets.widgets.services.device_browser.util import map_device_type_to_icon @@ -109,7 +107,7 @@ class DeviceBrowser(BECWidget, QWidget): ) def _create_add_dialog(self): - dialog = DeviceConfigDialog(parent=self, device=None, action="add") + dialog = DirectUpdateDeviceConfigDialog(parent=self, device=None, action="add") dialog.open() def on_device_update(self, action: ConfigAction, content: dict) -> None: @@ -134,7 +132,7 @@ class DeviceBrowser(BECWidget, QWidget): def _add_item_to_list(self, device: str, device_obj): - device_item = self.dev_list.add_item( + _, device_item = self.dev_list.add_item( id=device, parent=self, device=device, diff --git a/bec_widgets/widgets/services/device_browser/device_item/device_config_dialog.py b/bec_widgets/widgets/services/device_browser/device_item/device_config_dialog.py index d81a2beb..2fc8f70e 100644 --- a/bec_widgets/widgets/services/device_browser/device_item/device_config_dialog.py +++ b/bec_widgets/widgets/services/device_browser/device_item/device_config_dialog.py @@ -1,12 +1,12 @@ from ast import literal_eval -from typing import Literal +from typing import Any, Literal from bec_lib.atlas_models import Device as DeviceConfigModel from bec_lib.config_helper import CONF as DEVICE_CONF_KEYS from bec_lib.config_helper import ConfigHelper from bec_lib.logger import bec_logger -from pydantic import ValidationError, field_validator -from qtpy.QtCore import QSize, Qt, QThreadPool, Signal +from pydantic import field_validator +from qtpy.QtCore import QSize, Qt, QThreadPool, Signal # type: ignore from qtpy.QtWidgets import ( QApplication, QDialog, @@ -29,6 +29,8 @@ from bec_widgets.widgets.utility.spinner.spinner import SpinnerWidget logger = bec_logger.logger +_StdBtn = QDialogButtonBox.StandardButton + def _try_literal_eval(value: str): if value == "": @@ -39,20 +41,11 @@ def _try_literal_eval(value: str): raise ValueError(f"Entered config value {value} is not a valid python value!") from e -class DeviceConfigDialog(BECWidget, QDialog): +class DeviceConfigDialog(QDialog): RPC = False applied = Signal() - def __init__( - self, - *, - parent=None, - device: str | None = None, - config_helper: ConfigHelper | None = None, - action: Literal["update", "add"] = "update", - threadpool: QThreadPool | None = None, - **kwargs, - ): + def __init__(self, *, parent=None, **kwargs): """A dialog to edit the configuration of a device in BEC. Generated from the pydantic model for device specification in bec_lib.atlas_models. @@ -64,71 +57,26 @@ class DeviceConfigDialog(BECWidget, QDialog): """ self._initial_config = {} super().__init__(parent=parent, **kwargs) - self._config_helper = config_helper or ConfigHelper( - self.client.connector, self.client._service_name - ) - self._device = device - self._action: Literal["update", "add"] = action - self._q_threadpool = threadpool or QThreadPool() - self.setWindowTitle(f"Edit config for: {device}") + self._container = QStackedLayout() - self._container.setStackingMode(QStackedLayout.StackAll) + self._container.setStackingMode(QStackedLayout.StackingMode.StackAll) self._layout = QVBoxLayout() - user_warning = QLabel( - "Warning: edit items here at your own risk - minimal validation is applied to the entered values.\n" - "Items in the deviceConfig dictionary should correspond to python literals, e.g. numbers, lists, strings (including quotes), etc." - ) - user_warning.setWordWrap(True) - user_warning.setStyleSheet("QLabel { color: red; }") - self._layout.addWidget(user_warning) - self.get_bec_shortcuts() + self._add_form() - if self._action == "update": - self._form._validity.setVisible(False) - else: - self._set_schema_to_check_devices() - # TODO: replace when https://github.com/bec-project/bec/issues/528 https://github.com/bec-project/bec/issues/547 are resolved - # self._form._validity.setVisible(True) - self._form.validity_proc.connect(self.enable_buttons_for_validity) self._add_overlay() self._add_buttons() - + self.setWindowTitle("Add new device") self.setLayout(self._container) self._form.validate_form() self._overlay_widget.setVisible(False) - def _set_schema_to_check_devices(self): - class _NameValidatedConfigModel(DeviceConfigModel): - @field_validator("name") - @staticmethod - def _validate_name(value: str, *_): - if not value.isidentifier(): - raise ValueError( - f"Invalid device name: {value}. Device names must be valid Python identifiers." - ) - if value in self.dev: - raise ValueError(f"A device with name {value} already exists!") - return value - - self._form.set_schema(_NameValidatedConfigModel) - def _add_form(self): self._form_widget = QWidget() self._form_widget.setLayout(self._layout) self._form = DeviceConfigForm() self._layout.addWidget(self._form) - for row in self._form.enumerate_form_widgets(): - if ( - row.label.property("_model_field_name") in DEVICE_CONF_KEYS.NON_UPDATABLE - and self._action == "update" - ): - row.widget._set_pretty_display() - - if self._action == "update" and self._device in self.dev: - self._fetch_config() - self._fill_form() self._container.addWidget(self._form_widget) def _add_overlay(self): @@ -145,21 +93,12 @@ class DeviceConfigDialog(BECWidget, QDialog): self._container.addWidget(self._overlay_widget) def _add_buttons(self): - self.button_box = QDialogButtonBox( - QDialogButtonBox.Apply | QDialogButtonBox.Ok | QDialogButtonBox.Cancel - ) - self.button_box.button(QDialogButtonBox.Apply).clicked.connect(self.apply) + self.button_box = QDialogButtonBox(_StdBtn.Apply | _StdBtn.Ok | _StdBtn.Cancel) + self.button_box.button(_StdBtn.Apply).clicked.connect(self.apply) self.button_box.accepted.connect(self.accept) self.button_box.rejected.connect(self.reject) self._layout.addWidget(self.button_box) - def _fetch_config(self): - if ( - self.client.device_manager is not None - and self._device in self.client.device_manager.devices - ): - self._initial_config = self.client.device_manager.devices.get(self._device)._config - def _fill_form(self): self._form.set_data(DeviceConfigModel.model_validate(self._initial_config)) @@ -190,12 +129,12 @@ class DeviceConfigDialog(BECWidget, QDialog): @SafeSlot(bool) def enable_buttons_for_validity(self, valid: bool): # TODO: replace when https://github.com/bec-project/bec/issues/528 https://github.com/bec-project/bec/issues/547 are resolved - for button in [ - self.button_box.button(b) for b in [QDialogButtonBox.Apply, QDialogButtonBox.Ok] - ]: + for button in [self.button_box.button(b) for b in [_StdBtn.Apply, _StdBtn.Ok]]: button.setEnabled(valid) button.setToolTip(self._form._validity_message.text()) + def _process_action(self): ... + @SafeSlot(popup_error=True) def apply(self): self._process_action() @@ -206,10 +145,77 @@ class DeviceConfigDialog(BECWidget, QDialog): self._process_action() return super().accept() + +class DirectUpdateDeviceConfigDialog(BECWidget, DeviceConfigDialog): + def __init__( + self, + *, + parent=None, + device: str | None = None, + config_helper: ConfigHelper | None = None, + action: Literal["update"] | Literal["add"] = "update", + threadpool: QThreadPool | None = None, + **kwargs, + ): + self._device = device + self._q_threadpool = threadpool or QThreadPool() + self._config_helper = config_helper or ConfigHelper( + self.client.connector, self.client._service_name + ) + super().__init__(parent=parent, **kwargs) + self.get_bec_shortcuts() + self._action: Literal["update", "add"] = action + user_warning = QLabel( + "Warning: edit items here at your own risk - minimal validation is applied to the entered values.\n" + "Items in the deviceConfig dictionary should correspond to python literals, e.g. numbers, lists, strings (including quotes), etc." + ) + user_warning.setWordWrap(True) + user_warning.setStyleSheet("QLabel { color: red; }") + self._layout.insertWidget(0, user_warning) + self.setWindowTitle( + f"Edit config for: {device}" if action == "update" else "Add new device" + ) + + if self._action == "update": + self._modify_for_update() + else: + self._set_schema_to_check_devices() + # TODO: replace when https://github.com/bec-project/bec/issues/528 https://github.com/bec-project/bec/issues/547 are resolved + # self._form._validity.setVisible(True) + self._form.validity_proc.connect(self.enable_buttons_for_validity) + + def _modify_for_update(self): + for row in self._form.enumerate_form_widgets(): + if row.label.property("_model_field_name") in DEVICE_CONF_KEYS.NON_UPDATABLE: + row.widget._set_pretty_display() + if self._device in self.dev: + self._fetch_config() + self._fill_form() + self._form._validity.setVisible(False) + + def _set_schema_to_check_devices(self): + class _NameValidatedConfigModel(DeviceConfigModel): + @field_validator("name") + @staticmethod + def _validate_name(value: str, *_): + if not value.isidentifier(): + raise ValueError( + f"Invalid device name: {value}. Device names must be valid Python identifiers." + ) + if value in self.dev: + raise ValueError(f"A device with name {value} already exists!") + return value + + self._form.set_schema(_NameValidatedConfigModel) + + def _fetch_config(self): + if self.dev is not None and (device := self.dev.get(self._device)) is not None: # type: ignore + self._initial_config = device._config + def _process_action(self): updated_config = self.updated_config() if self._action == "add": - if (name := updated_config.get("name")) in self.dev: + if self.dev is not None and (name := updated_config.get("name")) in self.dev: raise ValueError( f"Can't create a new device with the same name as already existing device {name}!" ) @@ -269,10 +275,10 @@ def main(): # pragma: no cover app = QApplication(sys.argv) apply_theme("light") widget = QWidget() - widget.setLayout(QVBoxLayout()) + widget.setLayout(layout := QVBoxLayout()) device = QLineEdit() - widget.layout().addWidget(device) + layout.addWidget(device) def _destroy_dialog(*_): nonlocal dialog @@ -286,13 +292,13 @@ def main(): # pragma: no cover nonlocal dialog if dialog is None: kwargs = {"device": dev} if (dev := device.text()) else {"action": "add"} - dialog = DeviceConfigDialog(**kwargs) + dialog = DirectUpdateDeviceConfigDialog(**kwargs) # type: ignore dialog.accepted.connect(accept) dialog.rejected.connect(_destroy_dialog) dialog.open() button = QPushButton("Show device dialog") - widget.layout().addWidget(button) + layout.addWidget(button) button.clicked.connect(_show_dialog) widget.show() sys.exit(app.exec_()) diff --git a/bec_widgets/widgets/services/device_browser/device_item/device_item.py b/bec_widgets/widgets/services/device_browser/device_item/device_item.py index f24e2fd2..45f233cb 100644 --- a/bec_widgets/widgets/services/device_browser/device_item/device_item.py +++ b/bec_widgets/widgets/services/device_browser/device_item/device_item.py @@ -18,7 +18,7 @@ from bec_widgets.widgets.services.device_browser.device_item.config_communicator CommunicateConfigAction, ) from bec_widgets.widgets.services.device_browser.device_item.device_config_dialog import ( - DeviceConfigDialog, + DirectUpdateDeviceConfigDialog, ) from bec_widgets.widgets.services.device_browser.device_item.device_config_form import ( DeviceConfigForm, @@ -91,7 +91,7 @@ class DeviceItem(ExpandableGroupFrame): @SafeSlot() def _create_edit_dialog(self): - dialog = DeviceConfigDialog( + dialog = DirectUpdateDeviceConfigDialog( parent=self, device=self.device, config_helper=self._config_helper, diff --git a/tests/unit_tests/test_device_config_form_dialog.py b/tests/unit_tests/test_device_config_form_dialog.py index e176da5b..22d71226 100644 --- a/tests/unit_tests/test_device_config_form_dialog.py +++ b/tests/unit_tests/test_device_config_form_dialog.py @@ -6,7 +6,7 @@ from qtpy.QtWidgets import QDialogButtonBox, QPushButton from bec_widgets.utils.forms_from_types.items import StrFormItem from bec_widgets.widgets.services.device_browser.device_item.device_config_dialog import ( - DeviceConfigDialog, + DirectUpdateDeviceConfigDialog, _try_literal_eval, ) @@ -29,7 +29,7 @@ def mock_client(): @pytest.fixture def update_dialog(mock_client, qtbot): """Fixture to create a DeviceConfigDialog instance.""" - update_dialog = DeviceConfigDialog( + update_dialog = DirectUpdateDeviceConfigDialog( device="test_device", config_helper=MagicMock(), client=mock_client ) qtbot.addWidget(update_dialog) @@ -39,7 +39,7 @@ def update_dialog(mock_client, qtbot): @pytest.fixture def add_dialog(mock_client, qtbot): """Fixture to create a DeviceConfigDialog instance.""" - add_dialog = DeviceConfigDialog( + add_dialog = DirectUpdateDeviceConfigDialog( device=None, config_helper=MagicMock(), client=mock_client, action="add" ) qtbot.addWidget(add_dialog)