Unverified Commit 40f64d10 authored by Gemma Guest's avatar Gemma Guest Committed by GitHub
Browse files

Merge pull request #30079 from mantidproject/30071_FixRaggedWorkspaceCrash

Fix Double Click of Ragged workspaces
parents 50a8e570 771bd799
......@@ -135,6 +135,10 @@ public:
/// This throws an exception if the lengths are not identical across the
/// spectra.
virtual std::size_t blocksize() const = 0;
/// Returns the number of bins for a given histogram index.
virtual std::size_t getNumberBins(const std::size_t &index) const = 0;
/// Returns the maximum number of bins in a workspace (works on ragged data).
virtual std::size_t getMaxNumberBins() const = 0;
/// Returns the number of histograms in the workspace
virtual std::size_t getNumberHistograms() const = 0;
......
......@@ -50,6 +50,13 @@ public:
std::size_t size() const override { return m_spectra.size() * m_blocksize; }
/// Returns the size of each block of data returned by the dataY accessors
std::size_t blocksize() const override { return m_blocksize; }
/// Returns the number of bins for a given histogram index.
std::size_t getNumberBins(const std::size_t &index) const override {
UNUSED_ARG(index);
return m_blocksize;
}
/// Returns the maximum number of bins in a workspace (works on ragged data).
std::size_t getMaxNumberBins() const override { return m_blocksize; }
/// Returns the number of histograms in the workspace
std::size_t getNumberHistograms() const override { return m_spectra.size(); }
......
......@@ -102,6 +102,8 @@ public:
MOCK_CONST_METHOD0(clearMRU, void());
MOCK_CONST_METHOD0(isRaggedWorkspace, bool());
MOCK_CONST_METHOD0(blocksize, std::size_t());
MOCK_CONST_METHOD1(getNumberBins, std::size_t(const std::size_t &));
MOCK_CONST_METHOD0(getMaxNumberBins, std::size_t());
MOCK_CONST_METHOD0(size, std::size_t());
MOCK_CONST_METHOD0(getNumberHistograms, std::size_t());
MOCK_METHOD1(getSpectrum, Mantid::API::IEventList &(const std::size_t));
......
......@@ -68,12 +68,16 @@ public:
// Get the blocksize, aka the number of bins in the histogram
std::size_t blocksize() const override;
size_t getMemorySize() const override;
/// Returns the number of bins for a given histogram index.
std::size_t getNumberBins(const std::size_t &index) const override;
/// Returns the maximum number of bins in a workspace (works on ragged data).
std::size_t getMaxNumberBins() const override;
// Get the number of histograms. aka the number of pixels or detectors.
std::size_t getNumberHistograms() const override;
size_t getMemorySize() const override;
EventList &getSpectrum(const size_t index) override {
invalidateCommonBinsFlag();
return getSpectrumWithoutInvalidation(index);
......
......@@ -51,12 +51,17 @@ public:
/// Returns true if the workspace is ragged (has differently sized spectra).
bool isRaggedWorkspace() const override;
/// Returns the histogram number
std::size_t getNumberHistograms() const override;
// section required for iteration
std::size_t size() const override;
std::size_t blocksize() const override;
/// Returns the number of bins for a given histogram index.
std::size_t getNumberBins(const std::size_t &index) const override;
/// Returns the maximum number of bins in a workspace (works on ragged data).
std::size_t getMaxNumberBins() const override;
/// Returns the histogram number
std::size_t getNumberHistograms() const override;
Histogram1D &getSpectrum(const size_t index) override {
invalidateCommonBinsFlag();
......@@ -109,4 +114,4 @@ private:
virtual std::size_t getHistogramNumberHelper() const;
};
} // namespace DataObjects
} // Namespace Mantid
\ No newline at end of file
} // Namespace Mantid
......@@ -47,6 +47,13 @@ public:
/// Returns the size of each block of data returned by the dataX accessors
std::size_t blocksize() const override { return 1; }
/// Returns the number of bins for a given histogram index.
std::size_t getNumberBins(const std::size_t &index) const override {
UNUSED_ARG(index);
return 1;
}
/// Returns the maximum number of bins in a workspace.
std::size_t getMaxNumberBins() const override { return 1; }
/// @return the number of histograms (spectra)
std::size_t getNumberHistograms() const override { return 1; }
......
......@@ -178,6 +178,36 @@ size_t EventWorkspace::blocksize() const {
}
}
/** Returns the number of bins for a given histogram index.
* @param index :: The histogram index to check for the number of bins.
* @return the number of bins for a given histogram index.
*/
std::size_t EventWorkspace::getNumberBins(const std::size_t &index) const {
if (index < data.size())
return data[index]->histogram_size();
throw std::invalid_argument(
"Could not find number of bins in a histogram at index " +
std::to_string(index) + ": index is too large.");
}
/** Returns the maximum number of bins in a workspace (works on ragged data).
* @return the maximum number of bins in a workspace.
*/
std::size_t EventWorkspace::getMaxNumberBins() const {
if (data.empty()) {
return 0;
} else {
auto maxNumberOfBins = data[0]->histogram_size();
for (const auto &iter : data) {
const auto numberOfBins = iter->histogram_size();
if (numberOfBins > maxNumberOfBins)
maxNumberOfBins = numberOfBins;
}
return maxNumberOfBins;
}
}
/** Get the number of histograms, usually the same as the number of pixels or
detectors.
@returns the number of histograms / event lists
......
......@@ -149,6 +149,36 @@ size_t Workspace2D::blocksize() const {
}
}
/** Returns the number of bins for a given histogram index.
* @param index :: The histogram index to check for the number of bins.
* @return the number of bins for a given histogram index.
*/
std::size_t Workspace2D::getNumberBins(const std::size_t &index) const {
if (index < data.size())
return data[index]->size();
throw std::invalid_argument(
"Could not find number of bins in a histogram at index " +
std::to_string(index) + ": index is too large.");
}
/** Returns the maximum number of bins in a workspace (works on ragged data).
* @return the maximum number of bins in a workspace.
*/
std::size_t Workspace2D::getMaxNumberBins() const {
if (data.empty()) {
return 0;
} else {
auto maxNumberOfBins = data[0]->size();
for (const auto &iter : data) {
const auto numberOfBins = iter->size();
if (numberOfBins > maxNumberOfBins)
maxNumberOfBins = numberOfBins;
}
return maxNumberOfBins;
}
}
/**
* Copy the data (Y's) from an image to this workspace.
* @param image :: An image to copy the data from.
......
......@@ -163,6 +163,35 @@ public:
TS_ASSERT_THROWS(ew->blocksize(), const std::logic_error &);
}
void
test_that_getNumberBins_returns_the_correct_number_of_bins_for_different_histograms_in_a_ragged_EventWorkspace() {
ew = createEventWorkspace(true, false);
ew->getSpectrum(0).setHistogram(BinEdges({0., 10., 20.}));
TS_ASSERT(ew->isRaggedWorkspace());
TS_ASSERT_EQUALS(ew->getNumberBins(0), 2);
TS_ASSERT_EQUALS(ew->getNumberBins(1), 1);
}
void
test_that_getNumberBins_throws_when_provided_an_index_which_is_too_large() {
ew = createEventWorkspace(true, false);
const auto numberOfHistograms = ew->getNumberHistograms();
TS_ASSERT_THROWS_NOTHING(ew->getNumberBins(numberOfHistograms - 1u));
TS_ASSERT_THROWS(ew->getNumberBins(numberOfHistograms),
const std::invalid_argument &);
}
void
test_that_getMaxNumberBins_returns_the_correct_number_for_a_ragged_EventWorkspace() {
ew = createEventWorkspace(true, false);
ew->getSpectrum(0).setHistogram(BinEdges({0., 10., 20.}));
TS_ASSERT(ew->isRaggedWorkspace());
TS_ASSERT_EQUALS(ew->getMaxNumberBins(), 2);
}
void testUnequalBins() {
ew = createEventWorkspace(true, false);
// normal behavior
......
......@@ -94,6 +94,34 @@ public:
TS_ASSERT_THROWS(cloned->blocksize(), const std::logic_error &);
}
void
test_that_getNumberBins_returns_the_correct_number_of_bins_for_different_histograms_in_a_ragged_Workspace2D() {
Workspace2D_sptr cloned(ws->clone());
cloned->setHistogram(0, Points(0), Counts(0));
TS_ASSERT(cloned->isRaggedWorkspace());
TS_ASSERT_EQUALS(cloned->getNumberBins(0), 0);
TS_ASSERT_EQUALS(cloned->getNumberBins(1), 5);
}
void
test_that_getNumberBins_throws_when_provided_an_index_which_is_too_large() {
const auto numberOfHistograms = ws->getNumberHistograms();
TS_ASSERT_THROWS_NOTHING(ws->getNumberBins(numberOfHistograms - 1));
TS_ASSERT_THROWS(ws->getNumberBins(numberOfHistograms),
const std::invalid_argument &);
}
void
test_that_getMaxNumberBins_returns_the_correct_number_for_a_ragged_Workspace2D() {
Workspace2D_sptr cloned(ws->clone());
cloned->setHistogram(0, Points(0), Counts(0));
TS_ASSERT(cloned->isRaggedWorkspace());
TS_ASSERT_EQUALS(cloned->getMaxNumberBins(), 5);
}
void testUnequalBins() {
// try normal kind first
TS_ASSERT_EQUALS(ws->blocksize(), 5);
......
......@@ -105,4 +105,11 @@ public:
WorkspaceSingleValue ws;
TS_ASSERT(!ws.isRaggedWorkspace());
}
void
test_that_getNumberBins_and_getMaxNumberBins_returns_one_for_a_WorkspaceSingleValue() {
WorkspaceSingleValue ws;
TS_ASSERT_EQUALS(ws.getNumberBins(0), 1);
TS_ASSERT_EQUALS(ws.getMaxNumberBins(), 1);
}
};
......@@ -184,18 +184,6 @@ void setDxFromPyObject(MatrixWorkspace &self, const size_t wsIndex,
setSpectrumFromPyObject(self, &MatrixWorkspace::dataDx, wsIndex, values);
}
/**
* Adds a deprecation warning to the getNumberBins call to warn about using
* blocksize instead
* @param self A reference to the calling object
* @returns The blocksize()
*/
size_t getNumberBinsDeprecated(MatrixWorkspace &self) {
PyErr_Warn(PyExc_DeprecationWarning,
"``getNumberBins`` is deprecated, use ``blocksize`` instead.");
return self.blocksize();
}
/**
* Adds a deprecation warning to the getSampleDetails call to warn about using
* getRun instead
......@@ -350,6 +338,12 @@ void export_MatrixWorkspace() {
"spectra).")
.def("blocksize", &MatrixWorkspace::blocksize, arg("self"),
"Returns size of the Y data array")
.def("getNumberBins", &MatrixWorkspace::getNumberBins,
(arg("self"), arg("index")),
"Returns the number of bins for a given histogram index.")
.def("getMaxNumberBins", &MatrixWorkspace::getMaxNumberBins, arg("self"),
"Returns the maximum number of bins in a workspace (works on ragged "
"data).")
.def("getNumberHistograms", &MatrixWorkspace::getNumberHistograms,
arg("self"), "Returns the number of spectra in the workspace")
.def("getSpectrumNumbers", &getSpectrumNumbers, arg("self"),
......@@ -424,11 +418,6 @@ void export_MatrixWorkspace() {
"Find first index in Y equal to value. Start may be specified to "
"begin at a specifc index. Returns tuple with the "
"histogram and bin indices.")
// Deprecated
.def("getNumberBins", &getNumberBinsDeprecated, arg("self"),
"Returns size of the Y data array (deprecated, use "
":class:`~mantid.api.MatrixWorkspace.blocksize` "
"instead)")
.def("getSampleDetails", &getSampleDetailsDeprecated, arg("self"),
return_internal_reference<>(),
"Return the Run object for this workspace (deprecated, use "
......
......@@ -79,7 +79,7 @@ class PearlMCAbsorption(PythonAlgorithm):
"""
#c = math.exp(-1.0*mu*thickness)
num_hist = input_ws.getNumberHistograms()
num_vals = input_ws.getNumberBins()
num_vals = input_ws.blocksize()
for i in range(num_hist):
mu_values = input_ws.readY(i)
for j in range(num_vals):
......
......@@ -172,7 +172,7 @@ class LoadDNSLegacyTest(unittest.TestCase):
ws = AnalysisDataService.retrieve(outputWorkspaceName)
# dimensions
self.assertEqual(24, ws.getNumberHistograms())
self.assertEqual(100, ws.getNumberBins())
self.assertEqual(100, ws.blocksize())
# data array
self.assertEqual(8, ws.readY(19)[23])
self.assertAlmostEqual(tof1, ws.readX(0)[0], 3)
......
......@@ -31,31 +31,31 @@ class LoadGudrunOutputTest(unittest.TestCase):
def test_load_dcs(self):
actual = LoadGudrunOutput(self.file_name.format('.dcs'))
self.assertIsInstance(actual, Workspace)
self.assertEqual(actual.getNumberBins(), 100)
self.assertEqual(actual.blocksize(), 100)
self.assertEqual(actual.getNumberHistograms(), 5)
def test_load_mdsc(self):
actual = LoadGudrunOutput(self.file_name.format('.mdcs'))
self.assertIsInstance(actual, Workspace)
self.assertEqual(actual.getNumberBins(), 100)
self.assertEqual(actual.blocksize(), 100)
self.assertEqual(actual.getNumberHistograms(), 1)
def test_load_mint(self):
actual = LoadGudrunOutput(self.file_name.format('.mint'))
self.assertIsInstance(actual, Workspace)
self.assertEqual(actual.getNumberBins(), 100)
self.assertEqual(actual.blocksize(), 100)
self.assertEqual(actual.getNumberHistograms(), 1)
def test_load_mdor(self):
actual = LoadGudrunOutput(self.file_name.format('.mdor'))
self.assertIsInstance(actual, Workspace)
self.assertEqual(actual.getNumberBins(), 100)
self.assertEqual(actual.blocksize(), 100)
self.assertEqual(actual.getNumberHistograms(), 1)
def test_load_mgor(self):
actual = LoadGudrunOutput(self.file_name.format('.mgor'))
self.assertIsInstance(actual, Workspace)
self.assertEqual(actual.getNumberBins(), 100)
self.assertEqual(actual.blocksize(), 100)
self.assertEqual(actual.getNumberHistograms(), 1)
def test_one_column_data_file(self):
......
......@@ -23,6 +23,7 @@
* Author: Janik Zikovsky
*/
#include <algorithm>
#include <fstream>
#include <map>
#include <string>
......@@ -166,6 +167,26 @@ public:
}
return m_vec.empty() ? 0 : m_vec[0].dataY().size();
}
std::size_t getNumberBins(const std::size_t &index) const override {
if (index > m_vec.size())
return 0;
return m_vec[index].dataY().size();
}
std::size_t getMaxNumberBins() const override {
if (m_vec.empty()) {
return 0;
} else {
const auto iter = std::max_element(
m_vec.cbegin(), m_vec.cend(),
[](const SpectrumTester &s1, const SpectrumTester &s2) {
return s1.dataY().size() < s2.dataY().size();
});
return iter->dataY().size();
}
}
ISpectrum &getSpectrum(const size_t index) override {
invalidateCommonBinsFlag();
m_vec[index].setMatrixWorkspace(this, index);
......
......@@ -259,7 +259,7 @@ class AbsorptionCompare(systemtesting.MantidSystemTest):
SetSample(InputWorkspace='V_abs',
Material={'ChemicalFormula': 'V', 'SampleNumberDensity': 0.0721},
Geometry={'Shape': 'Cylinder', 'Height': 6.97, 'Radius': (0.63 / 2), 'Center': [0., 0., 0.]})
self.assertEqual(absorptionWS.getNumberBins(), num_wl_bins)
self.assertEqual(absorptionWS.blocksize(), num_wl_bins)
# calculate the absorption
CylinderAbsorption(InputWorkspace='V_abs', OutputWorkspace='V_abs',
NumberOfSlices=20, NumberOfAnnuli=3)
......
......@@ -52,5 +52,6 @@ Bugfixes
- Fix sort order of peaks in the peaks overlay on SliceViewer.
- The colorfill plot on ragged workspaces will have the correct horizontal extent.
- Fix replot of colorfill image after a workspace is replaced.
- Fixed a bug causing an error when double clicking a ragged workspace.
:ref:`Release 6.0.0 <v6.0.0>`
......@@ -11,8 +11,9 @@ import unittest
from unittest import mock
import matplotlib as mpl
from mantid.simpleapi import (CreateEmptyTableWorkspace, CreateSampleWorkspace,
GroupWorkspaces, CreateSingleValuedWorkspace, CreateMDHistoWorkspace)
from mantid.api import AnalysisDataService
from mantid.simpleapi import (CreateEmptyTableWorkspace, CreateSampleWorkspace, CreateSingleValuedWorkspace,
CreateMDHistoWorkspace, CreateWorkspace, ConjoinWorkspaces, GroupWorkspaces)
from mantidqt.utils.qt.testing import start_qapplication
from mantidqt.utils.qt.testing.qt_widget_finder import QtWidgetFinder
from qtpy.QtWidgets import QMainWindow
......@@ -33,14 +34,22 @@ MATRIXWORKSPACE_DISPLAY_TYPE = "StatusBarView"
@start_qapplication
class WorkspaceWidgetTest(unittest.TestCase, QtWidgetFinder):
def setUp(self):
self.ws_widget = WorkspaceWidget(QMainWindow())
@classmethod
def setUpClass(cls):
cls.ws_widget = WorkspaceWidget(QMainWindow())
mat_ws = CreateSampleWorkspace()
table_ws = CreateEmptyTableWorkspace()
group_ws = GroupWorkspaces([mat_ws, table_ws])
single_val_ws = CreateSingleValuedWorkspace(5, 6)
self.w_spaces = [mat_ws, table_ws, group_ws, single_val_ws]
self.ws_names = ['MatWS', 'TableWS', 'GroupWS', 'SingleValWS']
# Create ragged workspace
ws2d_ragged = CreateWorkspace(DataX=[10, 20, 30], DataY=[1, 2, 3], NSpec=1, OutputWorkspace='Ragged')
temp = CreateWorkspace(DataX=[15, 25, 35, 45], DataY=[1, 2, 3, 4], NSpec=1)
ConjoinWorkspaces(ws2d_ragged, temp, CheckOverlapping=False)
ws2d_ragged = AnalysisDataService.retrieve('Ragged')
cls.w_spaces = [mat_ws, table_ws, group_ws, single_val_ws]
cls.ws_names = ['MatWS', 'TableWS', 'GroupWS', 'SingleValWS']
# create md workspace
md_ws = CreateMDHistoWorkspace(SignalInput='1,2,3,4,2,1',
ErrorInput='1,1,1,1,1,1',
......@@ -52,13 +61,16 @@ class WorkspaceWidgetTest(unittest.TestCase, QtWidgetFinder):
OutputWorkspace='MDHistoWS1D')
# self.w_spaces = [mat_ws, table_ws, group_ws, single_val_ws, md_ws]
# self.ws_names = ['MatWS', 'TableWS', 'GroupWS', 'SingleValWS', 'MDHistoWS1D']
for ws_name, ws in zip(self.ws_names, self.w_spaces):
self.ws_widget._ads.add(ws_name, ws)
self.ws_names.append(md_ws.name())
self.w_spaces.append(md_ws)
for ws_name, ws in zip(cls.ws_names, cls.w_spaces):
cls.ws_widget._ads.add(ws_name, ws)
cls.ws_names.append(md_ws.name())
cls.w_spaces.append(md_ws)
cls.ws_names.append(ws2d_ragged.name())
cls.w_spaces.append(ws2d_ragged)
def tearDown(self):
self.ws_widget._ads.clear()
@classmethod
def tearDownClass(cls):
cls.ws_widget._ads.clear()
def test_algorithm_history_window_opens_with_workspace(self):
with mock.patch(ALGORITHM_HISTORY_WINDOW + '.show', lambda x: None):
......@@ -72,14 +84,14 @@ class WorkspaceWidgetTest(unittest.TestCase, QtWidgetFinder):
def test_algorithm_history_window_opens_multiple(self):
"""
There are 5 objects in ADS. But 1 of them is WorkspaceGroup
This sets total number of history windows to 4.
There are 6 objects in ADS. But 1 of them is WorkspaceGroup
This sets total number of history windows to 5.
:return:
"""
with mock.patch(ALGORITHM_HISTORY_WINDOW + '.show', lambda x: None):
self.ws_widget._do_show_algorithm_history(self.ws_names)
self.assert_number_of_widgets_matching(ALGORITHM_HISTORY_WINDOW_TYPE, 4)
self.assert_number_of_widgets_matching(ALGORITHM_HISTORY_WINDOW_TYPE, 5)
def test_detector_table_shows_with_workspace(self):
with mock.patch(MATRIXWORKSPACE_DISPLAY + '.show_view', lambda x: None):
......@@ -88,6 +100,7 @@ class WorkspaceWidgetTest(unittest.TestCase, QtWidgetFinder):
@mock.patch('workbench.plugins.workspacewidget.plot', autospec=True)
def test_plot_with_plot_bin(self, mock_plot):
self.ws_widget._ads.add(self.ws_names[0], self.w_spaces[0])
self.ws_widget._do_plot_bin([self.ws_names[0]], False, False)
mock_plot.assert_called_once_with(mock.ANY, errors=False, overplot=False, wksp_indices=[0],
plot_kwargs={'axis': MantidAxType.BIN})
......@@ -132,6 +145,13 @@ class WorkspaceWidgetTest(unittest.TestCase, QtWidgetFinder):
self.ws_widget._action_double_click_workspace(self.ws_names[3])
self.assert_widget_type_exists(MATRIXWORKSPACE_DISPLAY_TYPE)
@mock.patch('workbench.plugins.workspacewidget.plot_from_names', autospec=True)
def test_double_click_with_ragged_ws_calls_plot_from_names(self, mock_plot_from_names):
self.assertTrue(self.w_spaces[5].isRaggedWorkspace())
self.ws_widget._action_double_click_workspace(self.ws_names[5])
mock_plot_from_names.assert_called_once_with([self.ws_names[5]], errors=False, overplot=False,
show_colorfill_btn=True)
def test_empty_workspaces(self):
self.ws_widget._ads.clear()
self.assertEqual(self.ws_widget.empty_of_workspaces(), True)
......
......@@ -289,7 +289,7 @@ class WorkspaceWidget(PluginWidget):
TableWorkspaceDisplay.supports(ws)
self._do_show_data([name])
except ValueError:
if hasattr(ws, 'blocksize') and ws.blocksize() == 1:
if hasattr(ws, 'getMaxNumberBins') and ws.getMaxNumberBins() == 1:
#If this is ws is just a single value show the data, else plot the bin
if hasattr(ws, 'getNumberHistograms') and ws.getNumberHistograms() == 1:
self._do_show_data([name])
......
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