Commit 57c434a8 authored by Owen Arnold's avatar Owen Arnold
Browse files

refs #23706. Fix iterator advance via custom next

Also set return value policy to circumvent issue found in ScriptWindow
parent ba418a60
......@@ -87,12 +87,6 @@ private:
SpectrumInfoItem(const SpectrumInfo &spectrumInfo, const size_t index)
: m_spectrumInfo(&spectrumInfo), m_index(index) {}
// Provide copy and move constructors
SpectrumInfoItem(const SpectrumInfoItem &other) = default;
SpectrumInfoItem &operator=(const SpectrumInfoItem &rhs) = default;
SpectrumInfoItem(SpectrumInfoItem &&other) = default;
SpectrumInfoItem &operator=(SpectrumInfoItem &&rhs) = default;
// Non-owning pointer. A reference makes the class unable to define an
// assignment operator that we need.
const SpectrumInfo *m_spectrumInfo;
......
......@@ -74,12 +74,6 @@ private:
DetectorInfoItem(const DetectorInfo &detectorInfo, const size_t index)
: m_detectorInfo(&detectorInfo), m_index(index) {}
// Provide copy and move constructors
DetectorInfoItem(const DetectorInfoItem &other) = default;
DetectorInfoItem &operator=(const DetectorInfoItem &rhs) = default;
DetectorInfoItem(DetectorInfoItem &&other) = default;
DetectorInfoItem &operator=(DetectorInfoItem &&rhs) = default;
// Non-owning pointer. A reference makes the class unable to define an
// assignment operator that we need.
const DetectorInfo *m_detectorInfo;
......
......@@ -58,13 +58,14 @@ public:
m_firstOrDone(true) {}
const DetectorInfoItem &next() {
if (!m_firstOrDone) {
if (!m_firstOrDone)
++m_begin;
} else {
else
m_firstOrDone = false;
if (m_begin == m_end) {
m_firstOrDone = true;
objects::stop_iteration_error();
}
return *m_begin;
}
......
......@@ -62,10 +62,12 @@ public:
m_firstOrDone(true) {}
const SpectrumInfoItem &next() {
if (!m_firstOrDone) {
if (!m_firstOrDone)
++m_begin;
} else {
else
m_firstOrDone = false;
if (m_begin == m_end) {
m_firstOrDone = true;
objects::stop_iteration_error();
}
return *m_begin;
......
......@@ -12,7 +12,7 @@ using namespace boost::python;
void export_SpectrumInfoItem() {
// Export to Python
class_<SpectrumInfoItem, boost::noncopyable>("SpectrumInfoItem", no_init)
class_<SpectrumInfoItem>("SpectrumInfoItem", no_init)
.add_property("isMonitor", &SpectrumInfoItem::isMonitor)
.add_property("isMasked", &SpectrumInfoItem::isMasked)
.add_property("twoTheta", &SpectrumInfoItem::twoTheta)
......
......@@ -22,5 +22,14 @@ void export_SpectrumInfoPythonIterator() {
#endif
,
&SpectrumInfoPythonIterator::next,
return_value_policy<reference_existing_object>());
return_value_policy<copy_const_reference>());
/*
Return value policy for next is to copy the const reference. Copy by value is
essential for python 2.0 compatibility because items (SpectrumInfoItem) will
outlive their iterators if declared as part of for loops. There is no good
way to deal with this other than to force a copy so that internals of the
item are not also corrupted. Future developers may wish to choose a separte
policy for python 3.0 where this is not a concern, and const ref returns
would be faster.
*/
}
......@@ -12,7 +12,7 @@ using namespace boost::python;
void export_DetectorInfoItem() {
// Export to Python
class_<DetectorInfoItem, boost::noncopyable>("DetectorInfoItem", no_init)
class_<DetectorInfoItem>("DetectorInfoItem", no_init)
.add_property("isMonitor", &DetectorInfoItem::isMonitor)
.add_property("isMasked", &DetectorInfoItem::isMasked)
.add_property("twoTheta", &DetectorInfoItem::twoTheta)
......
......@@ -22,5 +22,14 @@ void export_DetectorInfoPythonIterator() {
#endif
,
&DetectorInfoPythonIterator::next,
return_value_policy<reference_existing_object>());
return_value_policy<copy_const_reference>());
/*
Return value policy for next is to copy the const reference. Copy by value is
essential for python 2.0 compatibility because items (DetectorInfoItem) will
outlive their iterators if declared as part of for loops. There is no good
way to deal with this other than to force a copy so that internals of the
item are not also corrupted. Future developers may wish to choose a separte
policy for python 3.0 where this is not a concern, and const ref returns
would be faster.
*/
}
......@@ -12,7 +12,7 @@ class SpectrumInfoTest(unittest.TestCase):
def setUp(self):
""" Set up code. """
if self.__class__._ws is None:
self.__class__._ws = WorkspaceCreationHelper.create2DWorkspaceWithFullInstrument(2, 1, False) # no monitors
self.__class__._ws = WorkspaceCreationHelper.create2DWorkspaceWithFullInstrument(3, 1, False) # no monitors
self.__class__._ws.getSpectrum(0).clearDetectorIDs()
"""
......@@ -26,13 +26,13 @@ class SpectrumInfoTest(unittest.TestCase):
""" Check that the number of spectra we initailly created
is the same as in memory. """
info = self._ws.spectrumInfo()
self.assertEquals(len(info), 2)
self.assertEquals(len(info), 3)
def test_size(self):
""" Check that the number of spectra we initailly created
is the same as in memory. """
info = self._ws.spectrumInfo()
self.assertEquals(info.size(), 2)
self.assertEquals(info.size(), 3)
def test_isMonitor(self):
""" Check if a monitor is present. """
......@@ -113,53 +113,41 @@ class SpectrumInfoTest(unittest.TestCase):
Iteration
---------------
"""
def test_iteration_for_isMonitor(self):
info = self._ws.spectrumInfo()
for specInfo in info:
if specInfo.hasUniqueDetector:
self.assertEquals(type(specInfo.isMonitor), bool)
def test_iteration_for_isMasked(self):
info = self._ws.spectrumInfo()
for specInfo in info:
if specInfo.hasUniqueDetector:
self.assertEquals(type(specInfo.isMasked), bool)
def test_iteration_for_twoTheta(self):
def test_basic_iteration(self):
info = self._ws.spectrumInfo()
for specInfo in info:
if specInfo.hasUniqueDetector:
self.assertEquals(type(specInfo.twoTheta), float)
expected_iterations = len(info)
actual_iterations = len(list(iter(info)))
self.assertEquals(expected_iterations, actual_iterations)
def test_iteration_for_signedTwoTheta(self):
def test_iterator_for_monitors(self):
info = self._ws.spectrumInfo()
for specInfo in info:
if specInfo.hasUniqueDetector:
self.assertEquals(type(specInfo.signedTwoTheta), float)
def test_iteration_for_l2(self):
info = self._ws.spectrumInfo()
for specInfo in info:
if specInfo.hasUniqueDetector:
self.assertEquals(type(specInfo.l2), float)
def test_iteration_for_hasUniqueDetector(self):
info = self._ws.spectrumInfo()
for specInfo in info:
self.assertEquals(type(specInfo.hasUniqueDetector), bool)
def test_iteration_for_spectrumDefinition(self):
# check no monitors in instrument
it = iter(info)
next(it) # skip first as detectors cleared
for item in it:
self.assertFalse(item.isMonitor)
def test_iterator_for_masked(self):
info = self._ws.spectrumInfo()
for specInfo in info:
if specInfo.hasUniqueDetector:
self.assertEquals(type(specInfo.spectrumDefinition), SpectrumDefinition)
# nothing should be masked
it = iter(info)
next(it) # skip first as detectors cleared
for item in it:
self.assertFalse(item.isMasked)
def test_iteration_for_position(self):
info = self._ws.spectrumInfo()
for specInfo in info:
if specInfo.hasUniqueDetector:
self.assertEquals(type(specInfo.position), V3D)
lastY = None
it = iter(info)
next(it) # skip first as detectors cleared
for i,item in enumerate(it):
pos = item.position
# See test helper for position construction
self.assertAlmostEquals(pos.X(), 0)
self.assertAlmostEquals(pos.Z(), 5)
if(lastY):
self.assertTrue(pos.Y() > lastY)
lastY = pos.Y()
"""
----------------------------------------------------------------------------
......
......@@ -106,30 +106,35 @@ class DetectorInfoTest(unittest.TestCase):
---------------
"""
def test_iteration_for_isMonitor(self):
def test_basic_iteration(self):
info = self._ws.detectorInfo()
for detInfo in info:
self.assertEquals(type(detInfo.isMonitor), bool)
expected_iterations = len(info)
actual_iterations = len(list(iter(info)))
self.assertEquals(expected_iterations, actual_iterations)
def test_iteration_for_isMasked(self):
def test_iterator_for_monitors(self):
info = self._ws.detectorInfo()
for detInfo in info:
self.assertEquals(type(detInfo.isMasked), bool)
def test_iteration_for_twoTheta(self):
# check no monitors in instrument
for item in info:
self.assertFalse(item.isMonitor)
def test_iterator_for_masked(self):
info = self._ws.detectorInfo()
for detInfo in info:
self.assertEquals(type(detInfo.twoTheta), float)
# nothing should be masked
for item in info:
self.assertFalse(item.isMasked)
def test_iteration_for_position(self):
info = self._ws.detectorInfo()
for detInfo in info:
self.assertEquals(type(detInfo.position), V3D)
def test_iteration_for_rotation(self):
info = self._ws.detectorInfo()
for detInfo in info:
self.assertEquals(type(detInfo.rotation), Quat)
lastY = None
for i,item in enumerate(info):
pos = item.position
# See test helper for position construction
self.assertAlmostEquals(pos.X(), 0)
self.assertAlmostEquals(pos.Z(), 5)
if(lastY):
self.assertTrue(pos.Y() > lastY)
lastY = pos.Y()
"""
......
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