Conversation
There was a problem hiding this comment.
Pull request overview
Fixes stability issues in the LMFit dialog (notably around “C++ object deleted” scenarios) and improves the compact/horizontal UI variant so it reliably supports the action column.
Changes:
- Guarded action-button enable/disable logic against deleted Qt objects (using
shiboken6.isValid) and pruned invalid button references. - Improved state handling when removing the currently displayed curve (clears relevant UI and action state).
- Updated
lmfit_dialog_compact.uito use a horizontal splitter layout and ensured the parameter tree includes the “Action” column; added a regression test for the compact UI.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
bec_widgets/widgets/dap/lmfit_dialog/lmfit_dialog.py |
Adds validity checks for action buttons and improves cleanup when removing displayed curve data. |
bec_widgets/widgets/dap/lmfit_dialog/lmfit_dialog_compact.ui |
Adjusts compact layout (horizontal splitter, margins/sizing) and ensures 4-column param tree with “Action”. |
tests/unit_tests/test_lmfit_dialog.py |
Adds a compact-UI regression test covering curve selection hiding and action button creation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if self.fit_curve_id == curve_id: | ||
| self._fit_curve_id = None | ||
| self.action_buttons = {} | ||
| self.ui.summary_tree.clear() | ||
| self.ui.param_tree.clear() |
There was a problem hiding this comment.
remove_dap_data() sets self._fit_curve_id = None, but fit_curve_id is annotated as returning str and selected_fit is Signal(str). Consider making fit_curve_id explicitly optional (str | None) and/or providing a clear/consistent way to signal that the selection was cleared (e.g., select the next available curve and emit it, or emit a dedicated “cleared” signal) to avoid type/behavior inconsistencies for consumers.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Description
Fix cpp object deleted bug and optimized horizontal layout variant. Needed for #1105
Related Issues