Python Shared Pointers (#41)

* make VoxelKey implicitly convertible to tuple

* bind points length

* make vector of point opaque

* add indexors to points obj

* remove .get from python bindings; add iterator methods

* add pytest config file

* update tests to not use Get()

* convert vector<point> to vector<shared_ptr> in c++

* fix examples

* fix shared pointers and tests

* format

* PR Comments

* add slicing operations to Points object

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

example/example-reader.cpp

@@ -47,7 +47,7 @@ void ReaderExample()
         cout << endl << "First 5 points: " << endl;
         for (int i = 0; i < 5; i++)
         {
-            cout << node_points.Get(i).ToString() << endl;
+            cout << node_points.Get(i)->ToString() << endl;
         }
     }
 

example/example-reader.py

@@ -37,8 +37,9 @@ def reader_example():
     print(node_points)
 
     print("First 5 points:")
-    for i in range(5):
-        print(node_points.Get(i))
+    # Points object supports slicing
+    for point in node_points[:5]:
+        print(point)
 
     # We can also get the raw compressed data if we want to decompress it ourselves:
 

example/example-writer.cpp

@@ -109,13 +109,13 @@ las::Points RandomPoints(VoxelKey key, int8_t point_format_id)
     {
         // Create a point with a given point format
         // The use of las::Point constructor is strongly discouraged, instead use las::Points::CreatePoint
-        las::Point point = points.CreatePoint();
+        auto point = points.CreatePoint();
         // point has getters/setters for all attributes
-        point.UnscaledX(rand_x(gen));
-        point.UnscaledY(rand_y(gen));
-        point.UnscaledZ(rand_z(gen));
+        point->UnscaledX(rand_x(gen));
+        point->UnscaledY(rand_y(gen));
+        point->UnscaledZ(rand_z(gen));
         // For visualization purposes
-        point.PointSourceID(key.d + key.x + key.y + key.d);
+        point->PointSourceID(key.d + key.x + key.y + key.d);
 
         points.AddPoint(point);
     }

include/copc-lib/las/point.hpp

@@ -437,8 +437,8 @@ class Point
     bool operator!=(const Point &other) const { return !(*this == other); };
     std::string ToString() const;
 
-    static Point Unpack(std::istream &in_stream, const int8_t &point_format_id, const Vector3 &scale,
-                        const Vector3 &offset, const uint16_t &num_extra_bytes);
+    static std::shared_ptr<Point> Unpack(std::istream &in_stream, const int8_t &point_format_id, const Vector3 &scale,
+                                         const Vector3 &offset, const uint16_t &num_extra_bytes);
     void Pack(std::ostream &out_stream) const;
     void ToPointFormat(const int8_t &point_format_id);
 

include/copc-lib/las/points.hpp

@@ -21,27 +21,36 @@ class Points
            const uint16_t &num_extra_bytes = 0);
     Points(const LasHeader &header);
     // Will create Points object given a points vector
-    Points(const std::vector<Point> &points);
+    Points(const std::vector<std::shared_ptr<Point>> &points);
 
     // Getters
     int8_t PointFormatID() const { return point_format_id_; }
     uint32_t PointRecordLength() const { return point_record_length_; }
     uint32_t NumExtraBytes() const { return ComputeNumExtraBytes(point_format_id_, point_record_length_); }
 
     // Vector functions
-    std::vector<Point> Get() { return points_; }
-    Point Get(const size_t &idx) { return points_[idx]; }
+    std::vector<std::shared_ptr<Point>> Get() { return points_; }
     size_t Size() const { return points_.size(); }
     void Reserve(const size_t &num) { points_.reserve(num); }
 
+    std::shared_ptr<Point> Get(const size_t &idx) { return points_[idx]; }
+    std::shared_ptr<Point> &operator[](size_t i) { return points_[i]; }
+    const std::shared_ptr<Point> &operator[](size_t i) const { return points_[i]; }
+
+    const std::vector<std::shared_ptr<Point>>::const_iterator begin() const { return points_.begin(); }
+    const std::vector<std::shared_ptr<Point>>::const_iterator end() const { return points_.end(); }
+
     // Add points functions
-    void AddPoint(const Point &point);
+    void AddPoint(const std::shared_ptr<Point> &point);
     void AddPoints(Points points);
     // TODO[Leo]: Add this to tests
-    void AddPoints(std::vector<las::Point> points);
+    void AddPoints(std::vector<std::shared_ptr<Point>> points);
 
     // Point functions
-    las::Point CreatePoint() { return las::Point(point_format_id_, scale_, offset_, NumExtraBytes()); }
+    std::shared_ptr<Point> CreatePoint()
+    {
+        return std::make_shared<Point>(point_format_id_, scale_, offset_, NumExtraBytes());
+    }
     void ToPointFormat(const int8_t &point_format_id);
 
     // Pack/unpack
@@ -58,7 +67,7 @@ class Points
     {
         std::vector<double> out;
         out.resize(Size());
-        std::transform(points_.begin(), points_.end(), out.begin(), [](Point p) { return p.X(); });
+        std::transform(points_.begin(), points_.end(), out.begin(), [](std::shared_ptr<Point> p) { return p->X(); });
         return out;
     }
     void X(const std::vector<double> &in)
@@ -67,14 +76,15 @@ class Points
             throw std::runtime_error("X setter array must be same size as Points array!");
 
         for (unsigned i = 0; i < points_.size(); ++i)
-            points_[i].X(in[i]);
+            points_[i]->X(in[i]);
     }
 
     std::vector<double> Y() const
     {
         std::vector<double> out;
         out.resize(Size());
-        std::transform(points_.begin(), points_.end(), out.begin(), [](const Point &p) { return p.Y(); });
+        std::transform(points_.begin(), points_.end(), out.begin(),
+                       [](const std::shared_ptr<Point> &p) { return p->Y(); });
         return out;
     }
     void Y(const std::vector<double> &in)
@@ -83,14 +93,15 @@ class Points
             throw std::runtime_error("Y setter array must be same size as Points array!");
 
         for (unsigned i = 0; i < points_.size(); ++i)
-            points_[i].Y(in[i]);
+            points_[i]->Y(in[i]);
     }
 
     std::vector<double> Z() const
     {
         std::vector<double> out;
         out.resize(Size());
-        std::transform(points_.begin(), points_.end(), out.begin(), [](const Point &p) { return p.Z(); });
+        std::transform(points_.begin(), points_.end(), out.begin(),
+                       [](const std::shared_ptr<Point> &p) { return p->Z(); });
         return out;
     }
     void Z(const std::vector<double> &in)
@@ -99,11 +110,11 @@ class Points
             throw std::runtime_error("Z setter array must be same size as Points array!");
 
         for (unsigned i = 0; i < points_.size(); ++i)
-            points_[i].Z(in[i]);
+            points_[i]->Z(in[i]);
     }
 
   private:
-    std::vector<Point> points_;
+    std::vector<std::shared_ptr<Point>> points_;
     int8_t point_format_id_;
     uint32_t point_record_length_;
     Vector3 scale_;

python/bindings.cpp

@@ -56,6 +56,7 @@ PYBIND11_MODULE(copclib, m)
         .def("ChildOf", &VoxelKey::ChildOf, py::arg("parent_key"))
         .def("__str__", &VoxelKey::ToString)
         .def("__repr__", &VoxelKey::ToString);
+    py::implicitly_convertible<py::tuple, VoxelKey>();
 
     py::class_<Node>(m, "Node")
         .def(py::init<>())
@@ -98,10 +99,10 @@ PYBIND11_MODULE(copclib, m)
             [](const py::tuple &t) { // __setstate__
                 return Vector3(t[0].cast<double>(), t[1].cast<double>(), t[2].cast<double>());
             }));
-
     py::implicitly_convertible<py::list, Vector3>();
+    py::implicitly_convertible<py::tuple, Vector3>();
 
-    py::class_<las::Point>(m, "Point")
+    py::class_<las::Point, std::shared_ptr<las::Point>>(m, "Point")
         .def_property("X", py::overload_cast<>(&las::Point::X, py::const_),
                       py::overload_cast<const double &>(&las::Point::X))
         .def_property("Y", py::overload_cast<>(&las::Point::Y, py::const_),
@@ -188,10 +189,21 @@ PYBIND11_MODULE(copclib, m)
         .def("__str__", &las::Point::ToString)
         .def("__repr__", &las::Point::ToString);
 
+    using DiffType = ssize_t;
+    using SizeType = size_t;
+
+    auto wrap_i = [](DiffType i, SizeType n) {
+        if (i < 0)
+            i += n;
+        if (i < 0 || (SizeType)i >= n)
+            throw py::index_error();
+        return i;
+    };
+
     py::class_<las::Points>(m, "Points")
         .def(py::init<const uint8_t &, const Vector3 &, const Vector3 &, const uint16_t &>(),
              py::arg("point_format_id"), py::arg("scale"), py::arg("offset"), py::arg("num_extra_bytes") = 0)
-        .def(py::init<const std::vector<las::Point> &>(), py::arg("points"))
+        .def(py::init<const std::vector<std::shared_ptr<las::Point>> &>(), py::arg("points"))
         .def(py::init<const las::LasHeader &>())
         .def_property("X", py::overload_cast<>(&las::Points::X, py::const_),
                       py::overload_cast<const std::vector<double> &>(&las::Points::X))
@@ -202,18 +214,74 @@ PYBIND11_MODULE(copclib, m)
         .def_property_readonly("PointFormatID", &las::Points::PointFormatID)
         .def_property_readonly("PointRecordLength", &las::Points::PointRecordLength)
         .def_property_readonly("NumExtraBytes", &las::Points::NumExtraBytes)
-        .def("Get", py::overload_cast<>(&las::Points::Get))
-        .def("Get", py::overload_cast<const size_t &>(&las::Points::Get), py::arg("idx"))
-        .def_property_readonly("Size", &las::Points::Size)
         .def("AddPoint", &las::Points::AddPoint)
         .def("AddPoints", py::overload_cast<las::Points>(&las::Points::AddPoints))
-        .def("AddPoints", py::overload_cast<std::vector<las::Point>>(&las::Points::AddPoints))
+        .def("AddPoints", py::overload_cast<std::vector<std::shared_ptr<las::Point>>>(&las::Points::AddPoints))
         .def("CreatePoint", &las::Points::CreatePoint)
         .def("ToPointFormat", &las::Points::ToPointFormat, py::arg("point_format_id"))
         .def("Pack", py::overload_cast<>(&las::Points::Pack))
         .def("Unpack", py::overload_cast<const std::vector<char> &, const las::LasHeader &>(&las::Points::Unpack))
         .def("Unpack", py::overload_cast<const std::vector<char> &, const int8_t &, const uint16_t &, const Vector3 &,
                                          const Vector3 &>(&las::Points::Unpack))
+        /// Bare bones interface
+        .def("__getitem__",
+             [wrap_i](const las::Points &s, DiffType i) {
+                 i = wrap_i(i, s.Size());
+                 return s[(SizeType)i];
+             })
+        .def("__setitem__",
+             [wrap_i](las::Points &s, DiffType i, std::shared_ptr<las::Point> v) {
+                 i = wrap_i(i, s.Size());
+                 s[(SizeType)i] = v;
+             })
+        // todo?
+        //.def(
+        //    "__delitem__",
+        //    [wrap_i](las::Points &s, size_t i) {
+        //        i = wrap_i(i, s.Size());
+        //    },
+        //    "Delete the list elements at index ``i``")
+        .def("__len__", &las::Points::Size)
+        /// Optional sequence protocol operations
+        .def(
+            "__iter__", [](las::Points &s) { return py::make_iterator(s.begin(), s.end()); },
+            py::keep_alive<0, 1>() /* Essential: keep object alive while iterator exists */)
+        .def("__contains__",
+             [](las::Points &s, std::shared_ptr<las::Point> v) { return std::find(s.begin(), s.end(), v) != s.end(); })
+        /// Slicing protocol (optional)
+        .def("__getitem__",
+             [](const las::Points &s, const py::slice &slice) -> las::Points * {
+                 size_t start = 0, stop = 0, step = 0, slicelength = 0;
+                 if (!slice.compute(s.Size(), &start, &stop, &step, &slicelength))
+                     throw py::error_already_set();
+
+                 std::vector<std::shared_ptr<las::Point>> new_points;
+                 new_points.reserve(slicelength);
+
+                 for (size_t i = 0; i < slicelength; ++i)
+                 {
+                     new_points.push_back(s[start]);
+                     start += step;
+                 }
+
+                 auto *seq = new las::Points(new_points);
+                 return seq;
+             })
+        .def("__setitem__",
+             [](las::Points &s, const py::slice &slice, const las::Points &value) {
+                 size_t start = 0, stop = 0, step = 0, slicelength = 0;
+                 if (!slice.compute(s.Size(), &start, &stop, &step, &slicelength))
+                     throw py::error_already_set();
+
+                 if (slicelength != value.Size())
+                     throw std::runtime_error("Left and right hand size of slice assignment have different sizes!");
+
+                 for (size_t i = 0; i < slicelength; ++i)
+                 {
+                     s[start] = value[i];
+                     start += step;
+                 }
+             })
         .def("__str__", &las::Points::ToString)
         .def("__repr__", &las::Points::ToString);
 

src/las/point.cpp

@@ -97,50 +97,50 @@ bool Point::operator==(const Point &other) const
     return true;
 }
 
-Point Point::Unpack(std::istream &in_stream, const int8_t &point_format_id, const Vector3 &scale, const Vector3 &offset,
-                    const uint16_t &num_extra_bytes)
+std::shared_ptr<Point> Point::Unpack(std::istream &in_stream, const int8_t &point_format_id, const Vector3 &scale,
+                                     const Vector3 &offset, const uint16_t &num_extra_bytes)
 {
-    Point p(point_format_id, scale, offset, num_extra_bytes);
+    std::shared_ptr<Point> p = std::make_shared<Point>(point_format_id, scale, offset, num_extra_bytes);
 
-    p.x_ = unpack<int32_t>(in_stream);
-    p.y_ = unpack<int32_t>(in_stream);
-    p.z_ = unpack<int32_t>(in_stream);
-    p.intensity_ = unpack<uint16_t>(in_stream);
-    if (p.extended_point_type_)
+    p->x_ = unpack<int32_t>(in_stream);
+    p->y_ = unpack<int32_t>(in_stream);
+    p->z_ = unpack<int32_t>(in_stream);
+    p->intensity_ = unpack<uint16_t>(in_stream);
+    if (p->extended_point_type_)
     {
-        p.extended_returns_ = unpack<uint8_t>(in_stream);
-        p.extended_flags_ = unpack<uint8_t>(in_stream);
-        p.extended_classification_ = unpack<uint8_t>(in_stream);
-        p.user_data_ = unpack<uint8_t>(in_stream);
-        p.extended_scan_angle_ = unpack<int16_t>(in_stream);
+        p->extended_returns_ = unpack<uint8_t>(in_stream);
+        p->extended_flags_ = unpack<uint8_t>(in_stream);
+        p->extended_classification_ = unpack<uint8_t>(in_stream);
+        p->user_data_ = unpack<uint8_t>(in_stream);
+        p->extended_scan_angle_ = unpack<int16_t>(in_stream);
     }
     else
     {
-        p.returns_flags_eof_ = unpack<uint8_t>(in_stream);
-        p.classification_ = unpack<uint8_t>(in_stream);
-        p.scan_angle_rank_ = unpack<int8_t>(in_stream);
-        p.user_data_ = unpack<uint8_t>(in_stream);
+        p->returns_flags_eof_ = unpack<uint8_t>(in_stream);
+        p->classification_ = unpack<uint8_t>(in_stream);
+        p->scan_angle_rank_ = unpack<int8_t>(in_stream);
+        p->user_data_ = unpack<uint8_t>(in_stream);
     }
-    p.point_source_id_ = unpack<uint16_t>(in_stream);
+    p->point_source_id_ = unpack<uint16_t>(in_stream);
 
-    if (p.has_gps_time_)
+    if (p->has_gps_time_)
     {
-        p.gps_time_ = unpack<double>(in_stream);
+        p->gps_time_ = unpack<double>(in_stream);
     }
-    if (p.has_rgb_)
+    if (p->has_rgb_)
     {
-        p.rgb_[0] = unpack<uint16_t>(in_stream);
-        p.rgb_[1] = unpack<uint16_t>(in_stream);
-        p.rgb_[2] = unpack<uint16_t>(in_stream);
+        p->rgb_[0] = unpack<uint16_t>(in_stream);
+        p->rgb_[1] = unpack<uint16_t>(in_stream);
+        p->rgb_[2] = unpack<uint16_t>(in_stream);
     }
-    if (p.has_nir_)
+    if (p->has_nir_)
     {
-        p.nir_ = unpack<uint16_t>(in_stream);
+        p->nir_ = unpack<uint16_t>(in_stream);
     }
 
     for (uint32_t i = 0; i < num_extra_bytes; i++)
     {
-        p.extra_bytes_[i] = unpack<uint8_t>(in_stream);
+        p->extra_bytes_[i] = unpack<uint8_t>(in_stream);
     }
     return p;
 }