From 29a34fd50781bf534dbc8e287cc5b076ca32f250 Mon Sep 17 00:00:00 2001 From: Christopher Lee Date: Wed, 22 Sep 2021 15:46:02 -0500 Subject: [PATCH] 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 to vector 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 | 2 +- example/example-reader.py | 5 +- example/example-writer.cpp | 10 +- include/copc-lib/las/point.hpp | 4 +- include/copc-lib/las/points.hpp | 37 ++++--- python/bindings.cpp | 82 ++++++++++++-- src/las/point.cpp | 54 +++++----- src/las/points.cpp | 22 ++-- test/point_test.cpp | 14 +-- test/points_test.cpp | 183 ++++++++++++++++++++++++-------- test/points_test.py | 158 +++++++++++++++++++++++---- test/reader_node_test.cpp | 128 +++++++++++----------- test/reader_test.py | 4 +- 13 files changed, 497 insertions(+), 206 deletions(-) diff --git a/example/example-reader.cpp b/example/example-reader.cpp index 42ed818e..55d5116d 100644 --- a/example/example-reader.cpp +++ b/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; } } diff --git a/example/example-reader.py b/example/example-reader.py index 290e1455..2877aa51 100644 --- a/example/example-reader.py +++ b/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: diff --git a/example/example-writer.cpp b/example/example-writer.cpp index ff334339..a9f47a3d 100644 --- a/example/example-writer.cpp +++ b/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); } diff --git a/include/copc-lib/las/point.hpp b/include/copc-lib/las/point.hpp index 5c832e32..7246b8b3 100644 --- a/include/copc-lib/las/point.hpp +++ b/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 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); diff --git a/include/copc-lib/las/points.hpp b/include/copc-lib/las/points.hpp index d747c08c..cfb3bde8 100644 --- a/include/copc-lib/las/points.hpp +++ b/include/copc-lib/las/points.hpp @@ -21,7 +21,7 @@ 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 &points); + Points(const std::vector> &points); // Getters int8_t PointFormatID() const { return point_format_id_; } @@ -29,19 +29,28 @@ class Points uint32_t NumExtraBytes() const { return ComputeNumExtraBytes(point_format_id_, point_record_length_); } // Vector functions - std::vector Get() { return points_; } - Point Get(const size_t &idx) { return points_[idx]; } + std::vector> Get() { return points_; } size_t Size() const { return points_.size(); } void Reserve(const size_t &num) { points_.reserve(num); } + std::shared_ptr Get(const size_t &idx) { return points_[idx]; } + std::shared_ptr &operator[](size_t i) { return points_[i]; } + const std::shared_ptr &operator[](size_t i) const { return points_[i]; } + + const std::vector>::const_iterator begin() const { return points_.begin(); } + const std::vector>::const_iterator end() const { return points_.end(); } + // Add points functions - void AddPoint(const Point &point); + void AddPoint(const std::shared_ptr &point); void AddPoints(Points points); // TODO[Leo]: Add this to tests - void AddPoints(std::vector points); + void AddPoints(std::vector> points); // Point functions - las::Point CreatePoint() { return las::Point(point_format_id_, scale_, offset_, NumExtraBytes()); } + std::shared_ptr CreatePoint() + { + return std::make_shared(point_format_id_, scale_, offset_, NumExtraBytes()); + } void ToPointFormat(const int8_t &point_format_id); // Pack/unpack @@ -58,7 +67,7 @@ class Points { std::vector 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 p) { return p->X(); }); return out; } void X(const std::vector &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 Y() const { std::vector 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 &p) { return p->Y(); }); return out; } void Y(const std::vector &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 Z() const { std::vector 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 &p) { return p->Z(); }); return out; } void Z(const std::vector &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 points_; + std::vector> points_; int8_t point_format_id_; uint32_t point_record_length_; Vector3 scale_; diff --git a/python/bindings.cpp b/python/bindings.cpp index 62c59b11..e4fe4bc6 100644 --- a/python/bindings.cpp +++ b/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::class_(m, "Node") .def(py::init<>()) @@ -98,10 +99,10 @@ PYBIND11_MODULE(copclib, m) [](const py::tuple &t) { // __setstate__ return Vector3(t[0].cast(), t[1].cast(), t[2].cast()); })); - py::implicitly_convertible(); + py::implicitly_convertible(); - py::class_(m, "Point") + py::class_>(m, "Point") .def_property("X", py::overload_cast<>(&las::Point::X, py::const_), py::overload_cast(&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_(m, "Points") .def(py::init(), py::arg("point_format_id"), py::arg("scale"), py::arg("offset"), py::arg("num_extra_bytes") = 0) - .def(py::init &>(), py::arg("points")) + .def(py::init> &>(), py::arg("points")) .def(py::init()) .def_property("X", py::overload_cast<>(&las::Points::X, py::const_), py::overload_cast &>(&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(&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::AddPoints)) - .def("AddPoints", py::overload_cast>(&las::Points::AddPoints)) + .def("AddPoints", py::overload_cast>>(&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 las::LasHeader &>(&las::Points::Unpack)) .def("Unpack", py::overload_cast &, 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 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 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> 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); diff --git a/src/las/point.cpp b/src/las/point.cpp index 037a5139..75ebc34a 100644 --- a/src/las/point.cpp +++ b/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::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 p = std::make_shared(point_format_id, scale, offset, num_extra_bytes); - p.x_ = unpack(in_stream); - p.y_ = unpack(in_stream); - p.z_ = unpack(in_stream); - p.intensity_ = unpack(in_stream); - if (p.extended_point_type_) + p->x_ = unpack(in_stream); + p->y_ = unpack(in_stream); + p->z_ = unpack(in_stream); + p->intensity_ = unpack(in_stream); + if (p->extended_point_type_) { - p.extended_returns_ = unpack(in_stream); - p.extended_flags_ = unpack(in_stream); - p.extended_classification_ = unpack(in_stream); - p.user_data_ = unpack(in_stream); - p.extended_scan_angle_ = unpack(in_stream); + p->extended_returns_ = unpack(in_stream); + p->extended_flags_ = unpack(in_stream); + p->extended_classification_ = unpack(in_stream); + p->user_data_ = unpack(in_stream); + p->extended_scan_angle_ = unpack(in_stream); } else { - p.returns_flags_eof_ = unpack(in_stream); - p.classification_ = unpack(in_stream); - p.scan_angle_rank_ = unpack(in_stream); - p.user_data_ = unpack(in_stream); + p->returns_flags_eof_ = unpack(in_stream); + p->classification_ = unpack(in_stream); + p->scan_angle_rank_ = unpack(in_stream); + p->user_data_ = unpack(in_stream); } - p.point_source_id_ = unpack(in_stream); + p->point_source_id_ = unpack(in_stream); - if (p.has_gps_time_) + if (p->has_gps_time_) { - p.gps_time_ = unpack(in_stream); + p->gps_time_ = unpack(in_stream); } - if (p.has_rgb_) + if (p->has_rgb_) { - p.rgb_[0] = unpack(in_stream); - p.rgb_[1] = unpack(in_stream); - p.rgb_[2] = unpack(in_stream); + p->rgb_[0] = unpack(in_stream); + p->rgb_[1] = unpack(in_stream); + p->rgb_[2] = unpack(in_stream); } - if (p.has_nir_) + if (p->has_nir_) { - p.nir_ = unpack(in_stream); + p->nir_ = unpack(in_stream); } for (uint32_t i = 0; i < num_extra_bytes; i++) { - p.extra_bytes_[i] = unpack(in_stream); + p->extra_bytes_[i] = unpack(in_stream); } return p; } diff --git a/src/las/points.cpp b/src/las/points.cpp index c518d412..13add607 100644 --- a/src/las/points.cpp +++ b/src/las/points.cpp @@ -20,15 +20,15 @@ Points::Points(const int8_t &point_format_id, const Vector3 &scale, const Vector Points::Points(const LasHeader &header) : Points(header.point_format_id, header.scale, header.offset, header.NumExtraBytes()){}; -Points::Points(const std::vector &points) +Points::Points(const std::vector> &points) { if (points.empty()) throw std::runtime_error("Can't add empty vector of points to Points!"); - point_record_length_ = points[0].PointRecordLength(); - point_format_id_ = points[0].PointFormatID(); - scale_ = points[0].Scale(); - offset_ = points[0].Offset(); + point_record_length_ = points[0]->PointRecordLength(); + point_format_id_ = points[0]->PointFormatID(); + scale_ = points[0]->Scale(); + offset_ = points[0]->Offset(); AddPoints(points); } @@ -38,13 +38,13 @@ void Points::ToPointFormat(const int8_t &point_format_id) if (point_format_id < 0 || point_format_id > 10) throw std::runtime_error("Point format must be 0-10."); for (auto &point : points_) - point.ToPointFormat(point_format_id); + point->ToPointFormat(point_format_id); point_format_id_ = point_format_id; } -void Points::AddPoint(const Point &point) +void Points::AddPoint(const std::shared_ptr &point) { - if (point.PointFormatID() == point_format_id_ && point.PointRecordLength() == point_record_length_) + if (point->PointFormatID() == point_format_id_ && point->PointRecordLength() == point_record_length_) points_.push_back(point); else throw std::runtime_error("New point must be of same format and byte_size."); @@ -59,11 +59,11 @@ void Points::AddPoints(Points points) points_.insert(points_.end(), point_vec.begin(), point_vec.end()); } -void Points::AddPoints(std::vector points) +void Points::AddPoints(std::vector> points) { for (const auto &point : points) { - if (point.PointFormatID() != point_format_id_ || point.PointRecordLength() != point_record_length_) + if (point->PointFormatID() != point_format_id_ || point->PointRecordLength() != point_record_length_) throw std::runtime_error("New points must be of same format and byte_size."); } @@ -103,7 +103,7 @@ Points Points::Unpack(const std::vector &point_data, const int8_t &point_f void Points::Pack(std::ostream &out_stream) { for (const auto &point : points_) - point.Pack(out_stream); + point->Pack(out_stream); } std::vector Points::Pack() diff --git a/test/point_test.cpp b/test/point_test.cpp index ae800370..04840ba5 100644 --- a/test/point_test.cpp +++ b/test/point_test.cpp @@ -666,49 +666,49 @@ TEST_CASE("Point tests", "[Point]") auto point = orig_point; point.Pack(ss); auto point_other = - Point::Unpack(ss, 0, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); + *Point::Unpack(ss, 0, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); REQUIRE(point == point_other); // Format 1 point.ToPointFormat(1); point.Pack(ss); point_other = - Point::Unpack(ss, 1, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); + *Point::Unpack(ss, 1, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); REQUIRE(point == point_other); // Format 2 point.ToPointFormat(2); point.Pack(ss); point_other = - Point::Unpack(ss, 2, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); + *Point::Unpack(ss, 2, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); REQUIRE(point == point_other); // Format 3 point.ToPointFormat(3); point.Pack(ss); point_other = - Point::Unpack(ss, 3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); + *Point::Unpack(ss, 3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); REQUIRE(point == point_other); // Format 6 point.ToPointFormat(6); point.Pack(ss); point_other = - Point::Unpack(ss, 6, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); + *Point::Unpack(ss, 6, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); REQUIRE(point == point_other); // Format 7 point.ToPointFormat(7); point.Pack(ss); point_other = - Point::Unpack(ss, 7, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); + *Point::Unpack(ss, 7, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); REQUIRE(point == point_other); // Format 8 point.ToPointFormat(8); point.Pack(ss); point_other = - Point::Unpack(ss, 8, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); + *Point::Unpack(ss, 8, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), point.NumExtraBytes()); REQUIRE(point == point_other); point.ToPointFormat(0); diff --git a/test/points_test.cpp b/test/points_test.cpp index f8f4449e..eca8ea61 100644 --- a/test/points_test.cpp +++ b/test/points_test.cpp @@ -16,11 +16,11 @@ TEST_CASE("Points tests", "[Point]") REQUIRE(points.PointRecordLength() == 38); REQUIRE(points.Get().empty()); - auto point_vec = std::vector(); + auto point_vec = std::vector>(); auto point1 = points.CreatePoint(); - point1.UnscaledX(11); - point1.UnscaledY(11); - point1.UnscaledZ(11); + point1->UnscaledX(11); + point1->UnscaledY(11); + point1->UnscaledZ(11); point_vec.push_back(point1); auto point2 = points.CreatePoint(); @@ -33,11 +33,11 @@ TEST_CASE("Points tests", "[Point]") REQUIRE(points.PointFormatID() == 3); REQUIRE(points.PointRecordLength() == 38); for (const auto &point : points.Get()) - REQUIRE(point.PointFormatID() == 3); + REQUIRE(point->PointFormatID() == 3); REQUIRE(points.PointRecordLength() == 38); - REQUIRE(points.Get(0).UnscaledX() == 11); - REQUIRE(points.Get(0).UnscaledY() == 11); - REQUIRE(points.Get(0).UnscaledZ() == 11); + REQUIRE(points.Get(0)->UnscaledX() == 11); + REQUIRE(points.Get(0)->UnscaledY() == 11); + REQUIRE(points.Get(0)->UnscaledZ() == 11); points.ToString(); } @@ -46,75 +46,75 @@ TEST_CASE("Points tests", "[Point]") { auto points = Points(3, {1, 1, 1}, {0, 0, 0}, 0); auto point = points.CreatePoint(); - point.UnscaledX(11); - point.UnscaledY(11); - point.UnscaledZ(11); + point->UnscaledX(11); + point->UnscaledY(11); + point->UnscaledZ(11); points.AddPoint(point); REQUIRE(points.Get().size() == 1); - REQUIRE(points.Get(0).UnscaledX() == 11); - REQUIRE(points.Get(0).UnscaledY() == 11); - REQUIRE(points.Get(0).UnscaledZ() == 11); + REQUIRE(points.Get(0)->UnscaledX() == 11); + REQUIRE(points.Get(0)->UnscaledY() == 11); + REQUIRE(points.Get(0)->UnscaledZ() == 11); point = points.CreatePoint(); - point.UnscaledX(22); - point.UnscaledY(22); - point.UnscaledZ(22); + point->UnscaledX(22); + point->UnscaledY(22); + point->UnscaledZ(22); points.AddPoint(point); REQUIRE(points.Get().size() == 2); - REQUIRE(points.Get(1).UnscaledX() == 22); - REQUIRE(points.Get(1).UnscaledY() == 22); - REQUIRE(points.Get(1).UnscaledZ() == 22); + REQUIRE(points.Get(1)->UnscaledX() == 22); + REQUIRE(points.Get(1)->UnscaledY() == 22); + REQUIRE(points.Get(1)->UnscaledZ() == 22); // Test check on point format - point = Point(6, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 0); + point = std::make_shared(6, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 0); REQUIRE_THROWS(points.AddPoint(point)); // Test check on extra bytes - point = Point(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 1); + point = std::make_shared(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 1); REQUIRE_THROWS(points.AddPoint(point)); } SECTION("Adding Points to Points") { - auto points = - Points(std::vector(10, Point(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 4))); - auto points_other = - Points(std::vector(10, Point(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 4))); + auto points = Points(std::vector>( + 10, std::make_shared(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 4))); + auto points_other = Points(std::vector>( + 10, std::make_shared(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 4))); points.AddPoints(points_other); REQUIRE(points.Get().size() == 20); // Test check on point format - points_other = - Points(std::vector(10, Point(6, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 4))); + points_other = Points(std::vector>( + 10, std::make_shared(6, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 4))); REQUIRE_THROWS(points.AddPoints(points_other)); // Test check on extra bytes - points_other = - Points(std::vector(10, Point(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 1))); + points_other = Points(std::vector>( + 10, std::make_shared(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 1))); REQUIRE_THROWS(points.AddPoints(points_other)); } SECTION("Points format conversion") { - auto points = - Points(std::vector(10, Point(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 4))); + auto points = Points(std::vector>( + 10, std::make_shared(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 4))); points.ToPointFormat(6); REQUIRE(points.PointFormatID() == 6); REQUIRE(points.PointRecordLength() == 38); - REQUIRE(points.Get(1).PointFormatID() == 6); - REQUIRE(points.Get(1).PointRecordLength() == 34); + REQUIRE(points.Get(1)->PointFormatID() == 6); + REQUIRE(points.Get(1)->PointRecordLength() == 34); REQUIRE_THROWS(points.ToPointFormat(-1)); REQUIRE_THROWS(points.ToPointFormat(11)); } - SECTION("Points Accessors") + SECTION("Points Group Accessors") { auto points = Points(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 4); @@ -123,9 +123,9 @@ TEST_CASE("Points tests", "[Point]") for (int i = 0; i < num_points; i++) { auto p = points.CreatePoint(); - p.X(i); - p.Y(i * 3); - p.Z(i - 80); + p->X(i); + p->Y(i * 3); + p->Z(i - 80); points.AddPoint(p); } @@ -175,15 +175,110 @@ TEST_CASE("Points tests", "[Point]") for (int i = 0; i < num_points - 1; i++) { auto p = points.Get(i); - REQUIRE(p.X() == i * 50 + 8); - REQUIRE(p.Y() == i + 800); - REQUIRE(p.Z() == i * 4); + REQUIRE(p->X() == i * 50 + 8); + REQUIRE(p->Y() == i + 800); + REQUIRE(p->Z() == i * 4); } // test last point auto last_point = points.Get(points.Size() - 1); - REQUIRE(last_point.X() == 1); - REQUIRE(last_point.Y() == 2); - REQUIRE(last_point.Z() == 3); + REQUIRE(last_point->X() == 1); + REQUIRE(last_point->Y() == 2); + REQUIRE(last_point->Z() == 3); + } + + SECTION("Points Indexers") + { + auto points = Points(3, copc::Vector3::DefaultScale(), copc::Vector3::DefaultOffset(), 4); + + // generate points + int num_points = 2000; + for (int i = 0; i < num_points; i++) + { + auto p = points.CreatePoint(); + p->X(i); + p->Y(i * 3); + p->Z(i - 80); + points.AddPoint(p); + } + + REQUIRE(points[0]->X() == 0); + REQUIRE(points.Get(0)->X() == 0); + REQUIRE(points[0]->Y() == 0); + REQUIRE(points.Get(0)->Y() == 0); + REQUIRE(points[0]->Z() == 0 - 80); + REQUIRE(points.Get(0)->Z() == 0 - 80); + + REQUIRE(points[1999]->X() == 1999); + REQUIRE(points.Get(1999)->X() == 1999); + REQUIRE(points[1999]->Y() == 1999 * 3); + REQUIRE(points.Get(1999)->Y() == 1999 * 3); + REQUIRE(points[1999]->Z() == 1999 - 80); + REQUIRE(points.Get(1999)->Z() == 1999 - 80); + + for (int i = 0; i < num_points; i++) + { + points[i]->X(-i); + points[i]->Y(-i * 2); + points[i]->Z(-i * 3); + } + + REQUIRE(points[0]->X() == 0); + REQUIRE(points.Get(0)->X() == 0); + REQUIRE(points[0]->Y() == 0); + REQUIRE(points.Get(0)->Y() == 0); + REQUIRE(points[0]->Z() == 0); + REQUIRE(points.Get(0)->Z() == 0); + + REQUIRE(points[1999]->X() == -1999); + REQUIRE(points.Get(1999)->X() == -1999); + REQUIRE(points[1999]->Y() == -1999 * 2); + REQUIRE(points.Get(1999)->Y() == -1999 * 2); + REQUIRE(points[1999]->Z() == -1999 * 3); + REQUIRE(points.Get(1999)->Z() == -1999 * 3); + + // generate vector of coordinates + std::vector Xn; + std::vector Yn; + std::vector Zn; + + REQUIRE_THROWS(points.X(Xn)); + REQUIRE_THROWS(points.Y(Yn)); + REQUIRE_THROWS(points.Z(Zn)); + + for (int i = 0; i < num_points - 1; i++) + { + Xn.push_back(i * 50 + 8); + Yn.push_back(i + 800); + Zn.push_back(i * 4); + } + + REQUIRE_THROWS(points.X(Xn)); + REQUIRE_THROWS(points.Y(Yn)); + REQUIRE_THROWS(points.Z(Zn)); + + // add the last point + Xn.push_back(1); + Yn.push_back(2); + Zn.push_back(3); + + // test setters + REQUIRE_NOTHROW(points.X(Xn)); + REQUIRE_NOTHROW(points.Y(Yn)); + REQUIRE_NOTHROW(points.Z(Zn)); + + for (int i = 0; i < num_points - 1; i++) + { + auto p = points.Get(i); + REQUIRE(p->X() == i * 50 + 8); + REQUIRE(p->Y() == i + 800); + REQUIRE(p->Z() == i * 4); + } + + // test last point + auto last_point = points.Get(points.Size() - 1); + REQUIRE(last_point->X() == 1); + REQUIRE(last_point->Y() == 2); + REQUIRE(last_point->Z() == 3); } } diff --git a/test/points_test.py b/test/points_test.py index 45b0b98b..1ae191a1 100644 --- a/test/points_test.py +++ b/test/points_test.py @@ -8,7 +8,7 @@ def test_points_constructor(): ) assert points.PointFormatID == 3 assert points.PointRecordLength == 38 - assert len(points.Get()) == 0 + assert len(points) == 0 point1 = copc.Points( 3, copc.Vector3.DefaultScale(), copc.Vector3.DefaultOffset(), num_extra_bytes=4 @@ -36,12 +36,11 @@ def test_points_constructor(): points = copc.Points(point_list) assert points.PointFormatID == 3 assert points.PointRecordLength == 38 - for point in points.Get(): + for point in points: assert point.PointFormatID == 3 assert points.PointRecordLength == 38 - assert points.Get(idx=0).UnscaledX == 11 - assert points.Get(0).UnscaledY == 11 - assert points.Get(0).UnscaledZ == 11 + assert points[0].UnscaledY == 11 + assert points[0].UnscaledZ == 11 str(points) @@ -59,10 +58,10 @@ def test_adding_point_to_points(): points.AddPoint(point) - assert len(points.Get()) == 1 - assert points.Get(0).UnscaledX == 11 - assert points.Get(0).UnscaledY == 11 - assert points.Get(0).UnscaledZ == 11 + assert len(points) == 1 + assert points[0].UnscaledX == 11 + assert points[0].UnscaledY == 11 + assert points[0].UnscaledZ == 11 point = copc.Points( 3, copc.Vector3.DefaultScale(), copc.Vector3.DefaultOffset(), num_extra_bytes=0 @@ -72,10 +71,10 @@ def test_adding_point_to_points(): point.UnscaledZ = 22 points.AddPoint(point) - assert len(points.Get()) == 2 - assert points.Get(1).UnscaledX == 22 - assert points.Get(1).UnscaledY == 22 - assert points.Get(1).UnscaledZ == 22 + assert len(points) == 2 + assert points[1].UnscaledX == 22 + assert points[1].UnscaledY == 22 + assert points[1].UnscaledZ == 22 # Test check on point format point = copc.Points( @@ -118,7 +117,7 @@ def test_adding_points_to_points(): points.AddPoints(points_other) - assert len(points.Get()) == 20 + assert len(points) == 20 # Test check on point format points_other = copc.Points( @@ -167,8 +166,8 @@ def test_points_format_conversion(): assert points.PointFormatID == 6 assert points.PointRecordLength == 38 - assert points.Get(0).PointFormatID == 6 - assert points.Get(0).PointRecordLength == 34 + assert points[0].PointFormatID == 6 + assert points[0].PointRecordLength == 34 with pytest.raises(RuntimeError): points.ToPointFormat(-1) @@ -176,7 +175,28 @@ def test_points_format_conversion(): points.ToPointFormat(11) -def test_points_accessors(): +def test_points_iterator(): + points = copc.Points( + 3, copc.Vector3.DefaultScale(), copc.Vector3.DefaultOffset(), 4 + ) + + # generate points + num_points = 2000 + for i in range(num_points): + p = points.CreatePoint() + p.Classification = i % 32 + points.AddPoint(p) + + classification_index = [points[i].Classification for i in range(num_points)] + classification_iterator = [p.Classification for p in points] + + for i, clas in enumerate(classification_index): + assert clas == i % 32 + + assert classification_index == classification_iterator + + +def test_points_group_accessors(): points = copc.Points( 3, copc.Vector3.DefaultScale(), copc.Vector3.DefaultOffset(), 4 ) @@ -190,7 +210,7 @@ def test_points_accessors(): p.Z = i - 80 points.AddPoint(p) - assert points.Size == num_points + assert len(points) == num_points # test that the getters work for i in range(num_points): @@ -228,13 +248,109 @@ def test_points_accessors(): points.Z = Zn for i in range(num_points - 1): - p = points.Get(i) + p = points[i] assert p.X == i * 50 + 8 assert p.Y == i + 800 assert p.Z == i * 4 - # test last point - last_point = points.Get(points.Size - 1) + # test negative indices + last_point = points[-1] assert last_point.X == 1 assert last_point.Y == 2 assert last_point.Z == 3 + + +def test_points_accessors(): + points = copc.Points( + 3, copc.Vector3.DefaultScale(), copc.Vector3.DefaultOffset(), 4 + ) + + # generate points + num_points = 2000 + for i in range(num_points): + p = points.CreatePoint() + p.X = i + p.Y = i * 3 + p.Z = i - 80 + points.AddPoint(p) + + assert min(points.X) == 0 + assert max(points.X) == num_points - 1 + assert min(points.Y) == 0 + assert max(points.Y) == (num_points - 1) * 3 + assert min(points.Z) == 0 - 80 + assert max(points.Z) == (num_points - 1) - 80 + + # test slice + assert points.X[5:10] == [x for x in range(5, 10)] + assert points.Y[-10:] == [x * 3 for x in range(1990, 2000)] + assert points.Z[:10] == [x - 80 for x in range(0, 10)] + + # test index setter + for i in range(len(points)): + p = points[i] + p.X = 20 + p.Y = 30 + p.Z = 40 + + assert all([x == 20 for x in points.X]) + assert all([y == 30 for y in points.Y]) + assert all([z == 40 for z in points.Z]) + + # test iterator setter + for p in points: + p.X = -50 + p.Y = -60 + p.Z = -70 + + assert all([x == -50 for x in points.X]) + assert all([y == -60 for y in points.Y]) + assert all([z == -70 for z in points.Z]) + + +def test_points_indexer_setter(): + points = copc.Points( + 3, copc.Vector3.DefaultScale(), copc.Vector3.DefaultOffset(), 4 + ) + + # generate points + num_points = 2000 + for i in range(num_points): + points.AddPoint(points.CreatePoint()) + + assert all([x == 0 for x in points.X]) + assert all([y == 0 for y in points.Y]) + assert all([z == 0 for z in points.Z]) + + points[0].X = 20 + points[0].Y = 40 + points[0].Z = 80 + points[2].Intensity = 60000 + + assert points[0].X == 20 + assert points[0].Y == 40 + assert points[0].Z == 80 + assert points[2].Intensity == 60000 + + # test slicing setters + points[:].X = [2] * len(points) + points[:].Y = [4] * len(points) + points[:].Z = [8] * len(points) + + assert all([x == 2 for x in points.X]) + assert all([x == 4 for x in points.Y]) + assert all([x == 8 for x in points.Z]) + + # TODO: Allow the user to set all values the same + points[1000:].X = [-2] * 1000 + points[1500:1600].Y = [-4] * 100 + points[-5:].Z = [-8] * 5 + + assert all([x == -2 for i, x in enumerate(points.X) if i >= 1000]) + assert all([x != -2 for i, x in enumerate(points.X) if not i >= 1000]) + assert all([x == -4 for i, x in enumerate(points.Y) if i >= 1500 and i < 1600]) + assert all( + [x != -4 for i, x in enumerate(points.Y) if not (i >= 1500 and i < 1600)] + ) + assert all([x == -8 for i, x in enumerate(points.Z) if i >= len(points) - 5]) + assert all([x != -8 for i, x in enumerate(points.Z) if not (i >= len(points) - 5)]) diff --git a/test/reader_node_test.cpp b/test/reader_node_test.cpp index 9de15590..2659a1b9 100644 --- a/test/reader_node_test.cpp +++ b/test/reader_node_test.cpp @@ -75,71 +75,71 @@ TEST_CASE("GetPoints Test", "[Reader] ") REQUIRE(points.size() == hier_entry.point_count); // Getters - REQUIRE(points[0].UnscaledX() == -144147); - REQUIRE(points[0].UnscaledY() == -13541); - REQUIRE(points[0].UnscaledZ() == -227681); - REQUIRE(points[0].Intensity() == 11); - REQUIRE(points[0].NumberOfReturns() == 3); - REQUIRE(points[0].ReturnNumber() == 1); - REQUIRE(points[0].ScanDirectionFlag() == true); - REQUIRE(points[0].EdgeOfFlightLineFlag() == false); - REQUIRE(points[0].Classification() == 5); - REQUIRE(points[0].ScanAngleRank() == 5); - REQUIRE(points[0].ScanAngle() == 5.0); - REQUIRE(points[0].UserData() == 124); - REQUIRE(points[0].PointSourceID() == 7330); - REQUIRE(points[0].GPSTime() > 247570); - REQUIRE(points[0].GPSTime() < 247570.5); - REQUIRE(points[0].Red() == 46); - REQUIRE(points[0].Green() == 60); - REQUIRE(points[0].Blue() == 92); - REQUIRE(points[0].PointFormatID() == 3); - REQUIRE(points[0].PointRecordLength() == 36); - REQUIRE(points[0].ExtraBytes().size() == 2); - REQUIRE_THROWS(points[0].NIR()); - REQUIRE_THROWS(points[0].ExtendedFlagsBitFields()); - REQUIRE_THROWS(points[0].ExtendedReturnsBitFields()); - REQUIRE_THROWS(points[0].ScannerChannel()); - REQUIRE_THROWS(points[0].ExtendedScanAngle()); + REQUIRE(points[0]->UnscaledX() == -144147); + REQUIRE(points[0]->UnscaledY() == -13541); + REQUIRE(points[0]->UnscaledZ() == -227681); + REQUIRE(points[0]->Intensity() == 11); + REQUIRE(points[0]->NumberOfReturns() == 3); + REQUIRE(points[0]->ReturnNumber() == 1); + REQUIRE(points[0]->ScanDirectionFlag() == true); + REQUIRE(points[0]->EdgeOfFlightLineFlag() == false); + REQUIRE(points[0]->Classification() == 5); + REQUIRE(points[0]->ScanAngleRank() == 5); + REQUIRE(points[0]->ScanAngle() == 5.0); + REQUIRE(points[0]->UserData() == 124); + REQUIRE(points[0]->PointSourceID() == 7330); + REQUIRE(points[0]->GPSTime() > 247570); + REQUIRE(points[0]->GPSTime() < 247570.5); + REQUIRE(points[0]->Red() == 46); + REQUIRE(points[0]->Green() == 60); + REQUIRE(points[0]->Blue() == 92); + REQUIRE(points[0]->PointFormatID() == 3); + REQUIRE(points[0]->PointRecordLength() == 36); + REQUIRE(points[0]->ExtraBytes().size() == 2); + REQUIRE_THROWS(points[0]->NIR()); + REQUIRE_THROWS(points[0]->ExtendedFlagsBitFields()); + REQUIRE_THROWS(points[0]->ExtendedReturnsBitFields()); + REQUIRE_THROWS(points[0]->ScannerChannel()); + REQUIRE_THROWS(points[0]->ExtendedScanAngle()); // Setters - points[0].UnscaledX(INT32_MAX); - points[0].UnscaledY(INT32_MAX); - points[0].UnscaledZ(INT32_MAX); - points[0].Intensity(UINT16_MAX); - points[0].NumberOfReturns(7); - points[0].ReturnNumber(7); - points[0].ScanDirectionFlag(false); - points[0].EdgeOfFlightLineFlag(true); - points[0].Classification(31); - points[0].ScanAngleRank(90); - points[0].UserData(UINT8_MAX); - points[0].PointSourceID(UINT8_MAX); - points[0].GPSTime(DBL_MAX); - points[0].Red(UINT16_MAX); - points[0].Green(UINT16_MAX); - points[0].Blue(UINT16_MAX); - REQUIRE_THROWS(points[0].NIR(UINT16_MAX)); - REQUIRE_THROWS(points[0].ExtendedFlagsBitFields(UINT8_MAX)); - REQUIRE_THROWS(points[0].ExtendedReturnsBitFields(UINT8_MAX)); - REQUIRE_THROWS(points[0].ScannerChannel(3)); - REQUIRE_THROWS(points[0].ScanAngle(INT16_MAX)); - - REQUIRE(points[0].UnscaledX() == INT32_MAX); - REQUIRE(points[0].UnscaledY() == INT32_MAX); - REQUIRE(points[0].UnscaledZ() == INT32_MAX); - REQUIRE(points[0].Intensity() == UINT16_MAX); - REQUIRE(points[0].NumberOfReturns() == 7); - REQUIRE(points[0].ReturnNumber() == 7); - REQUIRE(points[0].ScanDirectionFlag() == false); - REQUIRE(points[0].EdgeOfFlightLineFlag() == true); - REQUIRE(points[0].Classification() == 31); - REQUIRE(points[0].ScanAngleRank() == 90); - REQUIRE(points[0].UserData() == UINT8_MAX); - REQUIRE(points[0].PointSourceID() == UINT8_MAX); - REQUIRE(points[0].GPSTime() == DBL_MAX); - REQUIRE(points[0].Red() == UINT16_MAX); - REQUIRE(points[0].Green() == UINT16_MAX); - REQUIRE(points[0].Blue() == UINT16_MAX); + points[0]->UnscaledX(INT32_MAX); + points[0]->UnscaledY(INT32_MAX); + points[0]->UnscaledZ(INT32_MAX); + points[0]->Intensity(UINT16_MAX); + points[0]->NumberOfReturns(7); + points[0]->ReturnNumber(7); + points[0]->ScanDirectionFlag(false); + points[0]->EdgeOfFlightLineFlag(true); + points[0]->Classification(31); + points[0]->ScanAngleRank(90); + points[0]->UserData(UINT8_MAX); + points[0]->PointSourceID(UINT8_MAX); + points[0]->GPSTime(DBL_MAX); + points[0]->Red(UINT16_MAX); + points[0]->Green(UINT16_MAX); + points[0]->Blue(UINT16_MAX); + REQUIRE_THROWS(points[0]->NIR(UINT16_MAX)); + REQUIRE_THROWS(points[0]->ExtendedFlagsBitFields(UINT8_MAX)); + REQUIRE_THROWS(points[0]->ExtendedReturnsBitFields(UINT8_MAX)); + REQUIRE_THROWS(points[0]->ScannerChannel(3)); + REQUIRE_THROWS(points[0]->ScanAngle(INT16_MAX)); + + REQUIRE(points[0]->UnscaledX() == INT32_MAX); + REQUIRE(points[0]->UnscaledY() == INT32_MAX); + REQUIRE(points[0]->UnscaledZ() == INT32_MAX); + REQUIRE(points[0]->Intensity() == UINT16_MAX); + REQUIRE(points[0]->NumberOfReturns() == 7); + REQUIRE(points[0]->ReturnNumber() == 7); + REQUIRE(points[0]->ScanDirectionFlag() == false); + REQUIRE(points[0]->EdgeOfFlightLineFlag() == true); + REQUIRE(points[0]->Classification() == 31); + REQUIRE(points[0]->ScanAngleRank() == 90); + REQUIRE(points[0]->UserData() == UINT8_MAX); + REQUIRE(points[0]->PointSourceID() == UINT8_MAX); + REQUIRE(points[0]->GPSTime() == DBL_MAX); + REQUIRE(points[0]->Red() == UINT16_MAX); + REQUIRE(points[0]->Green() == UINT16_MAX); + REQUIRE(points[0]->Blue() == UINT16_MAX); } } diff --git a/test/reader_test.py b/test/reader_test.py index 59c22553..e67eed1e 100644 --- a/test/reader_test.py +++ b/test/reader_test.py @@ -89,8 +89,8 @@ def test_point_error_handling(): reader.GetPoints(invalid_node) reader.GetPoints(valid_node) - assert len(reader.GetPoints(invalid_node.key).Get()) == 0 - assert len(reader.GetPoints(valid_node.key).Get()) != 0 + assert len(reader.GetPoints(invalid_node.key)) == 0 + assert len(reader.GetPoints(valid_node.key)) != 0 with pytest.raises(RuntimeError): reader.GetPointDataCompressed(invalid_node)