Remove direct calls to presenter from cut viewer view

qt/python/mantidqt/mantidqt/widgets/sliceviewer/cutviewer/model.py

@@ -6,8 +6,8 @@
 # SPDX - License - Identifier: GPL - 3.0 +
 #  This file is part of the mantid workbench.
 #
-from numpy import isclose, sum, argsort, ones, sqrt, zeros, mean, where, dot, cross, array, vstack
-from numpy.linalg import det
+from numpy import isclose, sum, argsort, ones, sqrt, mean, dot, cross, array, vstack, argmax
+from numpy.linalg import det, norm
 
 DEFAULT_NBINS = 50
 
@@ -96,28 +96,27 @@ def validate_extents(self, irow, extents):
             extents[:, irow] = umax, umin
         return extents
 
-    def calc_cut_representation_parameters(self, vectors, extents, nbins, states):
+    def calc_cut_representation_parameters(self, vectors_in_plane, extents_in_plane, nbins_in_plane, states):
         self.xvec = self.proj_matrix[states.index(0), :]
-        self.xvec /= sqrt(sum(self.xvec**2))
+        self.xvec /= norm(self.xvec)  # make unit vector
         self.yvec = self.proj_matrix[states.index(1), :]
-        self.yvec /= sqrt(sum(self.yvec**2))
+        self.yvec /= norm(self.yvec)
         # find x/y coord of start/end point of cut
-        cens = mean(extents, axis=0)  # in u{1..3} basis of view table
-        icut = where(nbins > 1)[0][0] if where(nbins > 1)[0].size > 0 else 0  # index of cut axis
-        ivecs = list(range(len(vectors)))
-        ivecs.pop(icut)
-        zero_vec = zeros(vectors[0].shape)  # position at  0 along cut axis
-        for ivec in ivecs:
-            zero_vec = zero_vec + cens[ivec] * vectors[ivec]
-        start = zero_vec + extents[0, icut] * vectors[icut, :]
-        end = zero_vec + extents[1, icut] * vectors[icut, :]
+        cens = mean(extents_in_plane, axis=0)  # in u{1..3} basis of view table
+        icut = argmax(nbins_in_plane > 1)  # index of cut axis
+        if not icut:
+            icut, ithick = 0, 1
+        else:
+            ithick = int(not bool(icut))  # index of in-plane vector along cut thickness (i.e. integrated in plane)
+        origin = cens[ithick] * vectors_in_plane[ithick, :]
+        start = origin + extents_in_plane[0, icut] * vectors_in_plane[icut, :]
+        end = origin + extents_in_plane[1, icut] * vectors_in_plane[icut, :]
         xmin = dot(start, self.xvec)
         xmax = dot(end, self.xvec)
         ymin = dot(start, self.yvec)
         ymax = dot(end, self.yvec)
         # get thickness of cut defined for unit vector perp to cut (so scale by magnitude of vector in the table)
-        iint = nbins[:-1].tolist().index(1)
-        thickness = (extents[1, iint] - extents[0, iint]) * sqrt(sum(vectors[iint, :] ** 2))
+        thickness = (extents_in_plane[1, ithick] - extents_in_plane[0, ithick]) * norm(vectors_in_plane[ithick, :])
         return xmin, xmax, ymin, ymax, thickness
 
     def calc_bin_params_from_cut_representation(self, xmin, xmax, ymin, ymax, thickness, out_plane_vector):
@@ -132,4 +131,7 @@ def calc_bin_params_from_cut_representation(self, xmin, xmax, ymin, ymax, thickn
         u2 = u2 / sqrt(sum(u2**2))
         u2_cen = dot(start, u2)
         u2_min, u2_max = u2_cen - thickness / 2, u2_cen + thickness / 2
-        return vstack((u1, u2)), array([[u1_min, u2_min], [u1_max, u2_max]]), [50, 1]  # vecs, extents, nbins (first 2 rows)
+        vectors_in_plane = vstack((u1, u2))
+        extents_in_plane = array([[u1_min, u2_min], [u1_max, u2_max]])
+        nbins_in_plane = array([50, 1])
+        return vectors_in_plane, extents_in_plane, nbins_in_plane

qt/python/mantidqt/mantidqt/widgets/sliceviewer/cutviewer/presenter.py

@@ -6,20 +6,26 @@
 # SPDX - License - Identifier: GPL - 3.0 +
 #  This file is part of the mantid workbench.
 #
-from mantidqt.widgets.sliceviewer.cutviewer.view import CutViewerView
-from mantidqt.widgets.sliceviewer.cutviewer.model import CutViewerModel
+from numpy import zeros
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from mantidqt.widgets.sliceviewer.cutviewer.view import CutViewerView
+    from mantidqt.widgets.sliceviewer.cutviewer.model import CutViewerModel
+    from mantidqt.widgets.sliceviewer.presenters.presenter import SliceViewer
 
 
 class CutViewerPresenter:
-    def __init__(self, sliceviewer_presenter, canvas):
+    def __init__(self, sliceviewer_presenter: "SliceViewer", model: "CutViewerModel", view: "CutViewerView"):
         """
         :param painter: An object responsible for drawing the representation of the cut
         :param sliceinfo_provider: An object responsible for providing access to current slice information
         :param parent: An optional parent widget
         """
-        self.view = CutViewerView(self, canvas, sliceviewer_presenter.get_frame())
-        self.model = CutViewerModel(sliceviewer_presenter.get_proj_matrix())
+        self.view = view
+        self.model = model
         self._sliceview_presenter = sliceviewer_presenter
+        self.view.subscribe_presenter(self)
 
     def show_view(self):
         self.view.show()
@@ -33,17 +39,17 @@ def get_view(self):
         return self.view
 
     def reset_view_table(self):
-        self.view.set_bin_params(
-            *self.model.get_default_bin_params(
-                self._sliceview_presenter.get_dimensions(),
-                self._sliceview_presenter.get_data_limits_to_fill_current_axes(),
-                self._sliceview_presenter.get_sliceinfo().z_value,
-            )
+        vectors, extents, nbins = self.model.get_default_bin_params(
+            self._sliceview_presenter.get_dimensions(),
+            self._sliceview_presenter.get_data_limits_to_fill_current_axes(),
+            self._sliceview_presenter.get_sliceinfo().z_value,
         )
+        self.view.set_bin_params(vectors, extents, nbins)
+        self.view.plot_cut_representation(*self.get_cut_representation_parameters(vectors[:-1, :], extents[:, :-1], nbins[:-1]))
 
     def validate_bin_params(self, irow, icol):
         iunchanged = int(not bool(irow))  # index of u1 or u2 - which ever not changed (3rd row not editable)
-        vectors, extents, nbins = self.view.get_bin_params()
+        vectors, extents, nbins = self.get_bin_params_from_view()
         if icol < 3:
             vectors = self.model.validate_vectors(irow, iunchanged, vectors)
         elif icol == 5:
@@ -54,20 +60,35 @@ def validate_bin_params(self, irow, icol):
             extents = self.model.validate_extents(irow, extents)
         return vectors, extents, nbins
 
+    def get_bin_params_from_view(self):
+        vectors = zeros((3, 3), dtype=float)
+        extents = zeros((2, 3), dtype=float)
+        nbins = zeros(3, dtype=int)
+        for ivec in range(vectors.shape[0]):
+            vectors[ivec, :] = self.view.get_vector(ivec)
+            extents[:, ivec] = self.view.get_extents(ivec)
+            nbins[ivec] = self.view.get_nbin(ivec)
+        return vectors, extents, nbins
+
     def update_cut(self):
-        vectors, extents, nbins = self.view.get_bin_params()
+        vectors, extents, nbins = self.get_bin_params_from_view()
         if self.model.valid_bin_params(vectors, extents, nbins):
             self._sliceview_presenter.perform_non_axis_aligned_cut(vectors, extents.flatten(order="F"), nbins)
 
-    def get_cut_representation_parameters(self):
-        cut_rep_params = self.model.calc_cut_representation_parameters(
-            *self.view.get_bin_params(), self._sliceview_presenter.get_dimensions().get_states()
+    def get_cut_representation_parameters(self, vectors_in_plane, extents_in_plane, nbins_in_plane):
+        xmin, xmax, ymin, ymax, thickness = self.model.calc_cut_representation_parameters(
+            vectors_in_plane, extents_in_plane, nbins_in_plane, self._sliceview_presenter.get_dimensions().get_states()
         )
-        return *cut_rep_params, self._sliceview_presenter.get_sliceinfo().get_northogonal_transform()
+        axes_transform = self._sliceview_presenter.get_sliceinfo().get_northogonal_transform()
+        return xmin, xmax, ymin, ymax, thickness, axes_transform
 
-    def update_bin_params_from_cut_representation(self, xmin, xmax, ymin, ymax, thickness):
-        vectors, _, _ = self.view.get_bin_params()
-        self.view.set_bin_params(*self.model.calc_bin_params_from_cut_representation(xmin, xmax, ymin, ymax, thickness, vectors[-1, :]))
+    def handle_cut_representation_changed(self, xmin, xmax, ymin, ymax, thickness):
+        out_plane_vector = self.view.get_vector(2)  # integrated dimension (last row in table)
+        vectors_in_plane, extents_in_plane, nbins_in_plane = self.model.calc_bin_params_from_cut_representation(
+            xmin, xmax, ymin, ymax, thickness, out_plane_vector
+        )
+        self.view.set_bin_params(vectors_in_plane, extents_in_plane, nbins_in_plane)
+        self.view.plot_cut_representation(*self.get_cut_representation_parameters(vectors_in_plane, extents_in_plane, nbins_in_plane))
         self.update_cut()
 
     # signals
@@ -83,3 +104,9 @@ def on_slicepoint_changed(self):
 
     def on_cut_done(self, wsname):
         self.view.plot_cut_ws(wsname)
+
+    def handle_cell_changed(self, irow: int, icol: int):
+        vectors, extents, nbins = self.validate_bin_params(irow, icol)
+        self.view.set_bin_params(vectors, extents, nbins)
+        self.update_cut()
+        self.view.plot_cut_representation(*self.get_cut_representation_parameters(vectors[:-1, :], extents[:, :-1], nbins[:-1]))

qt/python/mantidqt/mantidqt/widgets/sliceviewer/cutviewer/test/test_cutviewer_model.py

@@ -77,7 +77,7 @@ def test_validate_step_when_noninteger_nbins_in_extents(self):
 
     def test_calc_cut_representation_parameters(self):
         xmin, xmax, ymin, ymax, thick = self.model.calc_cut_representation_parameters(
-            eye(3), tile(c_[[0.0, 1.0]], (1, 3)), array([10, 1, 1]), [0, 1, None]
+            eye(3)[:-1, :], tile(c_[[0.0, 1.0]], (1, 2)), array([10, 1]), [0, 1, None]
         )
 
         self.assertTrue(array_equal(self.model.xvec, array([1, 0, 0])))

qt/python/mantidqt/mantidqt/widgets/sliceviewer/cutviewer/test/test_cutviewer_presenter.py

@@ -8,39 +8,53 @@
 #
 import unittest
 from unittest import mock
-from numpy import eye, c_, array, tile, array_equal
+from numpy import eye, c_, array, tile, array_equal, arange
 
 from mantidqt.widgets.sliceviewer.cutviewer.presenter import CutViewerPresenter
+from mantidqt.widgets.sliceviewer.cutviewer.view import CutViewerView
+from mantidqt.widgets.sliceviewer.cutviewer.model import CutViewerModel
 from mantidqt.widgets.sliceviewer.presenters.presenter import SliceViewer
 
 
 class TestCutViewerModel(unittest.TestCase):
-    @mock.patch("mantidqt.widgets.sliceviewer.cutviewer.presenter.CutViewerModel", autospec=True)
-    @mock.patch("mantidqt.widgets.sliceviewer.cutviewer.presenter.CutViewerView", autospec=True)
-    def setUp(self, mock_view, mock_model):
+    def setUp(self):
         # load empty instrument so can create a peak table
+        mock_view = mock.create_autospec(CutViewerView)
+        mock_model = mock.create_autospec(CutViewerModel)
         self.mock_sv_presenter = mock.create_autospec(SliceViewer)
-        self.presenter = CutViewerPresenter(self.mock_sv_presenter, canvas=mock.MagicMock())
+        self.presenter = CutViewerPresenter(self.mock_sv_presenter, mock_model, mock_view)
 
     def test_on_cut_done(self):
         self.presenter.on_cut_done("wsname")
         self.presenter.view.plot_cut_ws.assert_called_once_with("wsname")
 
+    @mock.patch("mantidqt.widgets.sliceviewer.cutviewer.presenter.CutViewerPresenter.get_cut_representation_parameters")
     @mock.patch("mantidqt.widgets.sliceviewer.cutviewer.presenter.CutViewerPresenter.update_cut")
-    def test_show_view(self, mock_update_cut):
-        self.presenter.model.get_default_bin_params.return_value = 3 * [None]  # vecs, extents, nbins
+    def test_show_view(self, mock_update_cut, mock_get_cut_rep):
+        mock_get_cut_rep.return_value = 6 * [None]  # xmin, xmax, ymin, ymax, thickness, axes_transform
+        in_vecs = eye(3)
+        in_extents = arange(6).reshape(2, 3)
+        in_nbins = array([10, 1, 1])
+        self.presenter.model.get_default_bin_params.return_value = in_vecs, in_extents, in_nbins  # vecs, extents, nbins
 
         self.presenter.show_view()
 
         self.presenter.view.show.assert_called_once()
-        self.presenter.view.set_bin_params.assert_called_once()
+        self.presenter.view.set_bin_params.assert_called_once_with(in_vecs, in_extents, in_nbins)
         mock_update_cut.assert_called_once()
+        mock_get_cut_rep.assert_called_once()
+        args = mock_get_cut_rep.call_args[0]  # for some reason assert_called_once_with struggles wth np arrays here
+        self.assertTrue(array_equal(in_vecs[:-1, :], args[0]))  # only has 2D slice
+        self.assertTrue(array_equal(in_extents[:, :-1], args[1]))
+        self.assertTrue(array_equal(in_nbins[:-1], args[2]))
 
     def test_update_cut_with_valid_bin_params(self):
         in_vecs = eye(3)
         in_extents = tile(c_[[0.0, 1.0]], (1, 3))
         in_nbins = array([10, 1, 1])
-        self.presenter.view.get_bin_params.return_value = (in_vecs, in_extents, in_nbins)
+        self.presenter.view.get_vector.side_effect = lambda irow: in_vecs[irow, :]
+        self.presenter.view.get_extents.side_effect = lambda irow: in_extents[:, irow]
+        self.presenter.view.get_nbin.side_effect = lambda irow: in_nbins[irow]
         self.presenter.model.valid_bin_params.return_value = True
 
         self.presenter.update_cut()
@@ -50,21 +64,26 @@ def test_update_cut_with_valid_bin_params(self):
         self.assertTrue(array_equal(in_extents.flatten(order="F"), out_extents))
         self.assertTrue(array_equal(in_nbins, out_nbins))
 
-    def test_update_cut_with_invalid_bin_params(self):
-        self.presenter.view.get_bin_params.return_value = 3 * [None]
+    @mock.patch("mantidqt.widgets.sliceviewer.cutviewer.presenter.CutViewerPresenter.get_bin_params_from_view")
+    def test_update_cut_with_invalid_bin_params(self, mock_get_bin_params):
+        mock_get_bin_params.return_value = 3 * [None]
         self.presenter.model.valid_bin_params.return_value = False
 
         self.presenter.update_cut()
 
         self.mock_sv_presenter.perform_non_axis_aligned_cut.assert_not_called()
 
     @mock.patch("mantidqt.widgets.sliceviewer.cutviewer.presenter.CutViewerPresenter.update_cut")
-    def test_update_bin_params_from_cut_representation(self, mock_update_cut):
+    def test_handle_cut_representation_changed(self, mock_update_cut):
         xmin, xmax, ymin, ymax, thickness = 0, 1, 0, 1, 0.1
         self.presenter.model.calc_bin_params_from_cut_representation.return_value = 3 * [None]  # vecs, extents, nbins
-        self.presenter.view.get_bin_params.return_value = (eye(3), "ignored", "ignored")  # vecs, extents, nbins
+        self.presenter.model.calc_cut_representation_parameters.return_value = xmin, xmax, ymin, ymax, thickness
+        in_vecs = eye(3)
+        self.presenter.view.get_vector.side_effect = lambda irow: in_vecs[irow, :]
+        self.presenter.view.get_extents.return_value = [-1, 1]  # ignored
+        self.presenter.view.get_nbin.return_value = 2  # ignored
 
-        self.presenter.update_bin_params_from_cut_representation(xmin, xmax, ymin, ymax, thickness)
+        self.presenter.handle_cut_representation_changed(xmin, xmax, ymin, ymax, thickness)
 
         mock_update_cut.assert_called_once()
         # check last vector passed as out of plane vector

qt/python/mantidqt/mantidqt/widgets/sliceviewer/cutviewer/view.py

@@ -15,7 +15,11 @@
 import matplotlib.text as text
 from mantid.simpleapi import AnalysisDataService as ADS
 from mantid.kernel import SpecialCoordinateSystem
-from numpy import zeros
+from typing import TYPE_CHECKING
+
+if TYPE_CHECKING:
+    from mantidqt.widgets.sliceviewer.cutviewer.presenter import CutViewerPresenter  # noqa: F401
+    from workbench.plotting.mantidfigurecanvas import MantidFigureCanvas  # noqa: F401
 
 # local imports
 from .representation.cut_representation import CutRepresentation
@@ -26,14 +30,14 @@ class CutViewerView(QWidget):
     to interact with the peaks.
     """
 
-    def __init__(self, presenter, canvas, frame, parent=None):
+    def __init__(self, canvas: "SliceViewerCanvas", frame: SpecialCoordinateSystem):
         """
         :param painter: An object responsible for drawing the representation of the cut
         :param sliceinfo_provider: An object responsible for providing access to current slice information
         :param parent: An optional parent widget
         """
-        super().__init__(parent)
-        self.presenter = presenter
+        super().__init__()
+        self.presenter = None
         self.layout = None
         self.figure_layout = None
         self.table = None
@@ -45,6 +49,9 @@ def __init__(self, presenter, canvas, frame, parent=None):
         self._init_slice_table()
         self.table.cellChanged.connect(self.on_cell_changed)
 
+    def subscribe_presenter(self, presenter: "CutViewPresenter"):
+        self.presenter = presenter
+
     def hide(self):
         super().hide()
         if self.cut_rep is not None:
@@ -54,8 +61,7 @@ def hide(self):
     # signals
 
     def on_cell_changed(self, irow, icol):
-        self.set_bin_params(*self.presenter.validate_bin_params(irow, icol))
-        self.presenter.update_cut()
+        self.presenter.handle_cell_changed(irow, icol)
 
     def resizeEvent(self, event):
         super().resizeEvent(event)
@@ -66,17 +72,14 @@ def resizeEvent(self, event):
     def get_step(self, irow):
         return float(self.table.item(irow, 6).text())
 
-    def get_bin_params(self):
-        vectors = zeros((3, 3), dtype=float)
-        extents = zeros((2, 3), dtype=float)
-        nbins = zeros(3, dtype=int)
-        for ivec in range(vectors.shape[0]):
-            for icol in range(vectors.shape[0]):
-                vectors[ivec, icol] = float(self.table.item(ivec, icol).text())
-            extents[0, ivec] = float(self.table.item(ivec, 3).text())  # start
-            extents[1, ivec] = float(self.table.item(ivec, 4).text())  # stop
-            nbins[ivec] = int(self.table.item(ivec, 5).text())
-        return vectors, extents, nbins
+    def get_nbin(self, irow):
+        return int(self.table.item(irow, 5).text())
+
+    def get_extents(self, irow):
+        return [float(self.table.item(irow, icol).text()) for icol in (3, 4)]  # start, stop
+
+    def get_vector(self, irow):
+        return [float(self.table.item(irow, icol).text()) for icol in range(3)]
 
     # setters
 
@@ -93,13 +96,13 @@ def set_extent(self, irow, start=None, stop=None):
     def set_step(self, irow, step):
         self.table.item(irow, 6).setData(Qt.EditRole, float(step))
 
-    def update_step(self, irow):
-        _, extents, nbins = self.get_bin_params()
-        self.set_step(irow, (extents[1, irow] - extents[0, irow]) / nbins[irow])
+    def update_step(self, irow, nbin):
+        extents = self.get_extents(irow)
+        self.set_step(irow, (extents[1] - extents[0]) / nbin)
 
     def set_nbin(self, irow, nbin):
         self.table.item(irow, 5).setData(Qt.EditRole, int(nbin))
-        self.update_step(irow)
+        self.update_step(irow, nbin)
 
     def set_bin_params(self, vectors, extents, nbins):
         self.table.blockSignals(True)
@@ -108,8 +111,6 @@ def set_bin_params(self, vectors, extents, nbins):
             self.set_extent(irow, *extents[:, irow])
             self.set_nbin(irow, nbins[irow])  # do this last as step automatically updated given extents
         self.table.blockSignals(False)
-        self.plot_cut_representation()
-        return vectors, extents, nbins
 
     def set_slicepoint(self, slicept, width):
         self.table.blockSignals(True)
@@ -125,12 +126,13 @@ def plot_cut_ws(self, wsname):
         self._format_cut_figure()
         self.figure.canvas.draw()
 
-    def plot_cut_representation(self):
+    def plot_cut_representation(self, xmin, xmax, ymin, ymax, thickness, axes_transform):
         if self.cut_rep is not None:
             self.cut_rep.remove()
-        self.cut_rep = CutRepresentation(
-            self.canvas, self.presenter.update_bin_params_from_cut_representation, *self.presenter.get_cut_representation_parameters()
-        )
+        self.cut_rep = CutRepresentation(self.canvas, self.on_representation_changed, xmin, xmax, ymin, ymax, thickness, axes_transform)
+
+    def on_representation_changed(self, xmin, xmax, ymin, ymax, thickness):
+        self.presenter.handle_cut_representation_changed(xmin, xmax, ymin, ymax, thickness)
 
     # private api
     def _setup_ui(self):