From ab38ef9067b792e427b012f1df82ef02c00edda0 Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 21 Oct 2024 16:34:21 -0700 Subject: [PATCH 01/20] use the motor signal movement_finished() to update the Axis control displays --- bapsf_motion/gui/configure/motion_group_widget.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index 5833d7f7..6c20a38c 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -255,6 +255,7 @@ def link_axis(self, mg: MotionGroup, ax_index: int): self.axis_name_label.setText(self.axis.name) self.axis.motor.status_changed.connect(self._update_display_of_axis_status) + self.axis.motor.movement_finished.connect(self._update_display_of_axis_status) self._update_display_of_axis_status() self.axisLinked.emit() @@ -263,6 +264,9 @@ def unlink_axis(self): if self.axis is not None: # self.axis.terminate(delay_loop_stop=True) self.axis.motor.status_changed.disconnect(self._update_display_of_axis_status) + self.axis.motor.movement_finished.disconnect( + self._update_display_of_axis_status + ) self._mg = None self._axis_index = None From 1151aa180449be7f83eebe749a80dd4d3808c676 Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 21 Oct 2024 17:31:04 -0700 Subject: [PATCH 02/20] disable configuration controls while the probe drive is moving --- .../gui/configure/motion_group_widget.py | 63 ++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index 6c20a38c..14c92118 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -39,6 +39,8 @@ class AxisControlWidget(QWidget): axisLinked = Signal() axisUnlinked = Signal() + movementStarted = Signal(int) + movementStopped = Signal(int) def __init__(self, parent=None): super().__init__(parent) @@ -255,6 +257,8 @@ def link_axis(self, mg: MotionGroup, ax_index: int): self.axis_name_label.setText(self.axis.name) self.axis.motor.status_changed.connect(self._update_display_of_axis_status) + self.axis.motor.movement_started.connect(self._emit_movement_started) + self.axis.motor.movement_finished.connect(self._emit_movement_finished) self.axis.motor.movement_finished.connect(self._update_display_of_axis_status) self._update_display_of_axis_status() @@ -264,6 +268,8 @@ def unlink_axis(self): if self.axis is not None: # self.axis.terminate(delay_loop_stop=True) self.axis.motor.status_changed.disconnect(self._update_display_of_axis_status) + self.axis.motor.movement_started.connect(self._emit_movement_started) + self.axis.motor.movement_finished.connect(self._emit_movement_finished) self.axis.motor.movement_finished.disconnect( self._update_display_of_axis_status ) @@ -272,12 +278,29 @@ def unlink_axis(self): self._axis_index = None self.axisUnlinked.emit() + def _emit_movement_started(self): + self.movementStarted.emit(self.axis_index) + + def _emit_movement_finished(self): + self.movementStopped.emit(self.axis_index) + def closeEvent(self, event): self.logger.info("Closing AxisControlWidget") + + if isinstance(self.axis, Axis): + self.axis.motor.status_changed.disconnect(self._update_display_of_axis_status) + self.axis.motor.movement_started.connect(self._emit_movement_started) + self.axis.motor.movement_finished.connect(self._emit_movement_finished) + self.axis.motor.movement_finished.disconnect( + self._update_display_of_axis_status + ) + event.accept() class DriveControlWidget(QWidget): + movementStarted = Signal() + movementStopped = Signal() def __init__(self, parent=None): super().__init__(parent) @@ -478,6 +501,8 @@ def link_motion_group(self, mg): for ii, ax in enumerate(self.mg.drive.axes): acw = self._axis_control_widgets[ii] acw.link_axis(self.mg, ii) + acw.movementStarted.connect(self._drive_movement_started) + acw.movementStopped.connect(self._drive_movement_finished) acw.show() self.setEnabled(not self._mg.terminated) @@ -493,6 +518,20 @@ def unlink_motion_group(self): self._mg = None self.setEnabled(False) + @Slot(int) + def _drive_movement_started(self, axis_index): + self.movementStarted.emit() + + @Slot(int) + def _drive_movement_finished(self, axis_index): + if not isinstance(self.mg, MotionGroup) or not isinstance(self.mg.drive, Drive): + return + + is_moving = [ax.is_moving for ax in self.mg.drive.axes] + is_moving[axis_index] = False + if not any(is_moving): + self.movementStopped.emit() + def closeEvent(self, event): self.logger.info("Closing DriveControlWidget") event.accept() @@ -535,7 +574,6 @@ def __init__( "ips": deployed_ips, } - self._logger = gui_logger self._mg = None @@ -755,6 +793,9 @@ def _connect_signals(self): self._transform_dropdown_new_selection ) + self.drive_control_widget.movementStarted.connect(self.disable_config_controls) + self.drive_control_widget.movementStopped.connect(self.enable_config_controls) + self.done_btn.clicked.connect(self.return_and_close) self.discard_btn.clicked.connect(self.close) @@ -1628,6 +1669,26 @@ def _transform_dropdown_new_selection(self, index): self._change_transform(tr_default_config) + def disable_config_controls(self): + self.drive_dropdown.setEnabled(False) + self.drive_btn.setEnabled(False) + + self.mb_dropdown.setEnabled(False) + self.mb_btn.setEnabled(False) + + self.transform_dropdown.setEnabled(False) + self.transform_btn.setEnabled(False) + + def enable_config_controls(self): + self.drive_dropdown.setEnabled(True) + self.drive_btn.setEnabled(True) + + self.mb_dropdown.setEnabled(True) + self.mb_btn.setEnabled(True) + + self.transform_dropdown.setEnabled(True) + self.transform_btn.setEnabled(True) + def return_and_close(self): config = _deepcopy_dict(self.mg.config) index = -1 if self._mg_index is None else self._mg_index From 4f2599fed891f9cc686586023f5bd39c2d065997 Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 21 Oct 2024 18:05:07 -0700 Subject: [PATCH 03/20] if the status of one axis changes make the other AxisControlWidgets update --- .../gui/configure/motion_group_widget.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index 14c92118..44089314 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -2,6 +2,7 @@ import asyncio import logging +import warnings from PySide6.QtCore import Qt, Signal, Slot, QSize from PySide6.QtGui import QDoubleValidator @@ -41,6 +42,7 @@ class AxisControlWidget(QWidget): axisUnlinked = Signal() movementStarted = Signal(int) movementStopped = Signal(int) + axisStatusChanged = Signal() def __init__(self, parent=None): super().__init__(parent) @@ -257,6 +259,7 @@ def link_axis(self, mg: MotionGroup, ax_index: int): self.axis_name_label.setText(self.axis.name) self.axis.motor.status_changed.connect(self._update_display_of_axis_status) + self.axis.motor.status_changed.connect(self.axisStatusChanged.emit) self.axis.motor.movement_started.connect(self._emit_movement_started) self.axis.motor.movement_finished.connect(self._emit_movement_finished) self.axis.motor.movement_finished.connect(self._update_display_of_axis_status) @@ -268,6 +271,7 @@ def unlink_axis(self): if self.axis is not None: # self.axis.terminate(delay_loop_stop=True) self.axis.motor.status_changed.disconnect(self._update_display_of_axis_status) + self.axis.motor.status_changed.connect(self.axisStatusChanged.emit) self.axis.motor.movement_started.connect(self._emit_movement_started) self.axis.motor.movement_finished.connect(self._emit_movement_finished) self.axis.motor.movement_finished.disconnect( @@ -289,6 +293,7 @@ def closeEvent(self, event): if isinstance(self.axis, Axis): self.axis.motor.status_changed.disconnect(self._update_display_of_axis_status) + self.axis.motor.status_changed.disconnect(self.axisStatusChanged.emit) self.axis.motor.movement_started.connect(self._emit_movement_started) self.axis.motor.movement_finished.connect(self._emit_movement_finished) self.axis.motor.movement_finished.disconnect( @@ -503,6 +508,7 @@ def link_motion_group(self, mg): acw.link_axis(self.mg, ii) acw.movementStarted.connect(self._drive_movement_started) acw.movementStopped.connect(self._drive_movement_finished) + acw.axisStatusChanged.connect(self._update_all_axis_displays) acw.show() self.setEnabled(not self._mg.terminated) @@ -512,12 +518,28 @@ def unlink_motion_group(self): visible = True if ii == 0 else False acw.unlink_axis() + + with warnings.catch_warnings(): + warnings.simplefilter("ignore", category=RuntimeWarning) + acw.movementStarted.disconnect(self._drive_movement_started) + acw.movementStopped.disconnect(self._drive_movement_finished) + acw.axisStatusChanged.disconnect(self._update_all_axis_displays) + acw.setVisible(visible) # self.mg.terminate(delay_loop_stop=True) self._mg = None self.setEnabled(False) + def _update_all_axis_displays(self): + for acw in self._axis_control_widgets: + if acw.isHidden(): + continue + elif acw.axis.is_moving: + continue + + acw._update_display_of_axis_status() + @Slot(int) def _drive_movement_started(self, axis_index): self.movementStarted.emit() From cddf83b2a18b750163cf568550b4c19d9f08e77c Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 21 Oct 2024 18:29:51 -0700 Subject: [PATCH 04/20] make the drive widget update its display when the configuration is changed --- bapsf_motion/gui/configure/motion_group_widget.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index 44089314..f269b879 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -804,6 +804,7 @@ def _connect_signals(self): self.configChanged.connect(self._update_drive_dropdown) self.configChanged.connect(self._update_mb_dropdown) self.configChanged.connect(self._update_transform_dropdown) + self.configChanged.connect(self._update_drive_control_widget) self.drive_dropdown.currentIndexChanged.connect( self._drive_dropdown_new_selection @@ -1421,6 +1422,12 @@ def _update_toml_widget(self): def _update_mg_name_widget(self): self.mg_name_widget.setText(self.mg_config["name"]) + def _update_drive_control_widget(self): + if not self.drive_control_widget.isEnabled(): + return + + self.drive_control_widget._update_all_axis_displays() + def _rename_motion_group(self): self.logger.info("Renaming motion group") self.mg.config["name"] = self.mg_name_widget.text() From d2bba71de7dec58007941ca0d2d3f4e46fe25411 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 13:20:21 -0800 Subject: [PATCH 05/20] fix over indent --- bapsf_motion/gui/configure/motion_group_widget.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index f269b879..708cbd99 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -746,8 +746,8 @@ def __init__( # if MGWidget launched without a drive then use a default # drive (if defined) if ( - "drive" not in self.mg_config - and self.drive_defaults[0][0] != "Custom Drive" + "drive" not in self.mg_config + and self.drive_defaults[0][0] != "Custom Drive" ): self._mg_config["drive"] = _deepcopy_dict(self.drive_defaults[0][1]) From 6caafb387376c7bd1aaae99d0f0791f5642ff925 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 13:21:44 -0800 Subject: [PATCH 06/20] move all connected functions to MGWidget.configChanged into the _config_changed_handler() method --- .../gui/configure/motion_group_widget.py | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index 708cbd99..a7dcec6c 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -798,13 +798,7 @@ def _connect_signals(self): self.mg_name_widget.editingFinished.connect(self._rename_motion_group) - self.configChanged.connect(self._update_toml_widget) - self.configChanged.connect(self._update_mg_name_widget) - self.configChanged.connect(self._validate_motion_group) - self.configChanged.connect(self._update_drive_dropdown) - self.configChanged.connect(self._update_mb_dropdown) - self.configChanged.connect(self._update_transform_dropdown) - self.configChanged.connect(self._update_drive_control_widget) + self.configChanged.connect(self._config_changed_handler) self.drive_dropdown.currentIndexChanged.connect( self._drive_dropdown_new_selection @@ -1055,6 +1049,22 @@ def _build_transform_defaults(self): return self._transform_defaults + def _config_changed_handler(self): + # Note: none of the methods executed here should cause a + # configChanged event + self._validate_motion_group() + + # now update displays + self._update_mg_name_widget() + self._update_toml_widget() + self._update_drive_dropdown() + self._update_mb_dropdown() + self._update_transform_dropdown() + + # updating the drive control widget should always be the last + # step + self._update_drive_control_widget() + def _populate_drive_dropdown(self): for item in self.drive_defaults: self.drive_dropdown.addItem(item[0]) From b1868866d77a88fccae2fd8428cb2dfd3e79e421 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 13:22:46 -0800 Subject: [PATCH 07/20] when _update_drive_control_widget() execute _refresh_drive_control() if the drive control widget does not have the valid MG --- bapsf_motion/gui/configure/motion_group_widget.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index a7dcec6c..6d1886e7 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -1436,7 +1436,10 @@ def _update_drive_control_widget(self): if not self.drive_control_widget.isEnabled(): return - self.drive_control_widget._update_all_axis_displays() + if self.drive_control_widget.mg is None: + self._refresh_drive_control() + else: + self.drive_control_widget._update_all_axis_displays() def _rename_motion_group(self): self.logger.info("Renaming motion group") From 0b3ba895d334f16d3a7bcd1f8ea56b88fb57ada8 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 14:36:20 -0800 Subject: [PATCH 08/20] change default colors to be more color-blind friendly --- bapsf_motion/gui/widgets/buttons.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bapsf_motion/gui/widgets/buttons.py b/bapsf_motion/gui/widgets/buttons.py index 47634e92..2d0dfec0 100644 --- a/bapsf_motion/gui/widgets/buttons.py +++ b/bapsf_motion/gui/widgets/buttons.py @@ -203,8 +203,10 @@ def __init__(self, color: str = "#2980b9", parent=None): class GearValidButton(StyleButton): def __init__(self, parent=None): - self._valid_color = "#499C54" # rgb(14, 212, 0) - self._invalid_color = "#C75450" # rgb(13, 88, 0) + # self._valid_color = "#499C54" # rgb(14, 212, 0) + # self._invalid_color = "#C75450" # rgb(13, 88, 0) + self._valid_color = "#3498DB" # rgb(52, 152, 219) blue + self._invalid_color = "#FF5733" # rgb(242, 94, 62) orange self._valid_icon = qta.icon("fa.gear", color=self._valid_color) self._invalid_icon = qta.icon("fa.gear", color=self._invalid_color) From ed2a9e313b195c560e61e267f6070c8b5bd29a1a Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 15:10:55 -0800 Subject: [PATCH 09/20] Make sure MGWidget is seeded with a mg name if none is given, and the tooltip is accurately produced --- bapsf_motion/gui/configure/motion_group_widget.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index 6d1886e7..429bd817 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -788,6 +788,11 @@ def __init__( self._initial_mg_config = _deepcopy_dict(self._mg_config) + if "name" not in self._mg_config or self._mg_config["name"] == "": + self._mg_config["name"] = "A New MG" + self.logger.info(f"starting _mg_config: {self._mg_config}") + self._update_mg_name_widget() + self._spawn_motion_group() self._refresh_drive_control() @@ -1333,7 +1338,6 @@ def mg_config(self) -> Union[Dict[str, Any], "MotionGroupConfig"]: return self._mg_config elif self._mg_config is None: name = self.mg_name_widget.text() - name = "A New MG" if name == "" else name self._mg_config = {"name": name} return self._mg_config @@ -1529,8 +1533,8 @@ def _validate_motion_group(self): self.done_btn.setEnabled(False) def _validate_motion_group_name(self) -> bool: - self.logger.info("Validating motion group name") mg_name = self.mg_name_widget.text() + self.logger.info(f"Validating motion group name '{mg_name}'.") # clear previous tooltips and actions self.mg_name_widget.setToolTip("") From 66886a2fbfeb14ae8976ad379d6535d95b46d72f Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 15:44:48 -0800 Subject: [PATCH 10/20] add tooltips for the MotionBuilder gear btn --- bapsf_motion/gui/configure/motion_group_widget.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index 429bd817..0425e63c 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -1505,12 +1505,17 @@ def _validate_motion_group(self): if not isinstance(self.mg.mb, MotionBuilder): self.mb_btn.set_invalid() + self.mb_btn.setToolTip("Motion space needs to be defined.") self.done_btn.setEnabled(False) else: if "layer" not in self.mg.mb.config: self.mb_btn.set_invalid() + self.mb_btn.setToolTip( + "A point layer needs to be defined to generate a motion list." + ) else: self.mb_btn.set_valid() + self.mb_btn.setToolTip("") if not isinstance(self.mg.transform, BaseTransform): self.transform_btn.set_invalid() @@ -1574,6 +1579,7 @@ def _validate_drive(self) -> bool: self.mb_dropdown.setEnabled(False) self.mb_btn.setEnabled(False) self.mb_btn.set_invalid() + self.mb_btn.setToolTip("Motion space needs to be defined.") self.transform_dropdown.setEnabled(False) self.transform_btn.setEnabled(False) From 2ee22fd0242c65129f25474620fce5c34750bbdb Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 15:56:37 -0800 Subject: [PATCH 11/20] do not false invalidate drive if we are editing an existing motion group --- bapsf_motion/gui/configure/motion_group_widget.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index 0425e63c..29542e5a 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -586,6 +586,10 @@ def __init__( deployed_ips = [] if isinstance(self._parent.rm, RunManager): for mg in self._parent.rm.mgs.values(): + if dict_equal(mg_config, mg.config): + # assume we are editing an existing motion group + continue + deployed_mg_names.append(mg.config["name"]) for axis in mg.drive.axes: From 174a3108cd61714499d3471f99b011dca835b381 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 18:15:51 -0800 Subject: [PATCH 12/20] only compare dictionaries if mg_config is not None --- bapsf_motion/gui/configure/motion_group_widget.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bapsf_motion/gui/configure/motion_group_widget.py b/bapsf_motion/gui/configure/motion_group_widget.py index 29542e5a..4e190b4c 100644 --- a/bapsf_motion/gui/configure/motion_group_widget.py +++ b/bapsf_motion/gui/configure/motion_group_widget.py @@ -586,7 +586,7 @@ def __init__( deployed_ips = [] if isinstance(self._parent.rm, RunManager): for mg in self._parent.rm.mgs.values(): - if dict_equal(mg_config, mg.config): + if mg_config is not None and dict_equal(mg_config, mg.config): # assume we are editing an existing motion group continue From 533b5d08f3536b4658870a9a189a9d26e4e159f3 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 18:17:17 -0800 Subject: [PATCH 13/20] move all connected functionality to signal configChanged into method _config_changed_handler() --- .../gui/configure/motion_builder_overlay.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/bapsf_motion/gui/configure/motion_builder_overlay.py b/bapsf_motion/gui/configure/motion_builder_overlay.py index e556bf49..2bf33f48 100644 --- a/bapsf_motion/gui/configure/motion_builder_overlay.py +++ b/bapsf_motion/gui/configure/motion_builder_overlay.py @@ -124,10 +124,7 @@ def __init__(self, mg: MotionGroup, parent: "mgw.MGWidget" = None): def _connect_signals(self): super()._connect_signals() - self.configChanged.connect(self.update_canvas) - self.configChanged.connect(self.update_exclusion_list_box) - self.configChanged.connect(self.update_layer_list_box) - self.configChanged.connect(self._validate_mb) + self.configChanged.connect(self._config_changed_handler) self.add_ex_btn.clicked.connect(self._exclusion_configure_new) self.remove_ex_btn.clicked.connect(self._exclusion_remove_from_mb) @@ -612,6 +609,16 @@ def _define_params_field_widget(self, ex_or_ly, _type): # -- WIDGET INTERACTION FUNCTIONALITY -- + def _config_changed_handler(self): + # Note: none of the methods executed here should cause a + # configChanged event + self._validate_mb() + + # now update displays + self.update_exclusion_list_box() + self.update_layer_list_box() + self.update_canvas() + def _exclusion_configure_new(self): if not self._params_widget.isHidden(): self._hide_and_clear_params_widget() From dd8a794207e21ed7a42fbbabbcf1d4f4bae1f714 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 18:50:25 -0800 Subject: [PATCH 14/20] GovernExclusions must regenerate their mask during a global mask update --- bapsf_motion/motion_builder/exclusions/base.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bapsf_motion/motion_builder/exclusions/base.py b/bapsf_motion/motion_builder/exclusions/base.py index fdefc42f..51aad9d0 100644 --- a/bapsf_motion/motion_builder/exclusions/base.py +++ b/bapsf_motion/motion_builder/exclusions/base.py @@ -230,4 +230,7 @@ def update_global_mask(self): f"the exclusion can not be merged into the global maks." ) + # Since GovernExclusion use the existing mask to generate its own + # mask, the exclusion must be regenerated during every global update + self.regenerate_exclusion() self.mask[...] = self.exclusion[...] From f4a79129e34f09e5a58a34d81517453a844910e7 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 18:51:35 -0800 Subject: [PATCH 15/20] remove unused import --- bapsf_motion/motion_builder/exclusions/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bapsf_motion/motion_builder/exclusions/base.py b/bapsf_motion/motion_builder/exclusions/base.py index 51aad9d0..c752dd49 100644 --- a/bapsf_motion/motion_builder/exclusions/base.py +++ b/bapsf_motion/motion_builder/exclusions/base.py @@ -6,7 +6,7 @@ import xarray as xr from abc import ABC, abstractmethod -from typing import Any, Dict, List, Union +from typing import Any, Dict, Union from bapsf_motion.motion_builder.item import MBItem From 92794896c1e737049f80af1abf2e1e572a78dd04 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 18:54:31 -0800 Subject: [PATCH 16/20] make sure we do not index self.exclusions if no exclusion have been added to the list yet --- bapsf_motion/motion_builder/core.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bapsf_motion/motion_builder/core.py b/bapsf_motion/motion_builder/core.py index 04ed79d5..44cca0ea 100644 --- a/bapsf_motion/motion_builder/core.py +++ b/bapsf_motion/motion_builder/core.py @@ -279,7 +279,10 @@ def add_exclusion(self, ex_type: str, **settings): if not isinstance(exclusion, GovernExclusion): self._exclusions.append(exclusion) - elif not isinstance(self.exclusions[0], GovernExclusion): + elif ( + len(self.exclusions) == 0 + or not isinstance(self.exclusions[0], GovernExclusion) + ): self._exclusions.insert(0, exclusion) else: warnings.warn( From 24aa199a48fe1857be3cc3f093cde5fb8fe1db80 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 18:57:26 -0800 Subject: [PATCH 17/20] make MBItem._determine_name() an abstract method to children can better handle their naming --- bapsf_motion/motion_builder/core.py | 3 ++ .../motion_builder/exclusions/base.py | 36 ++++++++++++------- bapsf_motion/motion_builder/item.py | 20 +++-------- bapsf_motion/motion_builder/layers/base.py | 26 ++++++++++++-- 4 files changed, 55 insertions(+), 30 deletions(-) diff --git a/bapsf_motion/motion_builder/core.py b/bapsf_motion/motion_builder/core.py index 44cca0ea..b31471c8 100644 --- a/bapsf_motion/motion_builder/core.py +++ b/bapsf_motion/motion_builder/core.py @@ -192,6 +192,9 @@ def _build_initial_ds(self): return ds + def _determine_name(self): + return self.base_name + def add_layer(self, ly_type: str, **settings): """ Add a "point" layer to the motion builder. diff --git a/bapsf_motion/motion_builder/exclusions/base.py b/bapsf_motion/motion_builder/exclusions/base.py index c752dd49..2af0f009 100644 --- a/bapsf_motion/motion_builder/exclusions/base.py +++ b/bapsf_motion/motion_builder/exclusions/base.py @@ -1,6 +1,7 @@ """Module that defines the `BaseExclusion` abstract class.""" __all__ = ["BaseExclusion", "GovernExclusion"] +import ast import numpy as np import re import xarray as xr @@ -11,7 +12,7 @@ from bapsf_motion.motion_builder.item import MBItem -class BaseExclusion(ABC, MBItem): +class BaseExclusion(MBItem): """ Abstract base class for :term:`motion exclusion` classes. @@ -133,18 +134,6 @@ def inputs(self) -> Dict[str, Any]: """ return self._inputs - @MBItem.name.setter - def name(self, name: str): - if not self.skip_ds_add: - # The exclusion name is a part of the Dataset management, - # so we can NOT/ should NOT rename it - return - elif not isinstance(name, str): - return - - self._name = name - self._name_pattern = re.compile(rf"{name}(?P[0-9]+)") - @abstractmethod def _generate_exclusion(self) -> Union[np.ndarray, xr.DataArray]: """ @@ -161,6 +150,27 @@ def _validate_inputs(self) -> None: """ ... + def _determine_name(self): + try: + return self.name + except AttributeError: + # self._name has not been defined yet + pass + + names = set(self._ds.data_vars.keys()) + ids = [] + for name in names: + _match = self.name_pattern.fullmatch(name) + if _match is not None: + ids.append( + ast.literal_eval(_match.group("number")) + ) + + ids = list(set(ids)) + _id = 0 if not ids else ids[-1] + 1 + + return f"{self.base_name}{_id:d}" + def is_excluded(self, point): """ Check if ``point`` resides in an excluded region defined by diff --git a/bapsf_motion/motion_builder/item.py b/bapsf_motion/motion_builder/item.py index ee84e5ae..1d33edc2 100644 --- a/bapsf_motion/motion_builder/item.py +++ b/bapsf_motion/motion_builder/item.py @@ -8,6 +8,7 @@ import re import xarray as xr +from abc import ABC, abstractmethod from typing import Hashable, Tuple try: @@ -16,7 +17,7 @@ ErrorOptions = str -class MBItem: +class MBItem(ABC): r""" A base class for any :term:`motion builder` class that will interact with the `xarray` `~xarray.Dataset` containing the @@ -155,7 +156,8 @@ def _validate_ds(ds: xr.Dataset) -> xr.Dataset: return ds - def _determine_name(self): + @abstractmethod + def _determine_name(self) -> str: """ Determine the name for the motion builder item that will be used in the `~xarray.Dataset`. This is generally the name of the @@ -165,19 +167,7 @@ def _determine_name(self): :attr:`name_pattern` and generate a unique :attr:`name` for the motion builder item. """ - try: - return self.name - except AttributeError: - # self._name has not been defined yet - pass - - names = set(self._ds.data_vars.keys()) - n_existing = 0 - for name in names: - if self.name_pattern.fullmatch(name) is not None: - n_existing += 1 - - return f"{self.base_name}{n_existing + 1:d}" + ... def drop_vars(self, names: str, *, errors: ErrorOptions = "raise"): new_ds = self._ds.drop_vars(names, errors=errors) diff --git a/bapsf_motion/motion_builder/layers/base.py b/bapsf_motion/motion_builder/layers/base.py index 5496c6b8..78acfe1d 100644 --- a/bapsf_motion/motion_builder/layers/base.py +++ b/bapsf_motion/motion_builder/layers/base.py @@ -1,17 +1,18 @@ """Module that defines the `BaseLayer` abstract class.""" __all__ = ["BaseLayer"] +import ast import re import numpy as np import xarray as xr -from abc import ABC, abstractmethod +from abc import abstractmethod from typing import Any, Dict, List, Union from bapsf_motion.motion_builder.item import MBItem -class BaseLayer(ABC, MBItem): +class BaseLayer(MBItem): """ Abstract base class for :term:`motion layer` classes. @@ -142,6 +143,27 @@ def _validate_inputs(self) -> None: """ ... + def _determine_name(self): + try: + return self.name + except AttributeError: + # self._name has not been defined yet + pass + + names = set(self._ds.data_vars.keys()) + ids = [] + for name in names: + _match = self.name_pattern.fullmatch(name) + if _match is not None: + ids.append( + ast.literal_eval(_match.group("number")) + ) + + ids = list(set(ids)) + _id = 0 if not ids else ids[-1] + 1 + + return f"{self.base_name}{_id:d}" + def _generate_point_matrix_da(self): """ Generate the :term:`motion layer` array/matrix and add it to From 87d0c2e4628ed00ba13b6823bb0fd9e7f9f38496 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 18:58:02 -0800 Subject: [PATCH 18/20] simplify plotting of global mask --- bapsf_motion/gui/configure/motion_builder_overlay.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/bapsf_motion/gui/configure/motion_builder_overlay.py b/bapsf_motion/gui/configure/motion_builder_overlay.py index 2bf33f48..9f34454b 100644 --- a/bapsf_motion/gui/configure/motion_builder_overlay.py +++ b/bapsf_motion/gui/configure/motion_builder_overlay.py @@ -976,12 +976,9 @@ def update_canvas(self): self.logger.info(f"MB config = {self.mb.config}") self.mpl_canvas.figure.clear() - ax = self.mpl_canvas.figure.add_subplot(111) - self.mb.mask.plot( - x=self.mb.mask.dims[0], - y=self.mb.mask.dims[1], - ax=ax, - ) + ax = self.mpl_canvas.figure.gca() + xdim, ydim = self.mb.mspace_dims + self.mb.mask.plot(x=xdim, y=ydim, ax=ax) pts = self.mb.motion_list if pts is not None: From 0ea8507291c3b56551df93618fb5e6a30ca4bafd Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 19:33:43 -0800 Subject: [PATCH 19/20] update Shadow2DExclusion._paint_mask to handle triangles where all point are on a line --- bapsf_motion/motion_builder/exclusions/shadow.py | 10 ++++++++++ docs/notebooks/motion_list/Shadow2DExclusion.ipynb | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/bapsf_motion/motion_builder/exclusions/shadow.py b/bapsf_motion/motion_builder/exclusions/shadow.py index 881fc39c..c56cfee5 100644 --- a/bapsf_motion/motion_builder/exclusions/shadow.py +++ b/bapsf_motion/motion_builder/exclusions/shadow.py @@ -654,6 +654,16 @@ def _paint_mask(self, rays: np.ndarray) -> xr.DataArray: triangles[:, 2, :] - triangles[:, 0, :], triangles[:, 1, :] - triangles[:, 0, :] ) + + zero_mask = denominator == 0 + if np.any(zero_mask): + # denominator can be zero if all points on the triangle lie + # on a line + not_zero_mask = np.logical_not(zero_mask) + triangles = triangles[not_zero_mask, ...] + numerator = numerator[..., not_zero_mask] + denominator = denominator[not_zero_mask] + lambda_3 = numerator / denominator[None, None, ...] # calculate lambda_2 diff --git a/docs/notebooks/motion_list/Shadow2DExclusion.ipynb b/docs/notebooks/motion_list/Shadow2DExclusion.ipynb index 63f53161..b68d458d 100644 --- a/docs/notebooks/motion_list/Shadow2DExclusion.ipynb +++ b/docs/notebooks/motion_list/Shadow2DExclusion.ipynb @@ -955,6 +955,16 @@ " triangles[:, 2, :] - triangles[:, 0, :],\n", " triangles[:, 1, :] - triangles[:, 0, :]\n", " )\n", + " \n", + " zero_mask = denominator == 0\n", + " if np.any(zero_mask):\n", + " # denominator can be zero if all points on the triangle lie\n", + " # on a line\n", + " not_zero_mask = np.logical_not(zero_mask)\n", + " triangles = triangles[not_zero_mask, ...]\n", + " numerator = numerator[..., not_zero_mask]\n", + " denominator = denominator[not_zero_mask]\n", + "\n", " lambda_3 = numerator / denominator[None, None, ...]\n", "\n", " # calculate lambda_2\n", From 8d7d899473cd66e200151712ccebd630d4474e1b Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jan 2025 20:00:39 -0800 Subject: [PATCH 20/20] remove govern exclusion from dropdown if one is already defined --- bapsf_motion/gui/configure/motion_builder_overlay.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/bapsf_motion/gui/configure/motion_builder_overlay.py b/bapsf_motion/gui/configure/motion_builder_overlay.py index 9f34454b..cbde15f3 100644 --- a/bapsf_motion/gui/configure/motion_builder_overlay.py +++ b/bapsf_motion/gui/configure/motion_builder_overlay.py @@ -35,7 +35,7 @@ ) from bapsf_motion.motion_builder import MotionBuilder from bapsf_motion.motion_builder.layers import layer_registry -from bapsf_motion.motion_builder.exclusions import exclusion_registry +from bapsf_motion.motion_builder.exclusions import exclusion_registry, GovernExclusion from bapsf_motion.utils import _deepcopy_dict from bapsf_motion.utils import units as u @@ -628,6 +628,13 @@ def _exclusion_configure_new(self): _available = self.exclusion_registry.get_names_by_dimensionality( self.dimensionality ) + if self.mb.exclusions and isinstance(self.mb.exclusions[0], GovernExclusion): + # remove govern exclusion since we can only have one defined + for name in tuple(_available): + ex = self.exclusion_registry.get_exclusion(name) + if issubclass(ex, GovernExclusion): + _available.remove(name) + self._refresh_params_combo_box(_available) self.params_combo_box.setObjectName("exclusion")