Skip to content

feat: ToGeos impl#1455

Open
ianthetechie wants to merge 6 commits into
geoarrow:mainfrom
ianthetechie:to-geos-impl
Open

feat: ToGeos impl#1455
ianthetechie wants to merge 6 commits into
geoarrow:mainfrom
ianthetechie:to-geos-impl

Conversation

@ianthetechie

@ianthetechie ianthetechie commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

This is a follow-up to https://github.com/datafusion-contrib/geodatafusion/pull/78/changes#r3357780088. This ended up bit larger than I'd hoped but I've broken things up into more or less atomic commits to ease the review.

  • Adds a ToGeos trait. The "from" half already exists, so we now have reusable foundations for mapping over a GeoArrow array.
  • Upgrades the GEOS crate to version 11 (matches geodatafusion).
  • Adds support for M coordinate operations (there's a geed chance I'll want these for some of my work, and since GEOS finally supports it, this will make more operations "just work" in ways that didn't before :D)
  • Fix (IMO) some existing code to be less panick-y. There are still a lot of intentional panics which could be errors, but I tried to only touch the ones in nearby code.

@github-actions github-actions Bot added the feat label Jun 16, 2026
@ianthetechie

Copy link
Copy Markdown
Contributor Author

Here's an example of how it's used in a non-trivial map (the line merge example):

    // Convert array to GEOS types, merge each geometry, then convert back.
    let merged = geo_array
        .to_geos()?
        .into_iter()
        .map(|maybe_geom| {
            maybe_geom
                .map(|geom| {
                    if geom.is_empty()? {
                        Ok(geom)
                    } else if directed {
                        geom.line_merge_directed()
                    } else {
                        geom.line_merge()
                    }
                })
                .transpose()
        })
        .collect::<std::result::Result<Vec<_>, geos::Error>>()?;

    // Convert the merged GEOS geometries back into a GeoArrow `GeometryArray`.
    let to_type = GeoArrowType::from_arrow_field(args.return_field.as_ref())?;
    let GeoArrowType::Geometry(geometry_type) = to_type else {
        return Err(DataFusionError::Internal(
            "ST_LineMerge expected a Geometry return type".to_string(),
        )
        .into());
    };
    let result = GeometryArray::from_geos(merged, geometry_type)?;

The discussion on the other PR was a bit open-ended as to what the style and scope of refactoring out would be so I'd like some feedback on the initial design first. Then I can work on the further abstractions.

I think you're suggesting a line merge algorithm in geoarrow-expr-geo which geodatafusion would then import, right?

It might also make sense to have some sort of "map" helper to make these "kernels" easier to write. There's still a fair bit of boilerplate, particularly if you want the result back in a geoarrow array (the datafusion use case).

rust-version = { workspace = true }

[features]
geos-3_14 = ["geos/v3_14_0"]

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.

This is not enabled by default, but somewhat surprisingly (to me) geos is not feature flagged in here. I've used the same style of feature here as in geodatafusion, but under the hood this crate is still currently more like the geos crate in that there isn't an explicit version feature that tells you the floor.

This was implcitly added via protobuf,
which is no longer directly used.
Comment on lines +24 to +39
#[cfg(not(feature = "geos-3_14"))]
{
match geom.get_coordinate_dimension().unwrap() {
geos::CoordDimensions::TwoD => geo_traits::Dimensions::Xy,
geos::CoordDimensions::ThreeD => geo_traits::Dimensions::Xyz,
}
}
#[cfg(feature = "geos-3_14")]
{
match (geom.has_z().unwrap(), geom.has_m().unwrap()) {
(false, false) => geo_traits::Dimensions::Xy,
(true, false) => geo_traits::Dimensions::Xyz,
(false, true) => geo_traits::Dimensions::Xym,
(true, true) => geo_traits::Dimensions::Xyzm,
}
}

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.

This and the other paired cfg+inverse blocks could be a lot prettier if you're OK with an MSRV bump to 1.95.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant