Skip to content

Commit

Permalink
refactor: Separate out visitor-based native array writer from buffer-…
Browse files Browse the repository at this point in the history 
…based writer for native arrays (#114)

This PR improves the consistency of the C API by adding the
`GeoArrowNativeWriter` to match the `GeoArrowWKTWriter` and
`GeoArrowWKBWriter`. Before, the `GeoArrowBuilder` served the purpose of
both building by buffer and building by visitor, which was slightly
confusing since it could be used to build WKB and WKT arrays too (except
when using the visitor).

This hopefully makes it more clear that the `GeoArrowArrayWriter` and
`GeoArrowArrayReader` are the things you want if you are reading
arbitrary input/generating arbitrary output.
  • Loading branch information
@paleolimbot
paleolimbot authored Jan 13, 2025
1 parent db02936 commit 1b1ee2f
Show file tree
Hide file tree
Showing 14 changed files with 1,575 additions and 1,501 deletions.
4 changes: 4 additions & 0 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ set(GEOARROW_SOURCES
src/geoarrow/array_view.c
src/geoarrow/util.c
src/geoarrow/visitor.c
src/geoarrow/native_writer.c
src/geoarrow/wkb_reader.c
src/geoarrow/wkb_writer.c
src/geoarrow/wkt_reader.c
Expand Down Expand Up @@ -262,6 +263,7 @@ if(GEOARROW_BUILD_TESTS)
add_executable(kernel_test src/geoarrow/kernel_test.cc)
add_executable(visitor_test src/geoarrow/visitor_test.cc)
add_executable(util_test src/geoarrow/util_test.cc)
add_executable(native_writer_test src/geoarrow/native_writer_test.cc)
add_executable(wkb_reader_test src/geoarrow/wkb_reader_test.cc)
add_executable(wkb_writer_test src/geoarrow/wkb_writer_test.cc)
add_executable(wkt_reader_test src/geoarrow/wkt_reader_test.cc)
Expand All @@ -286,6 +288,7 @@ if(GEOARROW_BUILD_TESTS)
target_link_libraries(metadata_test geoarrow gtest_main nanoarrow::nanoarrow)
target_link_libraries(visitor_test geoarrow gtest_main)
target_link_libraries(util_test geoarrow gtest_main)
target_link_libraries(native_writer_test geoarrow gtest_main nanoarrow::nanoarrow)
target_link_libraries(wkb_reader_test geoarrow gtest_main nanoarrow::nanoarrow)
target_link_libraries(wkb_writer_test geoarrow gtest_main nanoarrow::nanoarrow)
target_link_libraries(wkt_reader_test geoarrow gtest_main nanoarrow::nanoarrow)
Expand Down Expand Up @@ -319,6 +322,7 @@ if(GEOARROW_BUILD_TESTS)
gtest_discover_tests(metadata_test)
gtest_discover_tests(visitor_test)
gtest_discover_tests(util_test)
gtest_discover_tests(native_writer_test)
gtest_discover_tests(wkb_reader_test)
gtest_discover_tests(wkb_writer_test)
gtest_discover_tests(wkt_reader_test)
Expand Down
14 changes: 7 additions & 7 deletions src/geoarrow/array_reader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ struct GeoArrowArrayReaderPrivate {
struct GeoArrowWKBReader wkb_reader;
};

static GeoArrowErrorCode GeoArrowArrayViewVisitWKT(
static GeoArrowErrorCode GeoArrowArrayViewVisitNativeWKT(
const struct GeoArrowArrayView* array_view, int64_t offset, int64_t length,
struct GeoArrowWKTReader* reader, struct GeoArrowVisitor* v) {
struct GeoArrowStringView item;
Expand All @@ -31,7 +31,7 @@ static GeoArrowErrorCode GeoArrowArrayViewVisitWKT(
return GEOARROW_OK;
}

static GeoArrowErrorCode GeoArrowArrayViewVisitWKB(
static GeoArrowErrorCode GeoArrowArrayViewVisitNativeWKB(
const struct GeoArrowArrayView* array_view, int64_t offset, int64_t length,
struct GeoArrowWKBReader* reader, struct GeoArrowVisitor* v) {
struct GeoArrowBufferView item;
Expand Down Expand Up @@ -160,13 +160,13 @@ GeoArrowErrorCode GeoArrowArrayReaderVisit(struct GeoArrowArrayReader* reader,

switch (private_data->array_view.schema_view.type) {
case GEOARROW_TYPE_WKT:
return GeoArrowArrayViewVisitWKT(&private_data->array_view, offset, length,
&private_data->wkt_reader, v);
return GeoArrowArrayViewVisitNativeWKT(&private_data->array_view, offset, length,
&private_data->wkt_reader, v);
case GEOARROW_TYPE_WKB:
return GeoArrowArrayViewVisitWKB(&private_data->array_view, offset, length,
&private_data->wkb_reader, v);
return GeoArrowArrayViewVisitNativeWKB(&private_data->array_view, offset, length,
&private_data->wkb_reader, v);
default:
return GeoArrowArrayViewVisit(&private_data->array_view, offset, length, v);
return GeoArrowArrayViewVisitNative(&private_data->array_view, offset, length, v);
}
}

Expand Down
34 changes: 17 additions & 17 deletions src/geoarrow/array_view.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ static inline void GeoArrowCoordViewUpdate(const struct GeoArrowCoordView* src,
dst->n_coords = length;
}

static GeoArrowErrorCode GeoArrowArrayViewVisitPoint(
static GeoArrowErrorCode GeoArrowArrayViewVisitNativePoint(
const struct GeoArrowArrayView* array_view, int64_t offset, int64_t length,
struct GeoArrowVisitor* v) {
struct GeoArrowCoordView coords = array_view->coords;
Expand Down Expand Up @@ -306,7 +306,7 @@ static GeoArrowErrorCode GeoArrowArrayViewVisitPoint(
return GEOARROW_OK;
}

static GeoArrowErrorCode GeoArrowArrayViewVisitLinestring(
static GeoArrowErrorCode GeoArrowArrayViewVisitNativeLinestring(
const struct GeoArrowArrayView* array_view, int64_t offset, int64_t length,
struct GeoArrowVisitor* v) {
struct GeoArrowCoordView coords = array_view->coords;
Expand Down Expand Up @@ -336,7 +336,7 @@ static GeoArrowErrorCode GeoArrowArrayViewVisitLinestring(
return GEOARROW_OK;
}

static GeoArrowErrorCode GeoArrowArrayViewVisitPolygon(
static GeoArrowErrorCode GeoArrowArrayViewVisitNativePolygon(
const struct GeoArrowArrayView* array_view, int64_t offset, int64_t length,
struct GeoArrowVisitor* v) {
struct GeoArrowCoordView coords = array_view->coords;
Expand Down Expand Up @@ -377,7 +377,7 @@ static GeoArrowErrorCode GeoArrowArrayViewVisitPolygon(
return GEOARROW_OK;
}

static GeoArrowErrorCode GeoArrowArrayViewVisitMultipoint(
static GeoArrowErrorCode GeoArrowArrayViewVisitNativeMultipoint(
const struct GeoArrowArrayView* array_view, int64_t offset, int64_t length,
struct GeoArrowVisitor* v) {
struct GeoArrowCoordView coords = array_view->coords;
Expand Down Expand Up @@ -412,7 +412,7 @@ static GeoArrowErrorCode GeoArrowArrayViewVisitMultipoint(
return GEOARROW_OK;
}

static GeoArrowErrorCode GeoArrowArrayViewVisitMultilinestring(
static GeoArrowErrorCode GeoArrowArrayViewVisitNativeMultilinestring(
const struct GeoArrowArrayView* array_view, int64_t offset, int64_t length,
struct GeoArrowVisitor* v) {
struct GeoArrowCoordView coords = array_view->coords;
Expand Down Expand Up @@ -454,7 +454,7 @@ static GeoArrowErrorCode GeoArrowArrayViewVisitMultilinestring(
return GEOARROW_OK;
}

static GeoArrowErrorCode GeoArrowArrayViewVisitMultipolygon(
static GeoArrowErrorCode GeoArrowArrayViewVisitNativeMultipolygon(
const struct GeoArrowArrayView* array_view, int64_t offset, int64_t length,
struct GeoArrowVisitor* v) {
struct GeoArrowCoordView coords = array_view->coords;
Expand Down Expand Up @@ -509,7 +509,7 @@ static GeoArrowErrorCode GeoArrowArrayViewVisitMultipolygon(
return GEOARROW_OK;
}

static GeoArrowErrorCode GeoArrowArrayViewVisitBox(
static GeoArrowErrorCode GeoArrowArrayViewVisitNativeBox(
const struct GeoArrowArrayView* array_view, int64_t offset, int64_t length,
struct GeoArrowVisitor* v) {
// We aren't going to attempt Z, M, or ZM boxes since there is no canonical
Expand Down Expand Up @@ -581,24 +581,24 @@ static GeoArrowErrorCode GeoArrowArrayViewVisitBox(
return GEOARROW_OK;
}

GeoArrowErrorCode GeoArrowArrayViewVisit(const struct GeoArrowArrayView* array_view,
int64_t offset, int64_t length,
struct GeoArrowVisitor* v) {
GeoArrowErrorCode GeoArrowArrayViewVisitNative(const struct GeoArrowArrayView* array_view,
int64_t offset, int64_t length,
struct GeoArrowVisitor* v) {
switch (array_view->schema_view.geometry_type) {
case GEOARROW_GEOMETRY_TYPE_BOX:
return GeoArrowArrayViewVisitBox(array_view, offset, length, v);
return GeoArrowArrayViewVisitNativeBox(array_view, offset, length, v);
case GEOARROW_GEOMETRY_TYPE_POINT:
return GeoArrowArrayViewVisitPoint(array_view, offset, length, v);
return GeoArrowArrayViewVisitNativePoint(array_view, offset, length, v);
case GEOARROW_GEOMETRY_TYPE_LINESTRING:
return GeoArrowArrayViewVisitLinestring(array_view, offset, length, v);
return GeoArrowArrayViewVisitNativeLinestring(array_view, offset, length, v);
case GEOARROW_GEOMETRY_TYPE_POLYGON:
return GeoArrowArrayViewVisitPolygon(array_view, offset, length, v);
return GeoArrowArrayViewVisitNativePolygon(array_view, offset, length, v);
case GEOARROW_GEOMETRY_TYPE_MULTIPOINT:
return GeoArrowArrayViewVisitMultipoint(array_view, offset, length, v);
return GeoArrowArrayViewVisitNativeMultipoint(array_view, offset, length, v);
case GEOARROW_GEOMETRY_TYPE_MULTILINESTRING:
return GeoArrowArrayViewVisitMultilinestring(array_view, offset, length, v);
return GeoArrowArrayViewVisitNativeMultilinestring(array_view, offset, length, v);
case GEOARROW_GEOMETRY_TYPE_MULTIPOLYGON:
return GeoArrowArrayViewVisitMultipolygon(array_view, offset, length, v);
return GeoArrowArrayViewVisitNativeMultipolygon(array_view, offset, length, v);
default:
return ENOTSUP;
}
Expand Down
77 changes: 46 additions & 31 deletions src/geoarrow/array_view_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidBox) {
EXPECT_EQ(array_view.coords.values[3][1], -INFINITY);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 3);
EXPECT_EQ(values[0], "POLYGON ((0 1, 2 1, 2 3, 0 3, 0 1))");
Expand All @@ -270,7 +271,7 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidBoxNonXY) {
struct GeoArrowVisitor v;
GeoArrowVisitorInitVoid(&v);
v.error = &error;
ASSERT_EQ(GeoArrowArrayViewVisit(&array_view, 0, 0, &v), ENOTSUP);
ASSERT_EQ(GeoArrowArrayViewVisitNative(&array_view, 0, 0, &v), ENOTSUP);
ASSERT_STREQ(error.message, "Can't visit box with non-XY dimensions");

ArrowSchemaRelease(&schema);
Expand Down Expand Up @@ -307,8 +308,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidPoint) {
EXPECT_EQ(array_view.coords.values[1][0], 10);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 2);
EXPECT_EQ(values[0], "POINT (30 10)");
Expand Down Expand Up @@ -349,8 +351,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidInterleavedPoint) {
EXPECT_EQ(array_view.coords.values[1][0], 10);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 2);
EXPECT_EQ(values[0], "POINT (30 10)");
Expand Down Expand Up @@ -402,8 +405,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidPointWithOffset) {
EXPECT_EQ(array_view.length[0], 2);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 2);
EXPECT_EQ(values[0], "<null value>");
Expand Down Expand Up @@ -453,8 +457,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidInterleavedPointWithOffset) {
EXPECT_EQ(array_view.length[0], 2);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 2);
EXPECT_EQ(values[0], "<null value>");
Expand Down Expand Up @@ -508,8 +513,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidLinestring) {
EXPECT_EQ(array_view.coords.values[1][1], 1);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 3);
EXPECT_EQ(values[0], "LINESTRING (30 10, 0 1)");
Expand Down Expand Up @@ -569,8 +575,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidLinestringWithOffset) {
EXPECT_EQ(array_view.length[1], 3);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 2);
EXPECT_EQ(values[0], "<null value>");
Expand Down Expand Up @@ -646,8 +653,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidPolygon) {
EXPECT_EQ(array_view.coords.values[1][3], 2);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 3);
EXPECT_EQ(values[0], "POLYGON ((1 2, 2 3, 4 5, 1 2))");
Expand Down Expand Up @@ -733,8 +741,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidPolygonWithOffset) {
EXPECT_EQ(array_view.length[2], 4);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 2);
EXPECT_EQ(values[0], "<null value>");
Expand Down Expand Up @@ -788,8 +797,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidMultipoint) {
EXPECT_EQ(array_view.coords.values[1][1], 1);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 3);
EXPECT_EQ(values[0], "MULTIPOINT ((30 10), (0 1))");
Expand Down Expand Up @@ -851,8 +861,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidMultiPointWithOffset) {
EXPECT_EQ(array_view.length[1], 3);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 2);
EXPECT_EQ(values[0], "<null value>");
Expand Down Expand Up @@ -932,8 +943,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidMultilinestring) {
EXPECT_EQ(array_view.coords.values[1][3], 2);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 3);
EXPECT_EQ(values[0], "MULTILINESTRING ((30 10, 0 1), (31 11, 1 2))");
Expand Down Expand Up @@ -1021,8 +1033,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidMultiLinestringWithOffset) {
EXPECT_EQ(array_view.length[2], 4);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 2);
EXPECT_EQ(values[0], "<null value>");
Expand Down Expand Up @@ -1114,8 +1127,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidMultipolygon) {
EXPECT_EQ(array_view.coords.values[1][3], 2);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 3);
EXPECT_EQ(values[0], "MULTIPOLYGON (((1 2, 2 3, 4 5, 1 2)))");
Expand Down Expand Up @@ -1223,8 +1237,9 @@ TEST(ArrayViewTest, ArrayViewTestSetArrayValidMultipolygonWithOffsets) {
EXPECT_EQ(array_view.length[3], 4);

WKXTester tester;
EXPECT_EQ(GeoArrowArrayViewVisit(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
EXPECT_EQ(
GeoArrowArrayViewVisitNative(&array_view, 0, array.length, tester.WKTVisitor()),
GEOARROW_OK);
auto values = tester.WKTValues("<null value>");
ASSERT_EQ(values.size(), 2);
EXPECT_EQ(values[0], "<null value>");
Expand Down
Loading

0 comments on commit 1b1ee2f

Please sign in to comment.