Fix for broken iterator works

re #23145

Framework/Geometry/CMakeLists.txt

@@ -221,6 +221,7 @@ set ( INC_FILES
 	inc/MantidGeometry/Instrument_fwd.h
 	inc/MantidGeometry/Instrument/DetectorInfoIterator.h
 	inc/MantidGeometry/Instrument/DetectorInfoItem.h
+	inc/MantidGeometry/Instrument/DetectorInfoPythonIterator.h
 	inc/MantidGeometry/MDGeometry/CompositeImplicitFunction.h
 	inc/MantidGeometry/MDGeometry/GeneralFrame.h
 	inc/MantidGeometry/MDGeometry/HKL.h

Framework/Geometry/inc/MantidGeometry/Instrument/DetectorInfoItem.h

@@ -7,6 +7,7 @@
 #include "MantidKernel/V3D.h"
 
 using Mantid::Kernel::V3D;
+using Mantid::Geometry::DetectorInfo;
 
 namespace Mantid {
 namespace Geometry {
@@ -14,28 +15,24 @@ namespace Geometry {
 class MANTID_GEOMETRY_DLL DetectorInfoItem {
 
 public:
-  DetectorInfoItem(const DetectorInfoItem &copy)
-      : m_detectorInfo(copy.m_detectorInfo), m_index(copy.m_index) {}
+  DetectorInfoItem(const DetectorInfoItem &other) = default;
+  DetectorInfoItem &operator=(const DetectorInfoItem &rhs) = default;
+  DetectorInfoItem(DetectorInfoItem &&other) = default;
+  DetectorInfoItem &operator=(DetectorInfoItem &&rhs) = default;
 
-  DetectorInfoItem &operator=(const DetectorInfoItem &rhs) {
-    this->m_index = rhs.m_index;
-    return *this;
+  Mantid::Kernel::V3D position() const {
+    return m_detectorInfo->position(m_index); 
   }
 
-  size_t position() const { return m_index; }
-
   void advance(int64_t delta) {
-    if (delta < 0) {
-      m_index = std::max(static_cast<uint64_t>(0),
-                         static_cast<uint64_t>(m_index) + delta);
-    } else {
-      m_index =
-          std::min(m_detectorInfo.size(), m_index + static_cast<size_t>(delta));
-    }
+    m_index = delta < 0 ? std::max(static_cast<uint64_t>(0),
+                                   static_cast<uint64_t>(m_index) + delta)
+                        : std::min(m_detectorInfo->size(),
+                                   m_index + static_cast<size_t>(delta));
   }
 
   void incrementIndex() {
-    if (m_index < m_detectorInfo.size()) {
+    if (m_index < m_detectorInfo->size()) {
       ++m_index;
     }
   }
@@ -55,13 +52,12 @@ private:
 
   // Private constructor, can only be created by DetectorInfoIterator
   DetectorInfoItem(const DetectorInfo &detectorInfo, const size_t index)
-      : m_detectorInfo(detectorInfo), m_index(index) {}
-
-  // DetectorInfoItem& operator=(const DetectorInfoItem &) = delete;
+      : m_detectorInfo(&detectorInfo), m_index(index) {}
 
-  // Variables to hold data
-  const DetectorInfo &m_detectorInfo;
-  size_t m_index = 0;
+  // Non-owning pointer. A reference makes the class unable to define assignment
+  // operator that we need.
+  const DetectorInfo *m_detectorInfo;
+  size_t m_index;
 };
 
 } // namespace Geometry

Framework/Geometry/inc/MantidGeometry/Instrument/DetectorInfoIterator.h

-
 #ifndef MANTID_GEOMETRY_DETECTORINFOITERATOR_H_
 #define MANTID_GEOMETRY_DETECTORINFOITERATOR_H_
 
@@ -7,25 +6,18 @@
 
 #include "MantidGeometry/Instrument/DetectorInfoItem.h"
 
+using Mantid::Geometry::DetectorInfoItem;
+
 namespace Mantid {
 namespace Geometry {
 
-class DetectorInfo;
-
 class MANTID_GEOMETRY_DLL DetectorInfoIterator
     : public boost::iterator_facade<DetectorInfoIterator,
                                     const DetectorInfoItem &,
-                                    boost::forward_traversal_tag> {
+                                    boost::bidirectional_traversal_tag> {
 
 public:
-  DetectorInfoIterator(const DetectorInfo &detectorInfo, const size_t index)
-      : m_item(detectorInfo, index) {
-  } // m_detectorInfo(detectorInfo), m_index(index) {}
-
-  DetectorInfoIterator(const DetectorInfoIterator &detInfoIterator)
-      : m_item(detInfoIterator.m_item) {
-  } // m_detectorInfo(detInfoIterator.m_detectorInfo),
-    // m_item(detInfoIterator.m_detectorInfo, detInfoIterator.m_index) {}
+  DetectorInfoIterator(const DetectorInfo& owner, const size_t index) : m_item(owner, index) {}
 
 private:
   friend class boost::iterator_core_access;
@@ -48,8 +40,6 @@ private:
   }
 
   DetectorInfoItem m_item;
-  // DetectorInfo m_detectorInfo;
-  // size_t m_index;
 };
 
 } // namespace Geometry

Framework/Geometry/inc/MantidGeometry/Instrument/DetectorInfoPythonIterator.h

+#ifndef MANTID_GEOMETRY_DETECTORINFOPYTHONITERATOR_H_
+#define MANTID_GEOMETRY_DETECTORINFOPYTHONITERATOR_H_
+
+#include <boost/python/class.hpp>
+#include <boost/python/copy_const_reference.hpp>
+#include <boost/python/def.hpp>
+#include <boost/python/iterator.hpp>
+#include <boost/python/module.hpp>
+#include <boost/python/reference_existing_object.hpp>
+
+#include "MantidGeometry/Instrument/DetectorInfo.h"
+#include "MantidGeometry/Instrument/DetectorInfoItem.h"
+#include "MantidGeometry/Instrument/DetectorInfoIterator.h"
+
+using Mantid::Geometry::DetectorInfo;
+using Mantid::Geometry::DetectorInfoItem;
+using Mantid::Geometry::DetectorInfoIterator;
+
+using namespace boost::python;
+
+namespace Mantid {
+namespace Geometry {
+
+class DetectorInfoPythonIterator {
+public:
+  explicit DetectorInfoPythonIterator(const DetectorInfo &source)
+      : m_begin(source.begin()), m_end(source.end()), m_current(*m_begin) {}
+
+  const DetectorInfoItem &next() {
+    if (m_begin == m_end) {
+      objects::stop_iteration_error();
+    }
+    m_current = *m_begin++;
+    return m_current;
+  }
+
+private:
+  DetectorInfoIterator m_begin;
+  DetectorInfoIterator m_end;
+  DetectorInfoItem m_current;
+};
+
+} // namespace Geometry
+} // namespace Mantid
+#endif /* MANTID_GEOMETRY_DETECTORINFOPYTHONITERATOR_H_ */

Framework/Geometry/src/Instrument/DetectorInfo.cpp

@@ -11,7 +11,6 @@
 
 namespace Mantid {
 namespace Geometry {
-
 /** Construct DetectorInfo based on an Instrument.
  *
  * The Instrument reference `instrument` must be the parameterized instrument

Framework/PythonInterface/mantid/geometry/CMakeLists.txt

@@ -41,6 +41,7 @@ set ( EXPORT_FILES
   src/Exports/DetectorInfo.cpp
   src/Exports/DetectorInfoItem.cpp
   src/Exports/DetectorInfoIterator.cpp
+  src/Exports/DetectorInfoPythonIterator.cpp
 )
 
 #############################################################################################

Framework/PythonInterface/mantid/geometry/src/Exports/DetectorInfo.cpp

@@ -10,16 +10,24 @@
 #include <boost/python/return_value_policy.hpp>
 #include <iterator>
 
+
 #include "MantidGeometry/Instrument/DetectorInfoItem.h"
 #include "MantidGeometry/Instrument/DetectorInfoIterator.h"
+#include "MantidGeometry/Instrument/DetectorInfoPythonIterator.h"
 
 using Mantid::Geometry::DetectorInfo;
 using Mantid::Geometry::DetectorInfoItem;
 using Mantid::Geometry::DetectorInfoIterator;
+using Mantid::Geometry::DetectorInfoPythonIterator;
+
 using Mantid::Kernel::Quat;
 using Mantid::Kernel::V3D;
 using namespace boost::python;
 
+DetectorInfoPythonIterator make_pyiterator(const DetectorInfo &source) {
+  return DetectorInfoPythonIterator(source);
+};
+
 void export_DetectorInfo() {
 
   // Function pointers to distinguish between overloaded versions
@@ -40,20 +48,10 @@ void export_DetectorInfo() {
   void (DetectorInfo::*setMasked)(const size_t, bool) =
       &DetectorInfo::setMasked;
 
-  /*
-  Mantid::Geometry::DetectorInfoIterator group_begin(DetectorInfo &self) {
-    return self.begin();
-  }
-
-  Mantid::Geometry::DetectorInfoIterator group_end(DetectorInfo &self) {
-    return self.end();
-  }*/
-
   // Export to Python
   class_<DetectorInfo, boost::noncopyable>("DetectorInfo", no_init)
 
-      .def("__iter__", range<return_value_policy<copy_const_reference>>(
-                           &DetectorInfo::begin, &DetectorInfo::end))
+      .def("__iter__", make_pyiterator)
 
       .def("__len__", &DetectorInfo::size, (arg("self")),
            "Returns the size of the DetectorInfo, i.e., the number of "

Framework/PythonInterface/mantid/geometry/src/Exports/DetectorInfoItem.cpp

@@ -12,6 +12,5 @@ void export_DetectorInfoItem() {
 
   // Export to Python
   class_<DetectorInfoItem>("DetectorInfoItem", no_init)
-      .add_property("position", &DetectorInfoItem::position);
-  //.def("position", &DetectorInfoItem::position, arg("self"));
+    .add_property("position", &DetectorInfoItem::position);
 }

Framework/PythonInterface/mantid/geometry/src/Exports/DetectorInfoPythonIterator.cpp

+#include <boost/python/class.hpp>
+#include <boost/python/copy_const_reference.hpp>
+#include <boost/python/def.hpp>
+#include <boost/python/iterator.hpp>
+#include <boost/python/module.hpp>
+#include <boost/python/reference_existing_object.hpp>
+
+#include "MantidGeometry/Instrument/DetectorInfoPythonIterator.h"
+
+using namespace boost::python;
+using Mantid::Geometry::DetectorInfoPythonIterator;
+
+void export_DetectorInfoPythonIterator() {
+
+  // Export to Python
+  class_<DetectorInfoPythonIterator>("DetectorInfoPythonIterator", no_init)
+    .def("__iter__", objects::identity_function())
+    .def(
+#if PY_VERSION_HEX >= 0x03000000
+      "__next__"
+#else 
+      "next"
+#endif
+      ,
+      &DetectorInfoPythonIterator::next,
+      return_value_policy<copy_const_reference>());
+   
+  //def("detectorInfo", detectorInfo, return_value_policy<reference_existing_object>());
+}
\ No newline at end of file

Framework/PythonInterface/test/python/mantid/geometry/DetectorInfoTest.py

@@ -106,19 +106,10 @@ class DetectorInfoTest(unittest.TestCase):
     ---------------
     """
 
-    def test_iteration(self):
-        #import time
-        #time.sleep(20)
-        info = self._ws.detectorInfo()
-        #self.assertEquals(len(info), 0)
-        #self.assertEquals(info.size(), 0)
-        for a in info:
-            #self.assertEquals(type(a), DetectorInfoItem)
-            #self.assertEquals(info.position(0), "")
-            print(a.position)
-            #print(a)
-        self.fail()
-
+    def test_iteration_for_position(self):
+        info = self._ws.detectorInfo()
+        for detInfo in info:
+            self.assertEquals(type(detInfo.position), V3D)
 
 
     """