Skip to content

feat(vortex-geo): Arrow import/export for the native Point type#8374

Merged
HarukiMoriarty merged 4 commits into
developfrom
nemo/geo-point-arrow
Jun 12, 2026
Merged

feat(vortex-geo): Arrow import/export for the native Point type#8374
HarukiMoriarty merged 4 commits into
developfrom
nemo/geo-point-arrow

Conversation

@HarukiMoriarty

Copy link
Copy Markdown
Contributor

Summary

This PR adds support for import/export to Arrow for the Point extension type, as the
geoarrow.point Arrow extension with separated (struct) coordinates.

Stacked on #8372.

Testing

Unit tests are added to exercise both code paths, plus a Vortex → Arrow → Vortex round-trip.

@codspeed-hq

codspeed-hq Bot commented Jun 11, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚡ 2 improved benchmarks
❌ 4 regressed benchmarks
✅ 1522 untouched benchmarks
⏩ 10 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation varbinview_large 112.9 µs 131.5 µs -14.19%
Simulation decompress_rd[f64, (100000, 0.01)] 845.9 µs 981.6 µs -13.83%
Simulation decompress_rd[f64, (100000, 0.1)] 845.9 µs 981.6 µs -13.82%
Simulation encode_varbin[(1000, 2)] 157.5 µs 176.8 µs -10.92%
Simulation decompress_rd[f64, (100000, 0.0)] 1,024.6 µs 845.9 µs +21.12%
Simulation decompress_rd[f32, (100000, 0.0)] 586.8 µs 499.3 µs +17.51%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing nemo/geo-point-arrow (f7321a0) with develop (46e7253)

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Base automatically changed from nemo/geo-point to develop June 12, 2026 15:22
@HarukiMoriarty HarukiMoriarty requested a review from a team June 12, 2026 15:22
Register ArrowImportVTable/ArrowExportVTable for the Point extension
type, mapping geoarrow.point fields (separated coordinates) to and from
vortex.geo.point with the CRS carried through. Interleaved coordinates
are rejected at field import. The CRS metadata conversion is shared
with the WKB vtables, and the coordinate storage dtype now has a
canonical constructor inverse to coordinate_dimension.

Signed-off-by: Nemo Yu <zyu379@wisc.edu>
@HarukiMoriarty HarukiMoriarty enabled auto-merge (squash) June 12, 2026 15:49
Comment thread vortex-geo/src/extension/coordinate.rs Outdated
Comment on lines +272 to +277
let cases = [
(Dimension::Xy, ["x", "y"].as_slice()),
(Dimension::Xyz, ["x", "y", "z"].as_slice()),
(Dimension::Xym, ["x", "y", "m"].as_slice()),
(Dimension::Xyzm, ["x", "y", "z", "m"].as_slice()),
];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we have `rstest::cases` for this. just search for that in the repo and you'll see several examples. you might need to import rstest into this crate to use it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, also applied to other places.

Comment thread vortex-geo/src/extension/point.rs Outdated
Comment on lines +173 to +178
if point_meta.coord_type() != CoordType::Separated {
vortex_bail!(
"geoarrow.point with interleaved coordinates is not supported; \
re-encode with separated (struct) coordinates"
);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a vortex_ensure!() macro which captures the if-not-vortex_bail flow

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Parameterize the table-driven tests with rstest cases, collapse the
three binary-family WKB export tests into one, and replace the
if-then-vortex_bail flows with vortex_ensure!.

Signed-off-by: Nemo Yu <zyu379@wisc.edu>
@HarukiMoriarty HarukiMoriarty requested a review from a10y June 12, 2026 16:03
@HarukiMoriarty HarukiMoriarty merged commit fb489a1 into develop Jun 12, 2026
60 of 62 checks passed
@HarukiMoriarty HarukiMoriarty deleted the nemo/geo-point-arrow branch June 12, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants