diff --git a/Framework/API/inc/MantidAPI/SpectrumInfoItem.h b/Framework/API/inc/MantidAPI/SpectrumInfoItem.h index bf004db7c5a272f80c6e1c7c256c358c3258859e..53996e9cdd8592a302b057f18daed9c061927a7f 100644 --- a/Framework/API/inc/MantidAPI/SpectrumInfoItem.h +++ b/Framework/API/inc/MantidAPI/SpectrumInfoItem.h @@ -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; diff --git a/Framework/Geometry/inc/MantidGeometry/Instrument/DetectorInfoItem.h b/Framework/Geometry/inc/MantidGeometry/Instrument/DetectorInfoItem.h index c2581d5fa089f3892c43920d6d9517fb18e4c2a0..1a1f4777f9d287b142940ef2789b9c281f657eab 100644 --- a/Framework/Geometry/inc/MantidGeometry/Instrument/DetectorInfoItem.h +++ b/Framework/Geometry/inc/MantidGeometry/Instrument/DetectorInfoItem.h @@ -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; diff --git a/Framework/PythonInterface/inc/MantidPythonInterface/api/DetectorInfoPythonIterator.h b/Framework/PythonInterface/inc/MantidPythonInterface/api/DetectorInfoPythonIterator.h index e40a0d3629c362968d25514624e5cd7697ea49b3..c3b50aa1c55490a1d63a924ce0a68efebd011e81 100644 --- a/Framework/PythonInterface/inc/MantidPythonInterface/api/DetectorInfoPythonIterator.h +++ b/Framework/PythonInterface/inc/MantidPythonInterface/api/DetectorInfoPythonIterator.h @@ -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; } diff --git a/Framework/PythonInterface/inc/MantidPythonInterface/api/SpectrumInfoPythonIterator.h b/Framework/PythonInterface/inc/MantidPythonInterface/api/SpectrumInfoPythonIterator.h index 54771445fa07920390e0be298f7a1ca591b8fb26..20b6d5bd5c8513529f92b8b241092699fcf3df44 100644 --- a/Framework/PythonInterface/inc/MantidPythonInterface/api/SpectrumInfoPythonIterator.h +++ b/Framework/PythonInterface/inc/MantidPythonInterface/api/SpectrumInfoPythonIterator.h @@ -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; diff --git a/Framework/PythonInterface/mantid/api/src/Exports/SpectrumInfoItem.cpp b/Framework/PythonInterface/mantid/api/src/Exports/SpectrumInfoItem.cpp index ad5ac9a730a838def85020d1037ec186f6d88715..4adc923d18308673e3dac1c78d7bab2268de9e8a 100644 --- a/Framework/PythonInterface/mantid/api/src/Exports/SpectrumInfoItem.cpp +++ b/Framework/PythonInterface/mantid/api/src/Exports/SpectrumInfoItem.cpp @@ -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) diff --git a/Framework/PythonInterface/mantid/api/src/Exports/SpectrumInfoPythonIterator.cpp b/Framework/PythonInterface/mantid/api/src/Exports/SpectrumInfoPythonIterator.cpp index b2a40b56e580d1cb1b9169e068d1f64760ed5aae..9a4f70a2c33ff70e572a7b156cc7c394edbd0791 100644 --- a/Framework/PythonInterface/mantid/api/src/Exports/SpectrumInfoPythonIterator.cpp +++ b/Framework/PythonInterface/mantid/api/src/Exports/SpectrumInfoPythonIterator.cpp @@ -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. + */ } diff --git a/Framework/PythonInterface/mantid/geometry/src/Exports/DetectorInfoItem.cpp b/Framework/PythonInterface/mantid/geometry/src/Exports/DetectorInfoItem.cpp index 7ec273568bdaa348f28bce20b10efde9c26d1ebf..567fab39109e53c240757ecf2d5321963be59d03 100644 --- a/Framework/PythonInterface/mantid/geometry/src/Exports/DetectorInfoItem.cpp +++ b/Framework/PythonInterface/mantid/geometry/src/Exports/DetectorInfoItem.cpp @@ -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) diff --git a/Framework/PythonInterface/mantid/geometry/src/Exports/DetectorInfoPythonIterator.cpp b/Framework/PythonInterface/mantid/geometry/src/Exports/DetectorInfoPythonIterator.cpp index 11e2763045ed1e6e1934252df1de44335ad2c7ea..4359eb367e9105f278ecdf45267bf81ccdc26570 100644 --- a/Framework/PythonInterface/mantid/geometry/src/Exports/DetectorInfoPythonIterator.cpp +++ b/Framework/PythonInterface/mantid/geometry/src/Exports/DetectorInfoPythonIterator.cpp @@ -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. + */ } diff --git a/Framework/PythonInterface/test/python/mantid/api/SpectrumInfoTest.py b/Framework/PythonInterface/test/python/mantid/api/SpectrumInfoTest.py index 0be8b0e9bdcbb87d9eab4c2544df288aba872d0e..3172e5745340f013a7bb8073f6bac03b117b64a8 100644 --- a/Framework/PythonInterface/test/python/mantid/api/SpectrumInfoTest.py +++ b/Framework/PythonInterface/test/python/mantid/api/SpectrumInfoTest.py @@ -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() """ ---------------------------------------------------------------------------- diff --git a/Framework/PythonInterface/test/python/mantid/geometry/DetectorInfoTest.py b/Framework/PythonInterface/test/python/mantid/geometry/DetectorInfoTest.py index 47dc8b924473586d70467596d93e70ac86383260..0a88526937c93ce71dd0e4fe2c29a8cd73dcb6ce 100644 --- a/Framework/PythonInterface/test/python/mantid/geometry/DetectorInfoTest.py +++ b/Framework/PythonInterface/test/python/mantid/geometry/DetectorInfoTest.py @@ -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() """