From dbf9cda8e6ae83851a47d633ff50b9c3495a5f5b Mon Sep 17 00:00:00 2001 From: oglego Date: Sun, 19 Apr 2026 13:20:22 -0500 Subject: [PATCH 1/3] feat: ST_IsClosed refactor: update ST_IsClosed implementation and add tests refactor: update ST_IsClosed tests with comprehensive cases chore: fixed format on is_closed.rs feat: add ST_IsClosed to Python bindings and update tests chore: fix formatting refactor: move ST_IsClosed to validation module refactor: move IsClosed from measurement to validation refactor: update error with IsClosed UDF refactor: add pytest validation test for ST_IsClosed UDF --- README.md | 2 +- python/python/geodatafusion/__init__.py | 1 + .../python/geodatafusion/geo/_validation.pyi | 4 + python/src/udf/geo/mod.rs | 1 + python/src/udf/geo/validation.rs | 3 +- python/tests/udf/geo/__init__.py | 0 python/tests/udf/geo/test_validation.py | 15 ++ .../src/udf/geo/validation/is_closed.rs | 185 ++++++++++++++++++ .../src/udf/geo/validation/mod.rs | 3 + 9 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 python/tests/udf/geo/__init__.py create mode 100644 python/tests/udf/geo/test_validation.py create mode 100644 rust/geodatafusion/src/udf/geo/validation/is_closed.rs diff --git a/README.md b/README.md index 23ed6aa..625ab63 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ Functions are explicitly modeled after the [PostGIS API](https://postgis.net/doc | ST_GeometryN | | Return an element of a geometry collection. | | ST_GeometryType | ✅ | Returns the SQL-MM type of a geometry as text. | | ST_InteriorRingN | | Returns the Nth interior ring (hole) of a Polygon. | -| ST_IsClosed | | Tests if a LineStrings's start and end points are coincident. | +| ST_IsClosed | ✅ | Tests if a LineStrings's start and end points are coincident. | | ST_IsCollection | | Tests if a geometry is a geometry collection type. | | ST_IsEmpty | | Tests if a geometry is empty. | | ST_IsPolygonCCW | | Tests if Polygons have exterior rings oriented counter-clockwise and interior rings oriented clockwise. | diff --git a/python/python/geodatafusion/__init__.py b/python/python/geodatafusion/__init__.py index fdf0e00..d7c5c01 100644 --- a/python/python/geodatafusion/__init__.py +++ b/python/python/geodatafusion/__init__.py @@ -43,6 +43,7 @@ def register_all_geo(ctx: SessionContext): ctx.register_udf(udf(geo.Within())) # validation + ctx.register_udf(udf(geo.IsClosed())) ctx.register_udf(udf(geo.IsValid())) ctx.register_udf(udf(geo.IsValidReason())) diff --git a/python/python/geodatafusion/geo/_validation.pyi b/python/python/geodatafusion/geo/_validation.pyi index 4a6f758..44891e1 100644 --- a/python/python/geodatafusion/geo/_validation.pyi +++ b/python/python/geodatafusion/geo/_validation.pyi @@ -1,3 +1,7 @@ +class IsClosed: + def __init__(self) -> None: ... + def __datafusion_scalar_udf__(self) -> object: ... + class IsValid: def __init__(self) -> None: ... def __datafusion_scalar_udf__(self) -> object: ... diff --git a/python/src/udf/geo/mod.rs b/python/src/udf/geo/mod.rs index 7dded2b..c046d4d 100644 --- a/python/src/udf/geo/mod.rs +++ b/python/src/udf/geo/mod.rs @@ -34,6 +34,7 @@ pub(crate) fn geo(m: &Bound) -> PyResult<()> { m.add_class::()?; // validation + m.add_class::()?; m.add_class::()?; m.add_class::()?; diff --git a/python/src/udf/geo/validation.rs b/python/src/udf/geo/validation.rs index e58e0c6..a7690e0 100644 --- a/python/src/udf/geo/validation.rs +++ b/python/src/udf/geo/validation.rs @@ -1,6 +1,7 @@ -use geodatafusion::udf::geo::validation::{IsValid, IsValidReason}; +use geodatafusion::udf::geo::validation::{IsClosed, IsValid, IsValidReason}; use crate::impl_udf; +impl_udf!(IsClosed, PyIsClosed, "IsClosed"); impl_udf!(IsValid, PyIsValid, "IsValid"); impl_udf!(IsValidReason, PyIsValidReason, "IsValidReason"); diff --git a/python/tests/udf/geo/__init__.py b/python/tests/udf/geo/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/python/tests/udf/geo/test_validation.py b/python/tests/udf/geo/test_validation.py new file mode 100644 index 0000000..64d39bf --- /dev/null +++ b/python/tests/udf/geo/test_validation.py @@ -0,0 +1,15 @@ +from __future__ import annotations + +from arro3.core import Table +from datafusion import SessionContext +import geodatafusion +from geodatafusion import register_all + + +def test_st_is_closed_geoarrow(): + ctx = SessionContext() + register_all(ctx) + sql = "SELECT ST_IsClosed(ST_GeomFromText('POLYGON((0 0, 0 1, 1 1, 1 0, 0 0))')) as geom" + df = ctx.sql(sql) + table = df.to_arrow_table() + assert table.column("geom")[0].as_py() is True diff --git a/rust/geodatafusion/src/udf/geo/validation/is_closed.rs b/rust/geodatafusion/src/udf/geo/validation/is_closed.rs new file mode 100644 index 0000000..eb02741 --- /dev/null +++ b/rust/geodatafusion/src/udf/geo/validation/is_closed.rs @@ -0,0 +1,185 @@ +use std::any::Any; +use std::sync::{Arc, OnceLock}; + +use arrow_array::BooleanArray; +use arrow_array::builder::BooleanBuilder; +use arrow_schema::DataType; +use datafusion::error::Result; +use datafusion::logical_expr::scalar_doc_sections::DOC_SECTION_OTHER; +use datafusion::logical_expr::{ + ColumnarValue, Documentation, ScalarFunctionArgs, ScalarUDFImpl, Signature, +}; +use geo_traits::{CoordTrait, GeometryTrait, LineStringTrait, MultiLineStringTrait}; +use geoarrow_array::{GeoArrowArrayAccessor, WrapArray, downcast_geoarrow_array}; +use geoarrow_schema::GeoArrowType; + +use crate::data_types::any_single_geometry_type_input; +use crate::error::GeoDataFusionResult; + +#[derive(Debug, Eq, PartialEq, Hash)] +pub struct IsClosed; + +impl IsClosed { + pub fn new() -> Self { + Self {} + } +} + +impl Default for IsClosed { + fn default() -> Self { + Self::new() + } +} + +static DOCUMENTATION: OnceLock = OnceLock::new(); + +impl ScalarUDFImpl for IsClosed { + fn as_any(&self) -> &dyn Any { + self + } + + fn name(&self) -> &str { + "st_isclosed" + } + + fn signature(&self) -> &Signature { + any_single_geometry_type_input() + } + + fn return_type(&self, _arg_types: &[DataType]) -> Result { + Ok(DataType::Boolean) + } + + fn invoke_with_args(&self, args: ScalarFunctionArgs) -> Result { + Ok(is_closed_impl(args)?) + } + + fn documentation(&self) -> Option<&Documentation> { + Some(DOCUMENTATION.get_or_init(|| { + Documentation::builder( + DOC_SECTION_OTHER, + "Tests if a LineStrings's start and end points are coincident.", + "ST_IsClosed(geom)", + ) + .with_argument("geom", "geometry") + .build() + })) + } +} + +fn is_closed_impl(args: ScalarFunctionArgs) -> GeoDataFusionResult { + let array = ColumnarValue::values_to_arrays(&args.args)? + .into_iter() + .next() + .unwrap(); + let geo_type = GeoArrowType::from_arrow_field(&args.arg_fields[0])?; + let geo_array = geo_type.wrap_array(&array)?; + let geo_array_ref = geo_array.as_ref(); + + let result = downcast_geoarrow_array!(geo_array_ref, impl_is_closed)?; + + Ok(ColumnarValue::Array(Arc::new(result))) +} + +fn impl_is_closed<'a>( + array: &'a impl GeoArrowArrayAccessor<'a>, +) -> GeoDataFusionResult { + let mut builder = BooleanBuilder::with_capacity(array.len()); + + for item in array.iter() { + match item { + Some(geom) => { + let geom = geom?; + let is_closed = match geom.as_type() { + geo_traits::GeometryType::LineString(ls) => check_ls_closed(ls), + geo_traits::GeometryType::MultiLineString(mls) => { + mls.num_line_strings() > 0 + && mls.line_strings().all(|ls| check_ls_closed(&ls)) + } + geo_traits::GeometryType::Point(_) + | geo_traits::GeometryType::MultiPoint(_) + | geo_traits::GeometryType::Polygon(_) + | geo_traits::GeometryType::MultiPolygon(_) => true, + _ => false, + }; + builder.append_value(is_closed); + } + None => { + builder.append_null(); + } + } + } + + Ok(builder.finish()) +} + +fn check_ls_closed(ls: &impl LineStringTrait) -> bool { + let n = ls.num_coords(); + if n < 2 { + return false; + } + if let (Some(first), Some(last)) = (ls.coord(0), ls.coord(n - 1)) { + first.x() == last.x() && first.y() == last.y() + } else { + false + } +} + +#[cfg(test)] +mod test { + use arrow_array::cast::AsArray; + use datafusion::prelude::SessionContext; + + use super::*; + use crate::udf::native::io::GeomFromText; + + #[tokio::test] + async fn test_st_isclosed() { + let ctx = SessionContext::new(); + ctx.register_udf(IsClosed::new().into()); + ctx.register_udf(GeomFromText::new(Default::default()).into()); + + let cases = vec![ + ("LINESTRING(0 0, 1 1, 0 1, 0 0)", true, "closed linestring"), + ("LINESTRING(0 0, 1 1, 1 0)", false, "open linestring"), + ("LINESTRING(0 0, 0 0)", true, "single segment closed"), + ("LINESTRING EMPTY", false, "empty linestring is not closed"), + ( + "MULTILINESTRING((0 0, 1 1, 0 0), (2 2, 3 3, 2 2))", + true, + "all closed", + ), + ( + "MULTILINESTRING((0 0, 1 1, 0 0), (2 2, 3 3, 2 3))", + false, + "one open", + ), + ( + "MULTILINESTRING EMPTY", + false, + "empty multilinestring is not closed", + ), + ("POINT(0 0)", true, "point is closed"), + ("MULTIPOINT(0 0, 1 1)", true, "multipoint is closed"), + ("POLYGON((0 0, 1 0, 1 1, 0 0))", true, "polygon is closed"), + ( + "MULTIPOLYGON(((0 0, 1 0, 1 1, 0 0)))", + true, + "multipolygon is closed", + ), + ]; + + for (wkt, expected, description) in cases { + let sql = format!("SELECT ST_IsClosed(ST_GeomFromText('{}'))", wkt); + let df = ctx + .sql(&sql) + .await + .unwrap_or_else(|_| panic!("Failed to execute SQL for {}", description)); + + let batch = df.collect().await.unwrap().into_iter().next().unwrap(); + let col = batch.column(0).as_boolean(); + + assert_eq!(col.value(0), expected, "Failed on {}: {}", description, wkt); + } + } +} diff --git a/rust/geodatafusion/src/udf/geo/validation/mod.rs b/rust/geodatafusion/src/udf/geo/validation/mod.rs index a0bd2f2..90f146e 100644 --- a/rust/geodatafusion/src/udf/geo/validation/mod.rs +++ b/rust/geodatafusion/src/udf/geo/validation/mod.rs @@ -1,10 +1,13 @@ +mod is_closed; mod is_valid; mod is_valid_reason; +pub use is_closed::IsClosed; pub use is_valid::IsValid; pub use is_valid_reason::IsValidReason; pub fn register(session_context: &datafusion::prelude::SessionContext) { + session_context.register_udf(IsClosed.into()); session_context.register_udf(IsValid.into()); session_context.register_udf(IsValidReason.into()); } From 7a8add68d0fe806868f739aa02a2c8775601907b Mon Sep 17 00:00:00 2001 From: oglego Date: Thu, 23 Apr 2026 15:56:25 -0500 Subject: [PATCH 2/3] refactor: move ST_IsClosed from validation to native accessors --- python/python/geodatafusion/__init__.py | 2 +- python/python/geodatafusion/geo/_validation.pyi | 4 ---- python/python/geodatafusion/native/_accessors.pyi | 4 ++++ python/src/udf/geo/mod.rs | 1 - python/src/udf/geo/validation.rs | 3 +-- python/src/udf/native/accessors.rs | 3 ++- python/src/udf/native/mod.rs | 1 + python/tests/udf/geo/__init__.py | 0 .../udf/{geo/test_validation.py => native/test_accessors.py} | 0 rust/geodatafusion/src/udf/geo/validation/mod.rs | 3 --- .../src/udf/{geo/validation => native/accessors}/is_closed.rs | 0 rust/geodatafusion/src/udf/native/accessors/mod.rs | 3 +++ 12 files changed, 12 insertions(+), 12 deletions(-) delete mode 100644 python/tests/udf/geo/__init__.py rename python/tests/udf/{geo/test_validation.py => native/test_accessors.py} (100%) rename rust/geodatafusion/src/udf/{geo/validation => native/accessors}/is_closed.rs (100%) diff --git a/python/python/geodatafusion/__init__.py b/python/python/geodatafusion/__init__.py index d7c5c01..2ef7463 100644 --- a/python/python/geodatafusion/__init__.py +++ b/python/python/geodatafusion/__init__.py @@ -43,7 +43,6 @@ def register_all_geo(ctx: SessionContext): ctx.register_udf(udf(geo.Within())) # validation - ctx.register_udf(udf(geo.IsClosed())) ctx.register_udf(udf(geo.IsValid())) ctx.register_udf(udf(geo.IsValidReason())) @@ -62,6 +61,7 @@ def register_all_native(ctx: SessionContext): # accessors ctx.register_udf(udf(native.CoordDim())) ctx.register_udf(udf(native.EndPoint())) + ctx.register_udf(udf(native.IsClosed())) ctx.register_udf(udf(native.GeometryType())) ctx.register_udf(udf(native.M())) ctx.register_udf(udf(native.NDims())) diff --git a/python/python/geodatafusion/geo/_validation.pyi b/python/python/geodatafusion/geo/_validation.pyi index 44891e1..4a6f758 100644 --- a/python/python/geodatafusion/geo/_validation.pyi +++ b/python/python/geodatafusion/geo/_validation.pyi @@ -1,7 +1,3 @@ -class IsClosed: - def __init__(self) -> None: ... - def __datafusion_scalar_udf__(self) -> object: ... - class IsValid: def __init__(self) -> None: ... def __datafusion_scalar_udf__(self) -> object: ... diff --git a/python/python/geodatafusion/native/_accessors.pyi b/python/python/geodatafusion/native/_accessors.pyi index cd461d6..4035145 100644 --- a/python/python/geodatafusion/native/_accessors.pyi +++ b/python/python/geodatafusion/native/_accessors.pyi @@ -6,6 +6,10 @@ class EndPoint: def __init__(self) -> None: ... def __datafusion_scalar_udf__(self) -> object: ... +class IsClosed: + def __init__(self) -> None: ... + def __datafusion_scalar_udf__(self) -> object: ... + class NDims: def __init__(self) -> None: ... def __datafusion_scalar_udf__(self) -> object: ... diff --git a/python/src/udf/geo/mod.rs b/python/src/udf/geo/mod.rs index c046d4d..7dded2b 100644 --- a/python/src/udf/geo/mod.rs +++ b/python/src/udf/geo/mod.rs @@ -34,7 +34,6 @@ pub(crate) fn geo(m: &Bound) -> PyResult<()> { m.add_class::()?; // validation - m.add_class::()?; m.add_class::()?; m.add_class::()?; diff --git a/python/src/udf/geo/validation.rs b/python/src/udf/geo/validation.rs index a7690e0..e58e0c6 100644 --- a/python/src/udf/geo/validation.rs +++ b/python/src/udf/geo/validation.rs @@ -1,7 +1,6 @@ -use geodatafusion::udf::geo::validation::{IsClosed, IsValid, IsValidReason}; +use geodatafusion::udf::geo::validation::{IsValid, IsValidReason}; use crate::impl_udf; -impl_udf!(IsClosed, PyIsClosed, "IsClosed"); impl_udf!(IsValid, PyIsValid, "IsValid"); impl_udf!(IsValidReason, PyIsValidReason, "IsValidReason"); diff --git a/python/src/udf/native/accessors.rs b/python/src/udf/native/accessors.rs index 18e19cd..c640052 100644 --- a/python/src/udf/native/accessors.rs +++ b/python/src/udf/native/accessors.rs @@ -1,5 +1,5 @@ use geodatafusion::udf::native::accessors::{ - CoordDim, EndPoint, GeometryType, M, NDims, NPoints, NumInteriorRings, ST_GeometryType, + CoordDim, EndPoint, IsClosed, GeometryType, M, NDims, NPoints, NumInteriorRings, ST_GeometryType, StartPoint, X, Y, Z, }; @@ -11,6 +11,7 @@ impl_udf!(X, PyX, "X"); impl_udf!(Y, PyY, "Y"); impl_udf!(Z, PyZ, "Z"); impl_udf!(M, PyM, "M"); +impl_udf!(IsClosed, PyIsClosed, "IsClosed"); impl_udf_coord_type_arg!(EndPoint, PyEndPoint, "EndPoint"); impl_udf_coord_type_arg!(StartPoint, PyStartPoint, "StartPoint"); impl_udf!(NPoints, PyNPoints, "NPoints"); diff --git a/python/src/udf/native/mod.rs b/python/src/udf/native/mod.rs index c03e3ba..25b2f29 100644 --- a/python/src/udf/native/mod.rs +++ b/python/src/udf/native/mod.rs @@ -10,6 +10,7 @@ pub(crate) fn native(m: &Bound) -> PyResult<()> { // accessors m.add_class::()?; m.add_class::()?; + m.add_class::()?; m.add_class::()?; m.add_class::()?; m.add_class::()?; diff --git a/python/tests/udf/geo/__init__.py b/python/tests/udf/geo/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/python/tests/udf/geo/test_validation.py b/python/tests/udf/native/test_accessors.py similarity index 100% rename from python/tests/udf/geo/test_validation.py rename to python/tests/udf/native/test_accessors.py diff --git a/rust/geodatafusion/src/udf/geo/validation/mod.rs b/rust/geodatafusion/src/udf/geo/validation/mod.rs index 90f146e..a0bd2f2 100644 --- a/rust/geodatafusion/src/udf/geo/validation/mod.rs +++ b/rust/geodatafusion/src/udf/geo/validation/mod.rs @@ -1,13 +1,10 @@ -mod is_closed; mod is_valid; mod is_valid_reason; -pub use is_closed::IsClosed; pub use is_valid::IsValid; pub use is_valid_reason::IsValidReason; pub fn register(session_context: &datafusion::prelude::SessionContext) { - session_context.register_udf(IsClosed.into()); session_context.register_udf(IsValid.into()); session_context.register_udf(IsValidReason.into()); } diff --git a/rust/geodatafusion/src/udf/geo/validation/is_closed.rs b/rust/geodatafusion/src/udf/native/accessors/is_closed.rs similarity index 100% rename from rust/geodatafusion/src/udf/geo/validation/is_closed.rs rename to rust/geodatafusion/src/udf/native/accessors/is_closed.rs diff --git a/rust/geodatafusion/src/udf/native/accessors/mod.rs b/rust/geodatafusion/src/udf/native/accessors/mod.rs index 5a716ca..582eeeb 100644 --- a/rust/geodatafusion/src/udf/native/accessors/mod.rs +++ b/rust/geodatafusion/src/udf/native/accessors/mod.rs @@ -1,5 +1,6 @@ mod coord_dim; mod geometry_type; +mod is_closed; mod line_string; mod npoints; mod num_interior_rings; @@ -7,6 +8,7 @@ mod point; pub use coord_dim::{CoordDim, NDims}; pub use geometry_type::{GeometryType, ST_GeometryType}; +pub use is_closed::IsClosed; pub use line_string::{EndPoint, StartPoint}; pub use npoints::NPoints; pub use num_interior_rings::NumInteriorRings; @@ -17,6 +19,7 @@ pub fn register(session_context: &datafusion::prelude::SessionContext) { session_context.register_udf(NDims.into()); session_context.register_udf(GeometryType.into()); session_context.register_udf(ST_GeometryType.into()); + session_context.register_udf(IsClosed.into()); session_context.register_udf(EndPoint::default().into()); session_context.register_udf(StartPoint::default().into()); session_context.register_udf(NPoints.into()); From 18a712de9ccfae61ed2980bf35ad96a9d8399592 Mon Sep 17 00:00:00 2001 From: oglego Date: Thu, 23 Apr 2026 16:03:21 -0500 Subject: [PATCH 3/3] refactor: fix fmt with cargo fmt --- python/src/udf/native/accessors.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/src/udf/native/accessors.rs b/python/src/udf/native/accessors.rs index c640052..680114a 100644 --- a/python/src/udf/native/accessors.rs +++ b/python/src/udf/native/accessors.rs @@ -1,6 +1,6 @@ use geodatafusion::udf::native::accessors::{ - CoordDim, EndPoint, IsClosed, GeometryType, M, NDims, NPoints, NumInteriorRings, ST_GeometryType, - StartPoint, X, Y, Z, + CoordDim, EndPoint, GeometryType, IsClosed, M, NDims, NPoints, NumInteriorRings, + ST_GeometryType, StartPoint, X, Y, Z, }; use crate::{impl_udf, impl_udf_coord_type_arg};