Unverified Commit 2b27c26d authored by Gagik Vardanyan's avatar Gagik Vardanyan Committed by GitHub
Browse files

Merge pull request #30375 from...

Merge pull request #30375 from mantidproject/30308_Sliceviewer_exception_when_changing_peak_workspace_when_overlaid

SliceViewer: Exception when changing peak workspace in the ADS when overlaid
parents 327ca959 3a5119a6
......@@ -66,6 +66,7 @@ Bugfixes
- Fixed a crash in the plot config when removing the last curve on a plot
- Fixed a bug with editing legends in-situ on Linux platforms.
- Fixed a bug in SliceViewer that caused shown data to not update correctly when changing axis selection.
- Fixed a bug in SliceViewer where peaks workspaces would still be present even though they had been changed, removed or cleared in workbench.
- Fixed bug supplying rebin arguments for non-orthogonal data in sliceviewer that meant that not all the availible data within the axes limits were being plotted.
- Fixed bug in supplying display indices when viewinng axes changed in non-orthog view.
- Fixed a crash in SliceViewer when hovering the cursor over Direct or Indirect data.
......
......@@ -43,6 +43,9 @@ class SliceViewerADSObserver(AnalysisDataServiceObserver):
self.observeReplace(True)
self.observeRename(True)
def __del__(self):
self.observeAll(False)
@_catch_exceptions
def clearHandle(self):
"""Called when the ADS is deleted all of its workspaces"""
......
......@@ -13,6 +13,7 @@ from enum import Enum
from mantidqt.widgets.workspacedisplay.table.presenter \
import TableWorkspaceDataPresenter, create_table_item
from .model import create_peaksviewermodel
from ..adsobsever import SliceViewerADSObserver
class PeaksWorkspaceDataPresenter(TableWorkspaceDataPresenter):
......@@ -39,7 +40,7 @@ class PeaksWorkspaceDataPresenter(TableWorkspaceDataPresenter):
return item
class PeaksViewerPresenter():
class PeaksViewerPresenter:
"""Controls a PeaksViewerView with a given model to display
the peaks table and interaction controls for single workspace.
"""
......@@ -140,7 +141,7 @@ class PeaksViewerPresenter():
raise ValueError("Expected a PeaksWorkspace. Found {}.".format(type(ws)))
class PeaksViewerCollectionPresenter():
class PeaksViewerCollectionPresenter:
"""Controls a widget comprising of multiple PeasViewerViews to display and
interact with multiple PeaksWorkspaces"""
......@@ -161,6 +162,16 @@ class PeaksViewerCollectionPresenter():
"""
self._view = view
self._child_presenters = []
self._ads_observer = None
self.setup_ads_observer()
def setup_ads_observer(self):
if self._ads_observer is None:
self._ads_observer = SliceViewerADSObserver(self.replace_handle, self.rename_handle, self.clear_handle,
self.delete_handle)
def clear_observer(self):
self._ads_observer = None
@property
def view(self):
......@@ -172,6 +183,7 @@ class PeaksViewerCollectionPresenter():
:param name: The name of a PeaksWorkspace.
:returns: The child presenter
"""
self.setup_ads_observer()
presenter = PeaksViewerPresenter(self._create_peaksviewer_model(name),
self._view.append_peaksviewer())
self._child_presenters.append(presenter)
......@@ -209,6 +221,7 @@ class PeaksViewerCollectionPresenter():
Remove the named workspace from display. No op if no workspace can be found with that name
:param name: The name of a workspace
"""
self.setup_ads_observer()
child_presenters = self._child_presenters
presenter_to_remove = None
for child in child_presenters:
......@@ -231,6 +244,7 @@ class PeaksViewerCollectionPresenter():
def notify(self, event):
"""Dispatch notification to all subpresenters"""
self.setup_ads_observer()
for presenter in self._child_presenters:
presenter.notify(event)
......@@ -254,3 +268,22 @@ class PeaksViewerCollectionPresenter():
fg_color = '#000000'
return create_peaksviewermodel(name, fg_color, self.DEFAULT_BG_COLOR)
def replace_handle(self, ws_name, _):
if ws_name in self.workspace_names():
self.remove_peaksworkspace(ws_name)
self.append_peaksworkspace(ws_name)
def delete_handle(self, ws_name):
if ws_name in self.workspace_names():
self.remove_peaksworkspace(ws_name)
def clear_handle(self):
# This is likely handled at a higher level anyway, because SliceViewer closes given a clear all on the ADS.
for ws_name in self.workspace_names():
self.remove_peaksworkspace(ws_name)
def rename_handle(self, ws_name, new_name):
if ws_name in self.workspace_names():
self.remove_peaksworkspace(ws_name)
self.append_peaksworkspace(new_name)
......@@ -11,7 +11,10 @@ import unittest
from unittest.mock import MagicMock, create_autospec, patch
# 3rdparty imports
from mantid.api import AnalysisDataService as ADS
from mantid.simpleapi import CreateSampleWorkspace, RenameWorkspace
from mantid.dataobjects import PeaksWorkspace
from mantidqt.widgets.sliceviewer.adsobsever import SliceViewerADSObserver
from mantidqt.widgets.sliceviewer.peaksviewer \
import PeaksViewerPresenter, PeaksViewerCollectionPresenter, PeaksViewerModel
......@@ -82,6 +85,105 @@ class PeaksViewerCollectionPresenterTest(unittest.TestCase):
for child in child_presenters:
child.notify.assert_called_once_with(PeaksViewerPresenter.Event.OverlayPeaks)
def test_ensure_that_the_ads_observer_calls_clear_handle(self, _):
presenter = PeaksViewerCollectionPresenter(MagicMock())
presenter.clear_handle = MagicMock()
self.assertTrue(isinstance(presenter._ads_observer, SliceViewerADSObserver))
presenter._ads_observer = SliceViewerADSObserver(presenter.replace_handle, presenter.rename_handle,
presenter.clear_handle, presenter.delete_handle)
CreateSampleWorkspace(OutputWorkspace="ws")
ADS.clear()
presenter.clear_handle.assert_called_once()
def test_ensure_that_the_ads_observer_calls_remove_handle(self, _):
presenter = PeaksViewerCollectionPresenter(MagicMock())
presenter.delete_handle = MagicMock()
self.assertTrue(isinstance(presenter._ads_observer, SliceViewerADSObserver))
presenter._ads_observer = SliceViewerADSObserver(presenter.replace_handle, presenter.rename_handle,
presenter.clear_handle, presenter.delete_handle)
CreateSampleWorkspace(OutputWorkspace="ws")
ADS.remove("ws")
presenter.delete_handle.assert_called_once_with("ws")
def test_ensure_that_the_ads_observer_calls_replace_handle(self, _):
presenter = PeaksViewerCollectionPresenter(MagicMock())
presenter.replace_handle = MagicMock()
self.assertTrue(isinstance(presenter._ads_observer, SliceViewerADSObserver))
presenter._ads_observer = SliceViewerADSObserver(presenter.replace_handle, presenter.rename_handle,
presenter.clear_handle, presenter.delete_handle)
CreateSampleWorkspace(OutputWorkspace="ws")
CreateSampleWorkspace(OutputWorkspace="ws")
presenter.replace_handle.assert_called_once()
def test_ensure_that_the_ads_observer_calls_rename_handle(self, _):
presenter = PeaksViewerCollectionPresenter(MagicMock())
presenter.rename_handle = MagicMock()
self.assertTrue(isinstance(presenter._ads_observer, SliceViewerADSObserver))
presenter._ads_observer = SliceViewerADSObserver(presenter.replace_handle, presenter.rename_handle,
presenter.clear_handle, presenter.delete_handle)
CreateSampleWorkspace(OutputWorkspace="ws")
RenameWorkspace(InputWorkspace="ws", OutputWorkspace="ws1")
presenter.rename_handle.assert_called_once_with("ws", "ws1")
def test_ensure_replace_handle_removes_and_re_adds_workspaces(self, _):
presenter = PeaksViewerCollectionPresenter(MagicMock())
presenter.append_peaksworkspace = MagicMock()
presenter.workspace_names = MagicMock(return_value=["ws"])
presenter.remove_peaksworkspace = MagicMock()
presenter.replace_handle("ws", None)
presenter.remove_peaksworkspace.assert_called_once_with("ws")
presenter.append_peaksworkspace.assert_called_once_with("ws")
def test_ensure_delete_handle_removes_workspace(self, _):
presenter = PeaksViewerCollectionPresenter(MagicMock())
presenter.remove_peaksworkspace = MagicMock()
presenter.workspace_names = MagicMock(return_value=["ws"])
presenter.delete_handle("ws")
presenter.remove_peaksworkspace.assert_called_once_with("ws")
def test_ensure_clear_handle_removes_all_workspaces(self, _):
presenter = PeaksViewerCollectionPresenter(MagicMock())
presenter.remove_peaksworkspace = MagicMock()
presenter.workspace_names = MagicMock(return_value=["ws", "ws1", "ws2"])
presenter.clear_handle()
presenter.remove_peaksworkspace.assert_any_call("ws")
presenter.remove_peaksworkspace.assert_any_call("ws1")
presenter.remove_peaksworkspace.assert_any_call("ws2")
self.assertEqual(3, presenter.remove_peaksworkspace.call_count)
def test_ensure_rename_handle_removes_and_re_adds_the_new_workspace_name(self, _):
presenter = PeaksViewerCollectionPresenter(MagicMock())
presenter.remove_peaksworkspace = MagicMock()
presenter.workspace_names = MagicMock(return_value=["ws"])
presenter.append_peaksworkspace = MagicMock()
presenter.rename_handle("ws", "ws1")
presenter.remove_peaksworkspace.assert_called_once_with("ws")
presenter.append_peaksworkspace.assert_called_once_with("ws1")
def test_ensure_clear_observer_sets_observer_to_none(self, _):
presenter = PeaksViewerCollectionPresenter(MagicMock())
presenter._ads_observer = MagicMock()
presenter.clear_observer()
self.assertEqual(presenter._ads_observer, None)
if __name__ == '__main__':
unittest.main()
......@@ -69,6 +69,8 @@ class SliceViewer(ObservingPresenter):
self.ads_observer = SliceViewerADSObserver(self.replace_workspace, self.rename_workspace,
self.ADS_cleared, self.delete_workspace)
self.view.destroyed.connect(self._on_view_destroyed)
def new_plot_MDH(self):
"""
Tell the view to display a new plot of an MDHistoWorkspace
......@@ -396,6 +398,8 @@ class SliceViewer(ObservingPresenter):
def clear_observer(self):
self.ads_observer = None
if self._peaks_presenter is not None:
self._peaks_presenter.clear_observer()
# private api
def _create_peaks_presenter_if_necessary(self):
......@@ -447,3 +451,6 @@ class SliceViewer(ObservingPresenter):
def _close_view_with_message(self, message: str):
self.view.emit_close() # inherited from ObservingView
self._logger.warning(message)
def _on_view_destroyed(self):
self.clear_observer()
......@@ -426,6 +426,29 @@ class SliceViewerTest(unittest.TestCase):
self.view.setWindowTitle.assert_called_with(self.model.get_title())
presenter.new_plot.assert_called_once()
def test_clear_observer_peaks_presenter_not_none(self):
presenter, _ = _create_presenter(self.model,
self.view,
mock.MagicMock(),
enable_nonortho_axes=False,
supports_nonortho=False)
presenter._peaks_presenter = mock.MagicMock()
presenter.clear_observer()
presenter._peaks_presenter.clear_observer.assert_called_once()
def test_clear_observer_peaks_presenter_is_none(self):
presenter, _ = _create_presenter(self.model,
self.view,
mock.MagicMock(),
enable_nonortho_axes=False,
supports_nonortho=False)
presenter._peaks_presenter = None
# Will raise exception if misbehaving.
presenter.clear_observer()
if __name__ == '__main__':
unittest.main()
# Mantid Repository : https://github.com/mantidproject/mantid
# Mantid Repository : https://github.com/mantidproject/mantid
#
# Copyright © 2019 ISIS Rutherford Appleton Laboratory UKRI,
# NScD Oak Ridge National Laboratory, European Spallation Source,
......@@ -9,13 +10,12 @@ import io
import sys
import unittest
from unittest.mock import patch
from numpy import hstack
import matplotlib as mpl
from mantidqt.widgets.colorbar.colorbar import MIN_LOG_VALUE
from numpy import hstack
mpl.use('Agg')
from mantidqt.widgets.colorbar.colorbar import MIN_LOG_VALUE # noqa: E402
from mantid.simpleapi import ( # noqa: E402
CreateMDHistoWorkspace, CreateMDWorkspace, CreateSampleWorkspace, DeleteWorkspace,
FakeMDEventData, ConvertToDistribution, Scale, SetUB, RenameWorkspace)
......@@ -30,26 +30,32 @@ from math import inf # noqa: E402
@start_qapplication
class SliceViewerViewTest(unittest.TestCase, QtWidgetFinder):
@classmethod
def setUpClass(cls):
cls.histo_ws = CreateMDHistoWorkspace(Dimensionality=3,
Extents='-3,3,-10,10,-1,1',
SignalInput=range(100),
ErrorInput=range(100),
NumberOfBins='5,5,4',
Names='Dim1,Dim2,Dim3',
Units='MomentumTransfer,EnergyTransfer,Angstrom',
OutputWorkspace='ws_MD_2d')
cls.hkl_ws = CreateMDWorkspace(Dimensions=3,
Extents='-10,10,-10,10,-10,10',
Names='A,B,C',
Units='r.l.u.,r.l.u.,r.l.u.',
Frames='HKL,HKL,HKL',
OutputWorkspace='hkl_ws')
def setUp(self):
self.histo_ws = CreateMDHistoWorkspace(Dimensionality=3,
Extents='-3,3,-10,10,-1,1',
SignalInput=range(100),
ErrorInput=range(100),
NumberOfBins='5,5,4',
Names='Dim1,Dim2,Dim3',
Units='MomentumTransfer,EnergyTransfer,Angstrom',
OutputWorkspace='ws_MD_2d')
self.hkl_ws = CreateMDWorkspace(Dimensions=3,
Extents='-10,10,-10,10,-10,10',
Names='A,B,C',
Units='r.l.u.,r.l.u.,r.l.u.',
Frames='HKL,HKL,HKL',
OutputWorkspace='hkl_ws')
expt_info = CreateSampleWorkspace()
cls.hkl_ws.addExperimentInfo(expt_info)
self.hkl_ws.addExperimentInfo(expt_info)
SetUB('hkl_ws', 1, 1, 1, 90, 90, 90)
def tearDown(self):
for ii in QApplication.topLevelWidgets():
ii.close()
QApplication.sendPostedEvents()
QApplication.sendPostedEvents()
self.assert_no_toplevel_widgets()
def test_deleted_on_close(self):
pres = SliceViewer(self.histo_ws)
self.assert_widget_created()
......@@ -263,7 +269,6 @@ class SliceViewerViewTest(unittest.TestCase, QtWidgetFinder):
self.assertEqual(pres.view.data_view.get_axes_limits()[0], (xmin, xmax))
# private helper functions
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment