[c++] Column abstraction: SOMAGeometryColumn, part 3
#3427
Conversation
XanthosXanthopoulos
changed the title
SOMAGeometryColumn, part 3SOMAGeometryColumn, part 3
There was a problem hiding this comment.
@XanthosXanthopoulos thanks for working on this! The split-up makes this PR much easier to review.
|
|
||
| // Create a range object and reuse if for all dimensions | ||
| std::vector<std::pair<double_t, double_t>> range(1); | ||
| size_t dimensionality = dimensions.size() / 2; |
There was a problem hiding this comment.
Please explain the magic number 2 here
| transformed_points = _transform_points( | ||
| std::any_cast<std::span<const std::vector<double_t>>>(points)); | ||
|
|
||
| auto domain_limits = _limits(ctx, *query->schema()); |
There was a problem hiding this comment.
Core has current domain and domain. SOMA has domain and maxdomain.
Saying "domain" is ambiguous.
I've been consistent to say "core_domain", "core_current_domain"/"current_domain", "soma_domain", "soma_maxdomain". Please do the same here and make the variable name indicate whether this domain variable is intended to be core domain or soma domain (which are not the same).
| } | ||
|
|
||
| void SOMAGeometryColumn::_set_current_domain_slot( | ||
| NDRectangle& rectangle, std::span<const std::any> domain) const { |
There was a problem hiding this comment.
As above: we must not use the plain variable name domain without qualification, as it is ambiguous and error-prone.
|
|
||
| void SOMAGeometryColumn::_set_current_domain_slot( | ||
| NDRectangle& rectangle, std::span<const std::any> domain) const { | ||
| if (2 * domain.size() != dimensions.size()) { |
There was a problem hiding this comment.
Please explain the magic number 2 here.
|
|
||
| std::pair<bool, std::string> SOMAGeometryColumn::_can_set_current_domain_slot( | ||
| std::optional<NDRectangle>& rectangle, | ||
| std::span<const std::any> new_domain) const { |
There was a problem hiding this comment.
| std::span<const std::any> new_domain) const { | |
| std::span<const std::any> new_current_domain) const { |
| std::span<const std::any> new_domain) const { | ||
| if (new_domain.size() != dimensions.size() / 2) { | ||
| throw TileDBSOMAError(std::format( | ||
| "[SOMADimension][_can_set_current_domain_slot] Expected domain " |
There was a problem hiding this comment.
| "[SOMADimension][_can_set_current_domain_slot] Expected domain " | |
| "[SOMADimension][_can_set_current_domain_slot] Expected current_domain " |
| const SOMAContext& ctx, const ArraySchema& schema) const { | ||
| std::vector<std::pair<double_t, double_t>> limits; | ||
|
|
||
| for (size_t i = 0; i < dimensions.size() / 2; ++i) { |
There was a problem hiding this comment.
Please explain the magic number 2 here
| ranges) const { | ||
| if (ranges.size() != 1) { | ||
| throw TileDBSOMAError( | ||
| "Multi ranges are not supported for geometry dimension"); |
There was a problem hiding this comment.
| "Multi ranges are not supported for geometry dimension"); | |
| "Multiranges are not supported for geometry dimension"); |
| const std::span<const std::vector<double_t>>& points) const { | ||
| if (points.size() != 1) { | ||
| throw TileDBSOMAError( | ||
| "Multi points are not supported for geometry dimension"); |
There was a problem hiding this comment.
| "Multi points are not supported for geometry dimension"); | |
| "Multipoints are not supported for geometry dimension"); |
| @@ -49,9 +49,6 @@ | |||
| #include "soma/column_buffer.h" | |||
| #include "soma/soma_array.h" | |||
| #include "soma/soma_collection.h" | |||
| #include "soma/soma_column.h" | |||
There was a problem hiding this comment.
I don't understand why these lines are being deleted on this PR
There was a problem hiding this comment.
They are intended to be used internally (within SOMAArray and subclasses) so exporting them seemed unnecessary. In any case if the usage pattern changes, we can export them again or do it preemptively now, I have no opinion on that.
XanthosXanthopoulos
force-pushed
the
xan/sc-59427/soma-geometry-column
branch
from
fc53e6e to
497224a
Compare
XanthosXanthopoulos
force-pushed
the
xan/sc-59427/soma-attribute
branch
2 times, most recently
from
73865cb to
5bcd88d
Compare
XanthosXanthopoulos
force-pushed
the
xan/sc-59427/soma-geometry-column
branch
from
497224a to
e02f78e
Compare
XanthosXanthopoulos
force-pushed
the
xan/sc-59427/soma-attribute
branch
from
a215ce4 to
b0a1976
Compare
…nt domain checks, replace vector with span when selecting points
Co-authored-by: Julia Dark <[email protected]>
Co-authored-by: Julia Dark <[email protected]>
XanthosXanthopoulos
force-pushed
the
xan/sc-59427/soma-attribute
branch
from
b0a1976 to
3faefd9
Compare
…nt domain checks, replace vector with span when selecting points
…nt domain checks, replace vector with span when selecting points
XanthosXanthopoulos
force-pushed
the
xan/sc-59427/soma-geometry-column
branch
from
e02f78e to
6f4d6a2
Compare
This PR introduces the
SOMAGeometryColumnconcrete class that implement a spatial index WKB (Well-Known Binary) column. A set of internal TileDB dimensions are used to provide the index mechanics as well as a TileDB Attribute to hold the geometry data encoded in a WKB blob.