Skip to content

Commit 64ca69c

Browse files
committed
Separate the Arrow layer from the Python bindings
geoarrow.cpp had grown to 1876 lines holding two quite different jobs: the Arrow work - reading GeoArrow arrays, splitting them, writing the pieces - and the Python work of where batches come from, who holds the GIL, and the stream handed back over the PyCapsule interface. Splitting them along that line was not only about size. extension's Catch2 target builds without pybind11, so while the Arrow code sat beside the bindings it could not be compiled into the test suite at all: none of it has ever had a C++ test, and every check on it went through geopandas. Now geoarrow_arrays.cpp has no pybind11 in it, run_tests compiles it directly, and tests_geoarrow.cpp goes at the layer as it is - building WKB byte by byte, so a test states its input rather than borrowing the code under test to produce it. The eight new cases cover what the Python suite reaches only indirectly: the per-feature type refusal and the row it names, a mixed batch of a point, a line and an empty collection, multi-part explosion, nesting two collections deep, polygons with holes, the exported schema of all three encodings, and a null being refused rather than dropped. Checked that they bite: removing the null_feat callback, the empty-geometry branch, or the type check each fails one, with no Python involved. Two small shape changes fell out of drawing the line. The batch splitters took an InputStream, which is Python's; they now take the WKB reader and a count, and there is one entry point per encoding, so the readers stay internal. And BatchData holds a unique_ptr to the WKB writer, so its destructor is defined out of line and splitMixedBatch makes the writer itself - which is what keeps WkbWriter out of the header. Nothing else moved: pieces are bit-identical on both geometry types, all 128 Python tests pass, and ru_maxrss is flat across the fourteen stream lifecycles. geoarrow.cpp 1876 -> 573 geoarrow_arrays.cpp 1282 geoarrow_arrays.hpp 160 tests_geoarrow.cpp 320 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SqdWshmD4AUHhrqeMh86GS
1 parent c2ab64d commit 64ca69c

6 files changed

Lines changed: 1794 additions & 1320 deletions

File tree

CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ find_package(Python REQUIRED COMPONENTS Interpreter Development.Module)
2323
find_package(pybind11 CONFIG REQUIRED)
2424

2525
python_add_library(intersections MODULE ${CPP_SRC_DIR}/intersections.cpp WITH_SOABI
26-
${CPP_SRC_DIR}/geoarrow.cpp ${CPP_SRC_DIR}/geometry.cpp
26+
${CPP_SRC_DIR}/geoarrow.cpp ${CPP_SRC_DIR}/geoarrow_arrays.cpp
27+
${CPP_SRC_DIR}/geometry.cpp
2728
${CPP_SRC_DIR}/operations.cpp
2829
${NANOARROW_DIR}/nanoarrow.c
2930
${GEOARROW_DIR}/geoarrow.c)

extension/CMakeLists.txt

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,37 @@
11
cmake_minimum_required(VERSION 3.15...3.26)
2-
project(snail)
2+
# C as well as C++: the vendored amalgamations are C99
3+
project(snail LANGUAGES C CXX)
34

45
set(CMAKE_CXX_STANDARD 17)
56
set(CMAKE_CXX_STANDARD_REQUIRED ON)
67
set(CMAKE_CXX_EXTENSIONS OFF)
78

89
set(CPP_SRC_DIR src)
910
set(CPP_TESTS_DIR tests)
11+
# The Arrow layer is built here too. It has no pybind11 in it - that is the
12+
# whole point of it being a file of its own - so the Catch2 suite can reach
13+
# the reading, splitting and writing of GeoArrow directly, which it could
14+
# not while that code sat beside the bindings.
15+
set(VENDOR_DIR vendor)
1016

1117
add_subdirectory(extern/Catch2)
1218
add_executable(run_tests
1319
${CPP_TESTS_DIR}/run_tests.cpp
1420
${CPP_TESTS_DIR}/tests_intersections.cpp
21+
${CPP_TESTS_DIR}/tests_geoarrow.cpp
1522
${CPP_TESTS_DIR}/tests_stress.cpp
1623
${CPP_TESTS_DIR}/tests_transform.cpp
24+
${CPP_SRC_DIR}/geoarrow_arrays.cpp
1725
${CPP_SRC_DIR}/geometry.cpp
1826
${CPP_SRC_DIR}/operations.cpp
27+
${VENDOR_DIR}/nanoarrow/nanoarrow.c
28+
${VENDOR_DIR}/geoarrow/geoarrow.c
1929
)
30+
set_target_properties(run_tests PROPERTIES C_STANDARD 99 C_STANDARD_REQUIRED ON)
2031
target_include_directories(run_tests PUBLIC ${CPP_SRC_DIR})
32+
# SYSTEM: vendored code is not ours to keep warning-free
33+
target_include_directories(run_tests SYSTEM PRIVATE
34+
${VENDOR_DIR} ${VENDOR_DIR}/nanoarrow)
2135
target_link_libraries(run_tests PRIVATE Catch2::Catch2)
2236

2337
add_executable(run_benchmarks

0 commit comments

Comments
 (0)