From 4f28cf6c8ebcf3584c3c981163dacbf8faf422e4 Mon Sep 17 00:00:00 2001 From: ashmeigh Date: Wed, 29 Jan 2025 20:01:24 +0000 Subject: [PATCH 01/11] Fix ROI movement not updating spectrum unless selected --- .../next/fix-2486-spec-plot-update | 1 + .../gui/windows/spectrum_viewer/presenter.py | 18 ++++++++---------- .../windows/spectrum_viewer/spectrum_widget.py | 4 ++-- .../gui/windows/spectrum_viewer/view.py | 2 +- 4 files changed, 12 insertions(+), 13 deletions(-) create mode 100644 docs/release_notes/next/fix-2486-spec-plot-update diff --git a/docs/release_notes/next/fix-2486-spec-plot-update b/docs/release_notes/next/fix-2486-spec-plot-update new file mode 100644 index 00000000000..5e166491181 --- /dev/null +++ b/docs/release_notes/next/fix-2486-spec-plot-update @@ -0,0 +1 @@ +2486: Fix ROI movement not updating spectrum unless selected diff --git a/mantidimaging/gui/windows/spectrum_viewer/presenter.py b/mantidimaging/gui/windows/spectrum_viewer/presenter.py index cccc8d1a574..0648a900db0 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/presenter.py +++ b/mantidimaging/gui/windows/spectrum_viewer/presenter.py @@ -201,19 +201,17 @@ def handle_range_slide_moved(self, tof_range: tuple[float, float] | tuple[int, i self.view.spectrum_widget.spectrum_plot_widget.set_tof_range_label(*self.model.tof_plot_range) self.update_displayed_image(autoLevels=False) - def handle_roi_moved(self, force_new_spectrums: bool = False) -> None: + def handle_roi_moved(self, roi: SpectrumROI) -> None: """ Handle changes to any ROI position and size. """ - for name in self.view.spectrum_widget.roi_dict: - current_roi = self.view.spectrum_widget.get_roi(name) - if force_new_spectrums: - spectrum = self.model.get_spectrum( - current_roi, - self.spectrum_mode, - self.view.shuttercount_norm_enabled(), - ) - self.view.set_spectrum(name, spectrum) + spectrum = self.model.get_spectrum( + roi.as_sensible_roi(), + self.spectrum_mode, + self.view.shuttercount_norm_enabled(), + ) + self.view.set_spectrum(roi.name, spectrum) + self.view.spectrum_widget.spectrum.update() def handle_roi_clicked(self, roi: SpectrumROI) -> None: if not roi.name == ROI_RITS: diff --git a/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py b/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py index 4e4c93bb30c..c1a23b86513 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py +++ b/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py @@ -138,7 +138,7 @@ class SpectrumWidget(QWidget): range_changed = pyqtSignal(object) roi_clicked = pyqtSignal(object) - roi_changed = pyqtSignal() + roi_changed = pyqtSignal(object) roiColorChangeRequested = pyqtSignal(str, tuple) spectrum_plot_widget: SpectrumPlotWidget @@ -213,7 +213,7 @@ def add_roi(self, roi: SensibleROI, name: str) -> None: self.roi_dict[name] = roi_object.roi self.max_roi_size = roi_object.size() - self.roi_dict[name].sigRegionChangeFinished.connect(self.roi_changed.emit) + self.roi_dict[name].sigRegionChangeFinished.connect(lambda: self.roi_changed.emit(self.roi_dict[name])) self.roi_dict[name].sigClicked.connect(self.roi_clicked.emit) self.image.vb.addItem(self.roi_dict[name]) self.roi_dict[name].hoverPen = mkPen(self.roi_dict[name].colour, width=3) diff --git a/mantidimaging/gui/windows/spectrum_viewer/view.py b/mantidimaging/gui/windows/spectrum_viewer/view.py index 1780f557337..18f049f868d 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/view.py +++ b/mantidimaging/gui/windows/spectrum_viewer/view.py @@ -189,7 +189,7 @@ def __init__(self, main_window: MainWindowView): self.spectrum.range_changed.connect(self.presenter.handle_range_slide_moved) self.spectrum_widget.roi_clicked.connect(self.presenter.handle_roi_clicked) - self.spectrum_widget.roi_changed.connect(self.presenter.handle_roi_moved) + self.spectrum_widget.roi_changed.connect(lambda roi: self.presenter.handle_roi_moved(roi)) self.spectrum_widget.roiColorChangeRequested.connect(self.presenter.change_roi_colour) self.spectrum_right_click_menu = self.spectrum.spectrum_viewbox.menu From d3f3ca0f5aba672bca9ba68a2570ffb4b0ef4a6c Mon Sep 17 00:00:00 2001 From: ashmeigh Date: Mon, 10 Feb 2025 15:04:23 +0000 Subject: [PATCH 02/11] Fix ROI movement updating spectrum --- .../gui/windows/spectrum_viewer/spectrum_widget.py | 9 +++++++-- mantidimaging/gui/windows/spectrum_viewer/view.py | 5 +++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py b/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py index c1a23b86513..b7d914cea6f 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py +++ b/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py @@ -211,9 +211,9 @@ def add_roi(self, roi: SensibleROI, name: str) -> None: roi_object.colour = self.colour_generator() roi_object.sig_colour_change.connect(lambda name, color: self.roiColorChangeRequested.emit(name, color)) - self.roi_dict[name] = roi_object.roi + self.roi_dict[name] = roi_object self.max_roi_size = roi_object.size() - self.roi_dict[name].sigRegionChangeFinished.connect(lambda: self.roi_changed.emit(self.roi_dict[name])) + self.roi_dict[name].sigRegionChangeFinished.connect(self._emit_roi_changed) self.roi_dict[name].sigClicked.connect(self.roi_clicked.emit) self.image.vb.addItem(self.roi_dict[name]) self.roi_dict[name].hoverPen = mkPen(self.roi_dict[name].colour, width=3) @@ -277,6 +277,11 @@ def rename_roi(self, old_name: str, new_name: str) -> None: self.spectrum_data_dict[new_name] = self.spectrum_data_dict.pop(old_name) self.roi_dict[new_name].rename_roi(new_name) + def _emit_roi_changed(self): + sender_roi = self.sender() + if sender_roi in self.roi_dict.values(): + self.roi_changed.emit(sender_roi) + class CustomViewBox(ViewBox): diff --git a/mantidimaging/gui/windows/spectrum_viewer/view.py b/mantidimaging/gui/windows/spectrum_viewer/view.py index 18f049f868d..ec97a657e03 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/view.py +++ b/mantidimaging/gui/windows/spectrum_viewer/view.py @@ -189,7 +189,7 @@ def __init__(self, main_window: MainWindowView): self.spectrum.range_changed.connect(self.presenter.handle_range_slide_moved) self.spectrum_widget.roi_clicked.connect(self.presenter.handle_roi_clicked) - self.spectrum_widget.roi_changed.connect(lambda roi: self.presenter.handle_roi_moved(roi)) + self.spectrum_widget.roi_changed.connect(self.presenter.handle_roi_moved) self.spectrum_widget.roiColorChangeRequested.connect(self.presenter.change_roi_colour) self.spectrum_right_click_menu = self.spectrum.spectrum_viewbox.menu @@ -541,12 +541,13 @@ def remove_roi(self) -> None: """ selected_row = self.roi_table_model.row_data(self.selected_row) roi_name = self.roi_table_model.get_element(self.selected_row, 0) + roi_object = self.spectrum_widget.roi_dict.pop(roi_name) if selected_row: self.roi_table_model.remove_row(self.selected_row) self.presenter.do_remove_roi(roi_name) self.spectrum_widget.spectrum_data_dict.pop(roi_name) self.spectrum_widget.spectrum.removeItem(roi_name) - self.presenter.handle_roi_moved() + self.presenter.handle_roi_moved(roi_object) self.selected_row = 0 self.tableView.selectRow(0) From 9473ea8a86e4425453affa600b43592995bea485 Mon Sep 17 00:00:00 2001 From: ashmeigh Date: Thu, 13 Feb 2025 12:22:26 +0000 Subject: [PATCH 03/11] Fix error in system test --- mantidimaging/gui/windows/spectrum_viewer/presenter.py | 4 +++- mantidimaging/gui/windows/spectrum_viewer/view.py | 7 ++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/mantidimaging/gui/windows/spectrum_viewer/presenter.py b/mantidimaging/gui/windows/spectrum_viewer/presenter.py index 0648a900db0..8b6cafc4a5e 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/presenter.py +++ b/mantidimaging/gui/windows/spectrum_viewer/presenter.py @@ -391,12 +391,14 @@ def do_remove_roi(self, roi_name: str | None = None) -> None: @param roi_name: Name of the ROI to remove """ if roi_name is None: - for name in self.get_roi_names(): + for name in list(self.get_roi_names()): self.view.spectrum_widget.remove_roi(name) self.view.spectrum_widget.roi_dict.clear() self.view.roi_table_model.clear_table() self.model.remove_all_roi() else: + if roi_name not in self.view.spectrum_widget.roi_dict: + return self.view.spectrum_widget.remove_roi(roi_name) def handle_export_tab_change(self, index: int) -> None: diff --git a/mantidimaging/gui/windows/spectrum_viewer/view.py b/mantidimaging/gui/windows/spectrum_viewer/view.py index ec97a657e03..e49b1379468 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/view.py +++ b/mantidimaging/gui/windows/spectrum_viewer/view.py @@ -541,16 +541,17 @@ def remove_roi(self) -> None: """ selected_row = self.roi_table_model.row_data(self.selected_row) roi_name = self.roi_table_model.get_element(self.selected_row, 0) - roi_object = self.spectrum_widget.roi_dict.pop(roi_name) + roi_object = self.spectrum_widget.roi_dict.pop(roi_name, None) + if roi_object is None: + return if selected_row: self.roi_table_model.remove_row(self.selected_row) self.presenter.do_remove_roi(roi_name) - self.spectrum_widget.spectrum_data_dict.pop(roi_name) + self.spectrum_widget.spectrum_data_dict.pop(roi_name, None) self.spectrum_widget.spectrum.removeItem(roi_name) self.presenter.handle_roi_moved(roi_object) self.selected_row = 0 self.tableView.selectRow(0) - if self.roi_table_model.rowCount() == 0: self.removeBtn.setEnabled(False) self.disable_roi_properties() From 85dd38dd80c4b3f86794b60aeab086bdecbad3e0 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Fri, 14 Feb 2025 17:46:29 +0000 Subject: [PATCH 04/11] Don't remove roi from dict yet Otherwise it does not get removed from the image in presenter.do_remove_roi() --- mantidimaging/gui/windows/spectrum_viewer/view.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/mantidimaging/gui/windows/spectrum_viewer/view.py b/mantidimaging/gui/windows/spectrum_viewer/view.py index e49b1379468..47beb63cbcf 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/view.py +++ b/mantidimaging/gui/windows/spectrum_viewer/view.py @@ -541,9 +541,7 @@ def remove_roi(self) -> None: """ selected_row = self.roi_table_model.row_data(self.selected_row) roi_name = self.roi_table_model.get_element(self.selected_row, 0) - roi_object = self.spectrum_widget.roi_dict.pop(roi_name, None) - if roi_object is None: - return + roi_object = self.spectrum_widget.roi_dict[roi_name] if selected_row: self.roi_table_model.remove_row(self.selected_row) self.presenter.do_remove_roi(roi_name) From ed0a65fe9a11c21b97dc727cf289655dd3d1063a Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Fri, 14 Feb 2025 17:48:39 +0000 Subject: [PATCH 05/11] Don't silently ignore missing dict entries If we hit these cases we probably want to know --- mantidimaging/gui/windows/spectrum_viewer/presenter.py | 2 -- mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py | 3 +-- mantidimaging/gui/windows/spectrum_viewer/view.py | 3 +-- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/mantidimaging/gui/windows/spectrum_viewer/presenter.py b/mantidimaging/gui/windows/spectrum_viewer/presenter.py index 8b6cafc4a5e..05f0227d881 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/presenter.py +++ b/mantidimaging/gui/windows/spectrum_viewer/presenter.py @@ -397,8 +397,6 @@ def do_remove_roi(self, roi_name: str | None = None) -> None: self.view.roi_table_model.clear_table() self.model.remove_all_roi() else: - if roi_name not in self.view.spectrum_widget.roi_dict: - return self.view.spectrum_widget.remove_roi(roi_name) def handle_export_tab_change(self, index: int) -> None: diff --git a/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py b/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py index b7d914cea6f..9aa0fc77986 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py +++ b/mantidimaging/gui/windows/spectrum_viewer/spectrum_widget.py @@ -279,8 +279,7 @@ def rename_roi(self, old_name: str, new_name: str) -> None: def _emit_roi_changed(self): sender_roi = self.sender() - if sender_roi in self.roi_dict.values(): - self.roi_changed.emit(sender_roi) + self.roi_changed.emit(sender_roi) class CustomViewBox(ViewBox): diff --git a/mantidimaging/gui/windows/spectrum_viewer/view.py b/mantidimaging/gui/windows/spectrum_viewer/view.py index 47beb63cbcf..044b12d4332 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/view.py +++ b/mantidimaging/gui/windows/spectrum_viewer/view.py @@ -545,8 +545,7 @@ def remove_roi(self) -> None: if selected_row: self.roi_table_model.remove_row(self.selected_row) self.presenter.do_remove_roi(roi_name) - self.spectrum_widget.spectrum_data_dict.pop(roi_name, None) - self.spectrum_widget.spectrum.removeItem(roi_name) + self.spectrum_widget.spectrum_data_dict.pop(roi_name) self.presenter.handle_roi_moved(roi_object) self.selected_row = 0 self.tableView.selectRow(0) From 6eadbd7367f9c93ed660862faf554f7c353079d5 Mon Sep 17 00:00:00 2001 From: JackEAllen Date: Mon, 10 Feb 2025 09:34:29 +0000 Subject: [PATCH 06/11] Update tableView to roiTableView Update tableView to roiTableView to improve clarity surrounding the purpose of the tableview as it is often confused with the roi table properties table --- mantidimaging/gui/ui/spectrum_viewer.ui | 2 +- .../gui/windows/spectrum_viewer/view.py | 25 ++++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/mantidimaging/gui/ui/spectrum_viewer.ui b/mantidimaging/gui/ui/spectrum_viewer.ui index f59ceaad1d1..14325b70c48 100644 --- a/mantidimaging/gui/ui/spectrum_viewer.ui +++ b/mantidimaging/gui/ui/spectrum_viewer.ui @@ -238,7 +238,7 @@ - + true diff --git a/mantidimaging/gui/windows/spectrum_viewer/view.py b/mantidimaging/gui/windows/spectrum_viewer/view.py index 044b12d4332..b36073ac755 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/view.py +++ b/mantidimaging/gui/windows/spectrum_viewer/view.py @@ -126,7 +126,7 @@ def as_roi(self): class SpectrumViewerWindowView(BaseMainWindowView): - tableView: RemovableRowTableView + roiTableView: RemovableRowTableView sampleStackSelector: DatasetSelectorWidgetView normaliseStackSelector: DatasetSelectorWidgetView @@ -237,10 +237,10 @@ def __init__(self, main_window: MainWindowView): self.exportButtonRITS.clicked.connect(self.presenter.handle_rits_export) # Point table - self.tableView.setSelectionBehavior(QAbstractItemView.SelectRows) - self.tableView.setSelectionMode(QAbstractItemView.SingleSelection) - self.tableView.setAlternatingRowColors(True) - self.tableView.clicked.connect(self.handle_table_click) + self.roiTableView.setSelectionBehavior(QAbstractItemView.SelectRows) + self.roiTableView.setSelectionMode(QAbstractItemView.SingleSelection) + self.roiTableView.setAlternatingRowColors(True) + self.roiTableView.clicked.connect(self.handle_table_click) # Roi Prop table self.roi_table_properties = ["Top", "Bottom", "Left", "Right"] @@ -270,7 +270,7 @@ def on_row_change(item: QModelIndex, _: Any) -> None: self.current_roi_name = self.roi_table_model.get_element(item.row(), 0) self.set_roi_properties() - self.tableView.selectionModel().currentRowChanged.connect(on_row_change) + self.roiTableView.selectionModel().currentRowChanged.connect(on_row_change) def on_data_in_table_change() -> None: """ @@ -298,7 +298,7 @@ def on_data_in_table_change() -> None: return self.roi_table_model.dataChanged.connect(on_data_in_table_change) - header = self.tableView.horizontalHeader() + header = self.roiTableView.horizontalHeader() header.setSectionResizeMode(0, QHeaderView.Stretch) header.setSectionResizeMode(1, QHeaderView.ResizeToContents) header.setSectionResizeMode(2, QHeaderView.ResizeToContents) @@ -351,10 +351,10 @@ def on_visibility_change(self) -> None: @property def roi_table_model(self) -> TableModel: - if self.tableView.model() is None: + if self.roiTableView.model() is None: mdl = TableModel() - self.tableView.setModel(mdl) - return self.tableView.model() + self.roiTableView.setModel(mdl) + return self.roiTableView.model() @property def current_dataset_id(self) -> UUID | None: @@ -530,7 +530,7 @@ def add_roi_table_row(self, name: str, colour: tuple[int, int, int]) -> None: """ self.roi_table_model.appendNewRow(name, colour, True) self.selected_row = self.roi_table_model.rowCount() - 1 - self.tableView.selectRow(self.selected_row) + self.roiTableView.selectRow(self.selected_row) self.current_roi_name = name self.removeBtn.setEnabled(True) self.set_old_table_names() @@ -548,7 +548,8 @@ def remove_roi(self) -> None: self.spectrum_widget.spectrum_data_dict.pop(roi_name) self.presenter.handle_roi_moved(roi_object) self.selected_row = 0 - self.tableView.selectRow(0) + self.roiTableView.selectRow(0) + if self.roi_table_model.rowCount() == 0: self.removeBtn.setEnabled(False) self.disable_roi_properties() From aea93ce9229e51e8b74e08521584630bdbc0b1e2 Mon Sep 17 00:00:00 2001 From: JackEAllen Date: Mon, 10 Feb 2025 13:03:17 +0000 Subject: [PATCH 07/11] Refactor roiTableView into ROITableWidget Class Refactor any logic and references to the roi table into a custom class to reduce code coupling in the spectrum viewer --- .../gui/windows/spectrum_viewer/view.py | 300 ++++++++++++------ 1 file changed, 211 insertions(+), 89 deletions(-) diff --git a/mantidimaging/gui/windows/spectrum_viewer/view.py b/mantidimaging/gui/windows/spectrum_viewer/view.py index b36073ac755..b84cf8edcb5 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/view.py +++ b/mantidimaging/gui/windows/spectrum_viewer/view.py @@ -125,6 +125,138 @@ def as_roi(self): return SensibleROI.from_list(new_points) +class ROITableWidget(RemovableRowTableView): + """ + A class to represent the ROI table widget in the spectrum viewer window. + """ + ElementType = str | tuple[int, int, int] | bool + RowType = list[ElementType] + old_table_names: list[str] + selected_row: int + last_clicked_roi: str + current_roi_name: str + + def __init__(self, parent=None): + super().__init__(parent) + if parent is not None: + layout = parent.layout() + if layout is not None: + layout.addWidget(self) + + self.old_table_names = [] + self.selected_row = 0 + self.last_clicked_roi = "" + self.current_roi_name = "" + + # Point table + self.setSelectionBehavior(QAbstractItemView.SelectRows) + self.setSelectionMode(QAbstractItemView.SingleSelection) + self.setAlternatingRowColors(True) + + # Initialise model + mdl = TableModel() + self.setModel(mdl) + self._roi_table_model = mdl + + # Configure up the table view + self.setVisible(True) + header = self.horizontalHeader() + header.setSectionResizeMode(0, QHeaderView.Stretch) + header.setSectionResizeMode(1, QHeaderView.ResizeToContents) + header.setSectionResizeMode(2, QHeaderView.ResizeToContents) + if self._roi_table_model.rowCount() > 0: + self.current_roi_name = self.last_clicked_roi = self._roi_table_model.roi_names()[0] + + @property + def roi_table_model(self) -> TableModel: + """The model for the ROI table""" + return self._roi_table_model + + def get_roi_names(self) -> list[str]: + return self._roi_table_model.roi_names() + + def get_row_data(self, row: int) -> RowType: + """ + Get the data for a specific row in the ROI table. + """ + name, data, visible = self._roi_table_model.row_data(row) + return [name, data, visible] + + def get_roi_name_by_row(self, row: int) -> str: + """ + Retrieve the name an ROI by its row index. + """ + return self._roi_table_model.get_element(row, 0) + + def get_roi_visibility_by_row(self, row: int) -> bool: + """ + Retrieve the visibility status of an ROI by its row index. + """ + return self._roi_table_model.get_element(row, 2) + + def row_count(self) -> int: + """ + Returns the number of rows in the ROI table model. + """ + return self._roi_table_model.rowCount() + + def find_row_for_roi(self, roi_name: str) -> int | None: + """ + Returns row index for ROI name, or None if not found. + """ + for row in range(self._roi_table_model.rowCount()): + if self._roi_table_model.index(row, 0).data() == roi_name: + return row + return None + + def set_roi_name_by_row(self, row: int, name: str) -> None: + """ + Set the name of the ROI for a given row in the ROI table. + """ + self._roi_table_model.set_element(row, 0, name) + + def set_old_table_names(self, old_table_names) -> None: + """ + Updates the list of old table names by removing specific entries if they exist. + """ + self.old_table_names = old_table_names + if 'all' in self.old_table_names: + self.old_table_names.remove('all') + if 'rits_roi' in self.old_table_names: + self.old_table_names.remove('rits_roi') + + def update_roi_color(self, roi_name: str, new_color: tuple[int, int, int]) -> None: + """ + Finds ROI by name in table and updates it's colour (R, G, B) format. + """ + row = self.find_row_for_roi(roi_name) + if row is not None: + self._roi_table_model.update_color(row, new_color) + + def add_row(self, name: str, colour: tuple[int, int, int], roi_names: list[str]) -> None: + """ + Add a new row to the ROI table + """ + self._roi_table_model.appendNewRow(name, colour, True) + self.selected_row = self._roi_table_model.rowCount() - 1 + self.selectRow(self.selected_row) + self.current_roi_name = name + self.set_old_table_names(roi_names) + + def remove_row(self, row: int) -> None: + """ + Remove a row from the ROI table + """ + self._roi_table_model.remove_row(row) + self.selectRow(0) + + def clear_table(self) -> None: + """ + Clears the ROI table in the spectrum viewer. + """ + self._roi_table_model.clear_table() + + class SpectrumViewerWindowView(BaseMainWindowView): roiTableView: RemovableRowTableView sampleStackSelector: DatasetSelectorWidgetView @@ -151,8 +283,6 @@ class SpectrumViewerWindowView(BaseMainWindowView): roiPropertiesGroupBox: QGroupBox roi_properties_widget: ROIPropertiesTableWidget - last_clicked_roi: str - spectrum_widget: SpectrumWidget number_roi_properties_procced: int = 0 @@ -176,6 +306,10 @@ def __init__(self, main_window: MainWindowView): self.roiPropertiesGroupBox) self.old_table_names: list[str] = [] + self.table_view = ROITableWidget(self.roiTableView) + + self.roiPropertiesSpinBoxes: dict[str, QSpinBox] = {} + self.roiPropertiesLabels: dict[str, QLabel] = {} self.presenter = SpectrumViewerWindowPresenter(self, main_window) @@ -236,11 +370,7 @@ def __init__(self, main_window: MainWindowView): self.exportButton.clicked.connect(self.presenter.handle_export_csv) self.exportButtonRITS.clicked.connect(self.presenter.handle_rits_export) - # Point table - self.roiTableView.setSelectionBehavior(QAbstractItemView.SelectRows) - self.roiTableView.setSelectionMode(QAbstractItemView.SingleSelection) - self.roiTableView.setAlternatingRowColors(True) - self.roiTableView.clicked.connect(self.handle_table_click) + self.table_view.clicked.connect(self.handle_table_click) # Roi Prop table self.roi_table_properties = ["Top", "Bottom", "Left", "Right"] @@ -251,8 +381,7 @@ def __init__(self, main_window: MainWindowView): self.spectrum_widget.roi_changed.connect(self.set_roi_properties) - _ = self.roi_table_model # Initialise model - self.current_roi_name = self.last_clicked_roi = self.roi_table_model.roi_names()[0] + self.current_roi_name = self.last_clicked_roi = self.table_view.roi_table_model.roi_names()[0] self.set_roi_properties() self.experimentSetupFormWidget = ExperimentSetupFormWidget(self.experimentSetupGroupBox) @@ -266,11 +395,11 @@ def on_row_change(item: QModelIndex, _: Any) -> None: @param item: item in table """ - self.selected_row = item.row() - self.current_roi_name = self.roi_table_model.get_element(item.row(), 0) + self.table_view.selected_row = item.row() + self.table_view.current_roi_name = self.table_view.get_roi_name_by_row(item.row()) self.set_roi_properties() - self.roiTableView.selectionModel().currentRowChanged.connect(on_row_change) + self.table_view.selectionModel().currentRowChanged.connect(on_row_change) def on_data_in_table_change() -> None: """ @@ -278,30 +407,33 @@ def on_data_in_table_change() -> None: If the ROI name has changed, update the ROI name in the spectrum widget. If the visibility of an ROI has changed, update the visibility of the ROI in the spectrum widget. """ - entered_name = self.roi_table_model.get_element(self.selected_row, 0) - if entered_name.lower() not in ["", " ", "all"] and entered_name != self.current_roi_name: + entered_name = self.table_view.get_roi_name_by_row(self.table_view.selected_row).strip() + if not entered_name: + self.table_view.set_roi_name_by_row(self.table_view.selected_row, self.table_view.current_roi_name) + self.table_view.last_clicked_roi = self.table_view.current_roi_name + self.table_view.set_old_table_names(self.presenter.get_roi_names()) + return + if entered_name.lower() not in ["", " ", "all"] and entered_name != self.table_view.current_roi_name: if entered_name in self.presenter.get_roi_names(): - entered_name = self.old_table_names[self.selected_row] - self.roi_table_model.set_element(self.selected_row, 0, self.old_table_names[self.selected_row]) - self.current_roi_name = entered_name - self.last_clicked_roi = self.current_roi_name + entered_name = self.table_view.old_table_names[self.table_view.selected_row] + self.table_view.roi_table_model.set_element( + self.table_view.selected_row, 0, self.table_view.old_table_names[self.table_view.selected_row]) + self.table_view.current_roi_name = entered_name + self.table_view.last_clicked_roi = self.table_view.current_roi_name else: - self.presenter.rename_roi(self.current_roi_name, entered_name) - self.current_roi_name = entered_name - self.last_clicked_roi = self.current_roi_name + self.presenter.rename_roi(self.table_view.current_roi_name, entered_name) + self.table_view.current_roi_name = entered_name + self.table_view.last_clicked_roi = self.table_view.current_roi_name self.set_roi_properties() else: - self.roi_table_model.set_element(self.selected_row, 0, self.old_table_names[self.selected_row]) + self.table_view.roi_table_model.set_element( + self.table_view.selected_row, 0, self.table_view.old_table_names[self.table_view.selected_row]) - self.set_old_table_names() + self.table_view.set_old_table_names(self.presenter.get_roi_names()) self.on_visibility_change() return - self.roi_table_model.dataChanged.connect(on_data_in_table_change) - header = self.roiTableView.horizontalHeader() - header.setSectionResizeMode(0, QHeaderView.Stretch) - header.setSectionResizeMode(1, QHeaderView.ResizeToContents) - header.setSectionResizeMode(2, QHeaderView.ResizeToContents) + self.table_view.roi_table_model.dataChanged.connect(on_data_in_table_change) self.formTabs.currentChanged.connect(self.handle_change_tab) @@ -323,38 +455,34 @@ def on_visibility_change(self) -> None: """ self.presenter.handle_storing_current_roi_name_on_tab_change() - if self.presenter.export_mode == ExportMode.ROI_MODE: self.set_roi_visibility_flags(ROI_RITS, visible=False) - if self.roi_table_model.rowCount() == 0: + if self.table_view.row_count() == 0: self.disable_roi_properties() else: self.set_roi_properties() - for roi_name, _, roi_visible in self.roi_table_model: + for row in range(self.table_view.row_count()): + roi_name = self.table_view.get_roi_name_by_row(row) + roi_visible = self.table_view.get_roi_visibility_by_row(row) self.set_roi_visibility_flags(roi_name, visible=roi_visible) if roi_visible: self.presenter.redraw_spectrum(roi_name) elif self.presenter.export_mode == ExportMode.IMAGE_MODE: - for roi_name, _, _ in self.roi_table_model: + for row in range(self.table_view.row_count()): + roi_name = self.table_view.get_roi_name_by_row(row) self.set_roi_visibility_flags(roi_name, visible=False) self.set_roi_visibility_flags(ROI_RITS, visible=True) self.presenter.redraw_spectrum(ROI_RITS) - self.current_roi_name = ROI_RITS + self.table_view.current_roi_name = ROI_RITS - for _, spinbox in self.roi_properties_widget.roiPropertiesSpinBoxes.items(): - spinbox.setEnabled(True) - self.set_roi_properties() + for _, spinbox in self.roiPropertiesSpinBoxes.items(): + spinbox.setEnabled(False) - @property - def roi_table_model(self) -> TableModel: - if self.roiTableView.model() is None: - mdl = TableModel() - self.roiTableView.setModel(mdl) - return self.roiTableView.model() + self.set_roi_properties() @property def current_dataset_id(self) -> UUID | None: @@ -470,34 +598,13 @@ def set_new_roi(self) -> None: def handle_table_click(self, index: QModelIndex) -> None: if index.isValid() and index.column() == 1: - roi_name = self.roi_table_model.index(index.row(), 0).data() + roi_name = self.table_view.get_roi_name_by_row(index.row()) self.set_spectum_roi_color(roi_name) def set_spectum_roi_color(self, roi_name: str) -> None: spectrum_roi = self.spectrum_widget.roi_dict[roi_name] spectrum_roi.change_color_action.trigger() - def update_roi_color(self, roi_name: str, new_color: tuple[int, int, int]) -> None: - """ - Finds ROI by name in table and updates colour. - @param roi_name: Name of the ROI to update. - @param new_color: The new color for the ROI in (R, G, B) format. - """ - row = self.find_row_for_roi(roi_name) - if row is not None: - self.roi_table_model.update_color(row, new_color) - - def find_row_for_roi(self, roi_name: str) -> int | None: - """ - Returns row index for ROI name, or None if not found. - @param roi_name: Name ROI find. - @return: Row index ROI or None. - """ - for row in range(self.roi_table_model.rowCount()): - if self.roi_table_model.index(row, 0).data() == roi_name: - return row - return None - def set_roi_visibility_flags(self, roi_name: str, visible: bool) -> None: """ Set the visibility for the selected ROI and update the spectrum to reflect the change. @@ -528,41 +635,39 @@ def add_roi_table_row(self, name: str, colour: tuple[int, int, int]) -> None: @param name: The name of the ROI @param colour: The colour of the ROI """ - self.roi_table_model.appendNewRow(name, colour, True) - self.selected_row = self.roi_table_model.rowCount() - 1 - self.roiTableView.selectRow(self.selected_row) + self.table_view.add_row(name, colour, self.presenter.get_roi_names()) self.current_roi_name = name self.removeBtn.setEnabled(True) - self.set_old_table_names() def remove_roi(self) -> None: """ Clear the selected ROI in the table view """ - selected_row = self.roi_table_model.row_data(self.selected_row) - roi_name = self.roi_table_model.get_element(self.selected_row, 0) + + roi_name, selected_row, _ = self.table_view.get_row_data(self.table_view.selected_row) + assert isinstance(roi_name, str) roi_object = self.spectrum_widget.roi_dict[roi_name] + if selected_row: - self.roi_table_model.remove_row(self.selected_row) + self.table_view.remove_row(self.table_view.selected_row) self.presenter.do_remove_roi(roi_name) self.spectrum_widget.spectrum_data_dict.pop(roi_name) self.presenter.handle_roi_moved(roi_object) - self.selected_row = 0 self.roiTableView.selectRow(0) - if self.roi_table_model.rowCount() == 0: + if self.table_view.roi_table_model.rowCount() == 0: self.removeBtn.setEnabled(False) self.disable_roi_properties() else: - self.set_old_table_names() - self.current_roi_name = self.roi_table_model.get_element(self.selected_row, 0) + self.table_view.set_old_table_names(self.presenter.get_roi_names()) + self.table_view.current_roi_name = self.table_view.get_roi_name_by_row(self.table_view.selected_row) self.set_roi_properties() def clear_all_rois(self) -> None: """ Clear all ROIs from the table view """ - self.roi_table_model.clear_table() + self.table_view.roi_table_model.clear_table() self.spectrum_widget.spectrum_data_dict = {} self.spectrum_widget.spectrum.clearPlots() self.removeBtn.setEnabled(False) @@ -597,32 +702,49 @@ def tof_units_mode(self) -> str: def set_roi_properties(self) -> None: if self.presenter.export_mode == ExportMode.IMAGE_MODE: - self.current_roi_name = ROI_RITS - if (self.current_roi_name not in self.presenter.view.spectrum_widget.roi_dict + self.table_view.current_roi_name = ROI_RITS + if (self.table_view.current_roi_name not in self.presenter.view.spectrum_widget.roi_dict or not self.roi_properties_widget.roiPropertiesSpinBoxes): return - current_roi = self.presenter.view.spectrum_widget.get_roi(self.current_roi_name) - self.roi_properties_widget.roiPropertiesGroupBox.setTitle(f"Roi Properties: {self.current_roi_name}") + current_roi = self.presenter.view.spectrum_widget.get_roi(self.table_view.current_roi_name) + self.roi_properties_widget.roiPropertiesGroupBox.setTitle(f"Roi Properties: {self.table_view.current_roi_name}") roi_iter_order = ["Left", "Top", "Right", "Bottom"] for row, pos in enumerate(current_roi): with QSignalBlocker(self.roi_properties_widget.roiPropertiesSpinBoxes[roi_iter_order[row]]): self.roi_properties_widget.roiPropertiesSpinBoxes[roi_iter_order[row]].setValue(pos) - self.presenter.redraw_spectrum(self.current_roi_name) + self.presenter.redraw_spectrum(self.table_view.current_roi_name) self.roi_properties_widget.roiPropertiesLabels["Width"].setText(str(current_roi.width)) self.roi_properties_widget.roiPropertiesLabels["Height"].setText(str(current_roi.height)) for spinbox in self.roi_properties_widget.roiPropertiesSpinBoxes.values(): spinbox.setEnabled(True) def disable_roi_properties(self) -> None: - self.last_clicked_roi = "roi" - self.roi_properties_widget.disable_roi_properties() + self.roiPropertiesGroupBox.setTitle("Roi Properties: None selected") + self.table_view.last_clicked_roi = "roi" + for _, spinbox in self.roiPropertiesSpinBoxes.items(): + with QSignalBlocker(spinbox): + spinbox.setMinimum(0) + spinbox.setValue(0) + spinbox.setDisabled(True) + for _, label in self.roiPropertiesLabels.items(): + label.setText("0") + + def get_roi_properties_spinboxes(self) -> dict[str, QSpinBox]: + return self.roiPropertiesSpinBoxes def get_checked_menu_option(self) -> QAction: return self.tof_mode_select_group.checkedAction() - def set_old_table_names(self) -> None: - self.old_table_names = self.presenter.get_roi_names() - if 'all' in self.old_table_names: - self.old_table_names.remove('all') - if 'rits_roi' in self.old_table_names: - self.old_table_names.remove('rits_roi') + def setup_roi_properties_spinboxes(self) -> None: + assert self.spectrum_widget.image.image_data is not None + for prop in self.roi_table_properties: + spin_box = QSpinBox() + if prop == "Top" or prop == "Bottom": + spin_box.setMaximum(self.spectrum_widget.image.image_data.shape[0]) + if prop == "Left" or prop == "Right": + spin_box.setMaximum(self.spectrum_widget.image.image_data.shape[1]) + spin_box.valueChanged.connect(self.presenter.do_adjust_roi) + self.roiPropertiesSpinBoxes[prop] = spin_box + for prop in self.roi_table_properties_secondary: + label = QLabel() + self.roiPropertiesLabels[prop] = label From 8f6c3d009954096c6fce3171d9d426d866953903 Mon Sep 17 00:00:00 2001 From: JackEAllen Date: Mon, 10 Feb 2025 13:06:02 +0000 Subject: [PATCH 08/11] Update References to Table Proprtiers to use ROITableWidget Class Update any references to properties that have been moved out of the SpectrumViewerWindowView class and into the ROITableWidget class --- .../gui/windows/spectrum_viewer/presenter.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/mantidimaging/gui/windows/spectrum_viewer/presenter.py b/mantidimaging/gui/windows/spectrum_viewer/presenter.py index 05f0227d881..01872b58b85 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/presenter.py +++ b/mantidimaging/gui/windows/spectrum_viewer/presenter.py @@ -215,8 +215,8 @@ def handle_roi_moved(self, roi: SpectrumROI) -> None: def handle_roi_clicked(self, roi: SpectrumROI) -> None: if not roi.name == ROI_RITS: - self.view.current_roi_name = roi.name - self.view.last_clicked_roi = roi.name + self.view.table_view.current_roi_name = roi.name + self.view.table_view.last_clicked_roi = roi.name self.view.set_roi_properties() def redraw_spectrum(self, name: str) -> None: @@ -352,7 +352,7 @@ def change_roi_colour(self, roi_name: str, new_colour: tuple[int, int, int]) -> """ if roi_name in self.view.spectrum_widget.roi_dict: self.view.spectrum_widget.roi_dict[roi_name].colour = new_colour - self.view.update_roi_color(roi_name, new_colour) + self.view.table_view.update_roi_color(roi_name, new_colour) self.view.on_visibility_change() def add_rits_roi(self) -> None: @@ -394,7 +394,7 @@ def do_remove_roi(self, roi_name: str | None = None) -> None: for name in list(self.get_roi_names()): self.view.spectrum_widget.remove_roi(name) self.view.spectrum_widget.roi_dict.clear() - self.view.roi_table_model.clear_table() + self.view.table_view.roi_table_model.clear_table() self.model.remove_all_roi() else: self.view.spectrum_widget.remove_roi(roi_name) @@ -437,21 +437,21 @@ def change_selected_menu_option(self, opt: str) -> None: def do_adjust_roi(self) -> None: new_roi = self.view.roi_properties_widget.as_roi() - self.view.spectrum_widget.adjust_roi(new_roi, self.view.current_roi_name) + self.view.spectrum_widget.adjust_roi(new_roi, self.view.table_view.current_roi_name) def handle_storing_current_roi_name_on_tab_change(self) -> None: - old_table_names = self.view.old_table_names - old_current_roi_name = self.view.current_roi_name - old_last_clicked_roi = self.view.last_clicked_roi + old_table_names = self.view.table_view.old_table_names + old_current_roi_name = self.view.table_view.current_roi_name + old_last_clicked_roi = self.view.table_view.last_clicked_roi if self.export_mode == ExportMode.ROI_MODE: if old_current_roi_name == ROI_RITS and old_last_clicked_roi in old_table_names: - self.view.current_roi_name = old_last_clicked_roi + self.view.table_view.current_roi_name = old_last_clicked_roi else: - self.view.last_clicked_roi = old_current_roi_name + self.view.table_view.last_clicked_roi = old_current_roi_name elif self.export_mode == ExportMode.IMAGE_MODE: if (old_current_roi_name != ROI_RITS and old_current_roi_name in old_table_names and old_last_clicked_roi != old_current_roi_name): - self.view.last_clicked_roi = old_current_roi_name + self.view.table_view.last_clicked_roi = old_current_roi_name @staticmethod def check_action(action: QAction, param: bool) -> None: From 3ba8ba16b81cfd63effe4698fbbd41bda04462d7 Mon Sep 17 00:00:00 2001 From: JackEAllen Date: Mon, 10 Feb 2025 13:07:15 +0000 Subject: [PATCH 09/11] Update References to Table and add Additional Mocks in Tests --- .../spectrum_viewer/test/presenter_test.py | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/mantidimaging/gui/windows/spectrum_viewer/test/presenter_test.py b/mantidimaging/gui/windows/spectrum_viewer/test/presenter_test.py index 317bbbc1e08..afc4dd99ac9 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/test/presenter_test.py +++ b/mantidimaging/gui/windows/spectrum_viewer/test/presenter_test.py @@ -7,9 +7,12 @@ from unittest import mock import numpy as np + +from PyQt5.QtCore import QAbstractTableModel from PyQt5.QtWidgets import QPushButton, QActionGroup, QGroupBox, QAction, QCheckBox, QTabWidget, QWidget, QSpinBox from parameterized import parameterized +from mantidimaging.gui.widgets import RemovableRowTableView from mantidimaging.core.data.dataset import Dataset from mantidimaging.core.utility.sensible_roi import SensibleROI from mantidimaging.gui.windows.main import MainWindowView @@ -38,6 +41,9 @@ def setUp(self) -> None: "Left": mock.create_autospec(QSpinBox, instance=True), "Right": mock.create_autospec(QSpinBox, instance=True) } + self.view.table_view = mock.create_autospec(RemovableRowTableView, instance=True) + self.view.table_view.roi_table_model = mock.create_autospec(QAbstractTableModel, instance=True) + self.view.table_view.roi_table_model.clear_table = mock.Mock() mock_spectrum_roi_dict = mock.create_autospec(dict, instance=True) self.view.spectrum_widget = mock.create_autospec(SpectrumWidget, roi_dict=mock_spectrum_roi_dict, instance=True) self.view.spectrum_widget.spectrum_plot_widget = mock.create_autospec(SpectrumPlotWidget, @@ -278,16 +284,15 @@ def test_WHEN_do_remove_roi_called_THEN_roi_removed(self): def test_WHEN_roi_clicked_THEN_roi_updated(self): roi = SpectrumROI("themightyroi", SensibleROI()) self.presenter.handle_roi_clicked(roi) - self.assertEqual(self.view.current_roi_name, "themightyroi") - self.assertEqual(self.view.last_clicked_roi, "themightyroi") + self.assertEqual(self.view.table_view.last_clicked_roi, "themightyroi") self.view.set_roi_properties.assert_called_once() def test_WHEN_rits_roi_clicked_THEN_rois_not_updated(self): - self.view.current_roi_name = self.view.last_clicked_roi = "NOT_RITS_ROI" + self.view.table_view.current_roi_name = self.view.table_view.last_clicked_roi = "NOT_RITS_ROI" roi = SpectrumROI(ROI_RITS, SensibleROI()) self.presenter.handle_roi_clicked(roi) - self.assertEqual(self.view.current_roi_name, "NOT_RITS_ROI") - self.assertEqual(self.view.last_clicked_roi, "NOT_RITS_ROI") + self.assertEqual(self.view.table_view.current_roi_name, "NOT_RITS_ROI") + self.assertEqual(self.view.table_view.last_clicked_roi, "NOT_RITS_ROI") self.view.set_roi_properties.assert_not_called() def test_WHEN_ROI_renamed_THEN_roi_renamed(self): @@ -395,7 +400,7 @@ def test_WHEN_menu_option_selected_THEN_menu_option_changed(self): def test_WHEN_roi_changed_via_spinboxes_THEN_roi_adjusted(self): self.view.roi_properties_widget.as_roi = mock.Mock(return_value=SensibleROI(10, 10, 20, 30)) - self.view.current_roi_name = "roi_1" + self.view.table_view.current_roi_name = "roi_1" self.presenter.do_adjust_roi() self.view.spectrum_widget.adjust_roi.assert_called_once_with(SensibleROI(10, 10, 20, 30), "roi_1") @@ -404,13 +409,13 @@ def test_WHEN_roi_changed_via_spinboxes_THEN_roi_adjusted(self): (["roi_1", "roi_2", "roi_3"], ROI_RITS, "roi_2", ExportMode.ROI_MODE, "roi_2")]) def test_WHEN_change_tab_THEN_current_roi_correct(self, old_table_names, current_roi_name, last_clicked_roi, export_mode, expected_roi): - self.view.old_table_names = old_table_names - self.view.current_roi_name = current_roi_name - self.view.last_clicked_roi = last_clicked_roi + self.view.table_view.old_table_names = old_table_names + self.view.table_view.current_roi_name = current_roi_name + self.view.table_view.last_clicked_roi = last_clicked_roi self.presenter.export_mode = export_mode self.presenter.handle_storing_current_roi_name_on_tab_change() - self.assertEqual(self.view.current_roi_name, expected_roi) - self.assertEqual(self.view.last_clicked_roi, expected_roi) + self.assertEqual(self.view.table_view.current_roi_name, expected_roi) + self.assertEqual(self.view.table_view.last_clicked_roi, expected_roi) def test_WHEN_refresh_spectrum_plot_THEN_spectrum_plot_refreshed(self): self.view.spectrum_widget.spectrum = mock.MagicMock() @@ -436,15 +441,15 @@ def test_WHEN_redraw_all_rois_THEN_rois_set_correctly(self): @parameterized.expand([("roi", "roi_clicked", "roi_clicked"), ("roi", ROI_RITS, "roi")]) def test_WHEN_roi_clicked_THEN_current_and_last_clicked_roi_updated_correctly(self, old_roi, clicked_roi, expected_roi): - self.view.current_roi_name = old_roi - self.view.last_clicked_roi = old_roi + self.view.table_view.current_roi_name = old_roi + self.view.table_view.last_clicked_roi = old_roi self.presenter.handle_roi_clicked(SpectrumROI(clicked_roi, SensibleROI(), pos=(0, 0))) - self.assertEqual(self.view.current_roi_name, expected_roi) - self.assertEqual(self.view.last_clicked_roi, expected_roi) + self.assertEqual(self.view.table_view.current_roi_name, expected_roi) + self.assertEqual(self.view.table_view.last_clicked_roi, expected_roi) def test_WHEN_roi_clicked_THEN_roi_properties_set(self): - self.view.current_roi_name = "" - self.view.last_clicked_roi = "" + self.view.table_view.current_roi_name = "" + self.view.table_view.last_clicked_roi = "" self.presenter.handle_roi_clicked(SpectrumROI("roi_clicked", SensibleROI(), pos=(0, 0))) self.view.set_roi_properties.assert_called_once() From 6094e2392eccb30473d4533224f5f4a0dac73818 Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Fri, 14 Feb 2025 13:38:06 +0000 Subject: [PATCH 10/11] Fix system tests for ROITableWidget changes --- .../gui/test/gui_system_spectrum_test.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/mantidimaging/gui/test/gui_system_spectrum_test.py b/mantidimaging/gui/test/gui_system_spectrum_test.py index 195226e3a63..6f4d3014de4 100644 --- a/mantidimaging/gui/test/gui_system_spectrum_test.py +++ b/mantidimaging/gui/test/gui_system_spectrum_test.py @@ -40,36 +40,36 @@ def tearDown(self) -> None: def test_spectrum_window_opens_with_data_in_default_state(self) -> None: self.assertFalse(self.spectrum_window.normaliseCheckBox.isChecked()) self.assertFalse(self.spectrum_window.normalise_ShutterCount_CheckBox.isEnabled()) - self.assertEqual(self.spectrum_window.roi_table_model.rowCount(), 1) - self.assertEqual(self.spectrum_window.roi_table_model.columnCount(), 3) + self.assertEqual(self.spectrum_window.table_view.roi_table_model.rowCount(), 1) + self.assertEqual(self.spectrum_window.table_view.roi_table_model.columnCount(), 3) self.assertTrue(self.spectrum_window.addBtn.isEnabled()) self.assertTrue(self.spectrum_window.removeBtn.isEnabled()) self.assertTrue(self.spectrum_window.exportButton.isEnabled()) - self.assertIn('roi', self.spectrum_window.roi_table_model.roi_names()) + self.assertIn('roi', self.spectrum_window.table_view.roi_table_model.roi_names()) self.assertIn('roi', self.spectrum_window.spectrum_widget.roi_dict) QTest.qWait(SHOW_DELAY) def test_add_roi(self) -> None: for i in range(1, 4): - initial_roi_count = self.spectrum_window.roi_table_model.rowCount() + initial_roi_count = self.spectrum_window.table_view.roi_table_model.rowCount() QTest.mouseClick(self.spectrum_window.addBtn, Qt.MouseButton.LeftButton) QTest.qWait(SHORT_DELAY) - final_roi_count = self.spectrum_window.roi_table_model.rowCount() + final_roi_count = self.spectrum_window.table_view.roi_table_model.rowCount() self.assertEqual(final_roi_count, initial_roi_count + 1) - self.assertIn(f'roi_{i}', self.spectrum_window.roi_table_model.roi_names()) + self.assertIn(f'roi_{i}', self.spectrum_window.table_view.roi_table_model.roi_names()) self.assertIn(f'roi_{i}', self.spectrum_window.spectrum_widget.roi_dict) def test_remove_roi(self) -> None: QTest.mouseClick(self.spectrum_window.addBtn, Qt.MouseButton.LeftButton) QTest.qWait(SHORT_DELAY) - initial_roi_count = self.spectrum_window.roi_table_model.rowCount() + initial_roi_count = self.spectrum_window.table_view.roi_table_model.rowCount() QTest.mouseClick(self.spectrum_window.removeBtn, Qt.MouseButton.LeftButton) QTest.qWait(SHORT_DELAY) - final_roi_count = self.spectrum_window.roi_table_model.rowCount() + final_roi_count = self.spectrum_window.table_view.roi_table_model.rowCount() self.assertEqual(final_roi_count, initial_roi_count - 1) - self.assertNotIn(f'roi_{initial_roi_count - 1}', self.spectrum_window.roi_table_model.roi_names()) + self.assertNotIn(f'roi_{initial_roi_count - 1}', self.spectrum_window.table_view.roi_table_model.roi_names()) self.assertNotIn(f'roi_{initial_roi_count - 1}', self.spectrum_window.spectrum_widget.roi_dict) def test_change_roi_color(self): @@ -96,7 +96,7 @@ def test_rename_roi(self): old_name = 'roi_1' new_name = 'roi_renamed' - table_model = self.spectrum_window.roi_table_model + table_model = self.spectrum_window.table_view.roi_table_model row = table_model.roi_names().index(old_name) table_model.set_element(row, 0, new_name) From 4835ed97f27558b99ed1d804cf8103bb119605dc Mon Sep 17 00:00:00 2001 From: Sam Tygier Date: Mon, 17 Feb 2025 16:20:35 +0000 Subject: [PATCH 11/11] Put the ROITableWidget in the UI file --- mantidimaging/gui/ui/spectrum_viewer.ui | 6 +++--- mantidimaging/gui/windows/spectrum_viewer/view.py | 9 ++------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/mantidimaging/gui/ui/spectrum_viewer.ui b/mantidimaging/gui/ui/spectrum_viewer.ui index 14325b70c48..f1ec56144ac 100644 --- a/mantidimaging/gui/ui/spectrum_viewer.ui +++ b/mantidimaging/gui/ui/spectrum_viewer.ui @@ -238,7 +238,7 @@ - + true @@ -545,9 +545,9 @@
mantidimaging.gui.widgets.dataset_selector
- RemovableRowTableView + ROITableWidget QTableView -
mantidimaging.gui.widgets
+
mantidimaging.gui.windows.spectrum_viewer.view
 diff --git a/mantidimaging/gui/windows/spectrum_viewer/view.py b/mantidimaging/gui/windows/spectrum_viewer/view.py index b84cf8edcb5..a0d9dda4997 100644 --- a/mantidimaging/gui/windows/spectrum_viewer/view.py +++ b/mantidimaging/gui/windows/spectrum_viewer/view.py @@ -138,10 +138,6 @@ class ROITableWidget(RemovableRowTableView): def __init__(self, parent=None): super().__init__(parent) - if parent is not None: - layout = parent.layout() - if layout is not None: - layout.addWidget(self) self.old_table_names = [] self.selected_row = 0 @@ -258,7 +254,7 @@ def clear_table(self) -> None: class SpectrumViewerWindowView(BaseMainWindowView): - roiTableView: RemovableRowTableView + table_view: ROITableWidget sampleStackSelector: DatasetSelectorWidgetView normaliseStackSelector: DatasetSelectorWidgetView @@ -306,7 +302,6 @@ def __init__(self, main_window: MainWindowView): self.roiPropertiesGroupBox) self.old_table_names: list[str] = [] - self.table_view = ROITableWidget(self.roiTableView) self.roiPropertiesSpinBoxes: dict[str, QSpinBox] = {} self.roiPropertiesLabels: dict[str, QLabel] = {} @@ -653,7 +648,7 @@ def remove_roi(self) -> None: self.presenter.do_remove_roi(roi_name) self.spectrum_widget.spectrum_data_dict.pop(roi_name) self.presenter.handle_roi_moved(roi_object) - self.roiTableView.selectRow(0) + self.table_view.selectRow(0) if self.table_view.roi_table_model.rowCount() == 0: self.removeBtn.setEnabled(False)