Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Separate out visitor-based native array writer from buffer-based writer for native arrays #114

Merged
merged 12 commits into from
Jan 13, 2025
Merged

refactor: Separate out visitor-based native array writer from buffer-based writer for native arrays #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e4b3657
start new object
paleolimbot Jan 9, 2025
1c89685
docs
paleolimbot Jan 9, 2025
df3f7c1
copy some stuff over
paleolimbot Jan 9, 2025
d1b6e24
maybe working
paleolimbot Jan 12, 2025
bcc2d98
passing point test
paleolimbot Jan 12, 2025
2520c01
port tests over
paleolimbot Jan 12, 2025
f84a1f1
rename tests
paleolimbot Jan 13, 2025
dc8b692
use native writer in array writer
paleolimbot Jan 13, 2025
8093c39
remove some functions
paleolimbot Jan 13, 2025
dd35c82
remove dead code from builder
paleolimbot Jan 13, 2025
31a23d9
one more
paleolimbot Jan 13, 2025
d31f4e1
rename array view reader
paleolimbot Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading