Copilot commented on code in PR #844:
URL: https://github.com/apache/sedona-db/pull/844#discussion_r3245151164
##########
rust/sedona-functions/src/st_points.rs:
##########
@@ -271,7 +278,7 @@ mod tests {
use datafusion_expr::ScalarUDF;
use rstest::rstest;
- use sedona_schema::datatypes::{WKB_GEOMETRY_ITEM_CRS, WKB_VIEW_GEOMETRY};
+ use sedona_schema::datatypes::{WKB_GEOGRAPHY, WKB_GEOGRAPHY_ITEM_CRS,
WKB_GEOMETRY_ITEM_CRS};
Review Comment:
The test module imports `WKB_GEOGRAPHY` but uses `WKB_GEOMETRY` in
`#[values(WKB_GEOMETRY, ...)]` without importing it. This will not compile; add
`WKB_GEOMETRY` to the datatypes import list (or qualify it).
##########
rust/sedona-functions/src/st_start_point.rs:
##########
@@ -171,7 +182,7 @@ fn extract_start_or_end_coord<'a>(
mod tests {
use datafusion_expr::ScalarUDF;
use rstest::rstest;
- use sedona_schema::datatypes::{WKB_GEOMETRY_ITEM_CRS, WKB_VIEW_GEOMETRY};
+ use sedona_schema::datatypes::{WKB_GEOGRAPHY_ITEM_CRS,
WKB_GEOMETRY_ITEM_CRS};
Review Comment:
The test module uses `WKB_GEOMETRY`/`WKB_GEOGRAPHY` in the rstest
`#[values(...)]` but they are not imported (only `*_ITEM_CRS` is). This will
fail to compile; import the constants (or qualify them) before using them in
the test parameters.
##########
rust/sedona-functions/src/st_reverse.rs:
##########
@@ -184,17 +191,17 @@ where
mod tests {
use datafusion_common::ScalarValue;
use rstest::rstest;
- use sedona_schema::datatypes::{WKB_GEOMETRY, WKB_GEOMETRY_ITEM_CRS,
WKB_VIEW_GEOMETRY};
+ use sedona_schema::datatypes::{WKB_GEOGRAPHY_ITEM_CRS,
WKB_GEOMETRY_ITEM_CRS};
Review Comment:
The test module uses `WKB_GEOMETRY`/`WKB_GEOGRAPHY` in `#[values(...)]` but
does not import them (only `*_ITEM_CRS`). This causes a compile error; import
the constants (or qualify them) in the test module.
##########
rust/sedona-functions/src/st_interiorringn.rs:
##########
@@ -126,18 +139,18 @@ mod tests {
let tester = ScalarUdfTester::new(
st_interiorringn_udf().into(),
vec![
- sedona_type,
+ sedona_type.clone(),
SedonaType::Arrow(arrow_schema::DataType::Int64),
],
);
- tester.assert_return_type(WKB_GEOMETRY);
+ tester.assert_return_type(sedona_type);
tester
}
// 1. Tests for Non-Polygon Geometries (Should return NULL)
#[rstest]
fn test_st_interiorringn_non_polygons(
- #[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type: SedonaType,
+ #[values(WKB_GEOMETRY, WKB_GEOGRAPHY)] sedona_type: SedonaType,
) {
let tester = setup_tester(sedona_type);
Review Comment:
This test is parameterized over geometry and geography, but the arrays
created inside the test (inputs and expected outputs) are still built using a
hard-coded `WKB_GEOMETRY` Sedona type. For the geography variant this will
mismatch the tester’s configured types; use `sedona_type` when constructing
those arrays.
##########
c/sedona-geos/src/st_line_merge.rs:
##########
@@ -120,23 +137,23 @@ mod tests {
use rstest::rstest;
use sedona_expr::scalar_udf::SedonaScalarUDF;
use sedona_schema::datatypes::{
- SedonaType, WKB_GEOMETRY, WKB_GEOMETRY_ITEM_CRS, WKB_VIEW_GEOMETRY,
+ SedonaType, WKB_GEOGRAPHY, WKB_GEOGRAPHY_ITEM_CRS, WKB_GEOMETRY,
WKB_GEOMETRY_ITEM_CRS,
};
use sedona_testing::create::create_array;
use sedona_testing::testers::ScalarUdfTester;
use super::*;
#[rstest]
- fn udf(#[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type: SedonaType)
{
+ fn udf(#[values(WKB_GEOMETRY, WKB_GEOGRAPHY)] sedona_type: SedonaType) {
use arrow_schema::DataType;
let udf = SedonaScalarUDF::from_impl("st_linemerge",
st_line_merge_impl());
let tester = ScalarUdfTester::new(
udf.into(),
- vec![sedona_type, SedonaType::Arrow(DataType::Boolean)],
+ vec![sedona_type.clone(), SedonaType::Arrow(DataType::Boolean)],
);
- tester.assert_return_type(WKB_GEOMETRY);
+ tester.assert_return_type(sedona_type.clone());
Review Comment:
This test is parameterized over geometry and geography inputs, but the
`expected`/`expected_directed` arrays are still created with `&WKB_GEOMETRY`.
Since `st_linemerge` now returns the same type as the input, the geography case
will fail; construct expected arrays using `sedona_type`.
##########
rust/sedona-functions/src/st_pointn.rs:
##########
@@ -129,7 +139,7 @@ fn write_wkb_point_from_coord(
mod tests {
use datafusion_expr::ScalarUDF;
use rstest::rstest;
- use sedona_schema::datatypes::{WKB_GEOMETRY_ITEM_CRS, WKB_VIEW_GEOMETRY};
+ use sedona_schema::datatypes::{WKB_GEOGRAPHY_ITEM_CRS,
WKB_GEOMETRY_ITEM_CRS};
Review Comment:
The test module uses `WKB_GEOMETRY`/`WKB_GEOGRAPHY` in `#[values(...)]` but
doesn’t import them (only `*_ITEM_CRS`). This will fail to compile; import
those constants (or qualify them) in the test module.
##########
rust/sedona-functions/src/st_geometryn.rs:
##########
@@ -123,22 +133,22 @@ fn invoke_scalar(geom: &Wkb, index: usize, writer: &mut
impl std::io::Write) ->
mod tests {
use datafusion_common::ScalarValue;
use rstest::rstest;
- use sedona_schema::datatypes::{WKB_GEOMETRY_ITEM_CRS, WKB_VIEW_GEOMETRY};
+ use sedona_schema::datatypes::{WKB_GEOGRAPHY_ITEM_CRS,
WKB_GEOMETRY_ITEM_CRS};
Review Comment:
The test module uses `WKB_GEOMETRY`/`WKB_GEOGRAPHY` in `#[values(...)]` but
doesn’t import them (only `*_ITEM_CRS`). This will not compile; import the
constants (or qualify them) in the test module.
##########
rust/sedona-functions/src/st_reverse.rs:
##########
@@ -184,17 +191,17 @@ where
mod tests {
use datafusion_common::ScalarValue;
use rstest::rstest;
- use sedona_schema::datatypes::{WKB_GEOMETRY, WKB_GEOMETRY_ITEM_CRS,
WKB_VIEW_GEOMETRY};
+ use sedona_schema::datatypes::{WKB_GEOGRAPHY_ITEM_CRS,
WKB_GEOMETRY_ITEM_CRS};
use sedona_testing::compare::assert_array_equal;
use sedona_testing::create::create_array;
use sedona_testing::testers::ScalarUdfTester;
use super::*;
#[rstest]
- fn udf(#[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type: SedonaType)
{
- let tester = ScalarUdfTester::new(st_reverse_udf().into(),
vec![sedona_type]);
- tester.assert_return_type(WKB_GEOMETRY);
+ fn udf(#[values(WKB_GEOMETRY, WKB_GEOGRAPHY)] sedona_type: SedonaType) {
+ let tester = ScalarUdfTester::new(st_reverse_udf().into(),
vec![sedona_type.clone()]);
+ tester.assert_return_type(sedona_type.clone());
Review Comment:
`st_reverse` now asserts the return type matches `sedona_type`, but the
expected output array later in this test is still created with a hard-coded
`WKB_GEOMETRY` Sedona type. This will fail for the geography case; construct
the expected array using `sedona_type`.
##########
rust/sedona-functions/src/st_interiorringn.rs:
##########
@@ -115,7 +128,7 @@ fn invoke_scalar(geom: &Wkb, index: usize, writer: &mut
impl std::io::Write) ->
mod tests {
use datafusion_common::ScalarValue;
use rstest::rstest;
- use sedona_schema::datatypes::{WKB_GEOMETRY_ITEM_CRS, WKB_VIEW_GEOMETRY};
+ use sedona_schema::datatypes::{WKB_GEOGRAPHY_ITEM_CRS,
WKB_GEOMETRY_ITEM_CRS};
Review Comment:
The test module uses `WKB_GEOMETRY`/`WKB_GEOGRAPHY` in `#[values(...)]` but
doesn’t import them (only `*_ITEM_CRS`). This will not compile; import the
constants (or qualify them) in the test module.
##########
rust/sedona-functions/src/st_flipcoordinates.rs:
##########
@@ -120,9 +120,7 @@ mod tests {
use super::*;
use datafusion_expr::ScalarUDF;
use rstest::rstest;
- use sedona_schema::crs::lnglat;
- use sedona_schema::datatypes::SedonaType::Wkb;
- use sedona_schema::datatypes::{Edges, WKB_GEOMETRY_ITEM_CRS,
WKB_VIEW_GEOMETRY};
+ use sedona_schema::datatypes::{WKB_GEOGRAPHY_ITEM_CRS,
WKB_GEOMETRY_ITEM_CRS};
Review Comment:
The test module uses `WKB_GEOMETRY`/`WKB_GEOGRAPHY` in `#[values(...)]` but
doesn’t import them (only `*_ITEM_CRS`). This will fail to compile; import
those constants (or qualify them) in the test module.
##########
rust/sedona-functions/src/st_geometryn.rs:
##########
@@ -123,22 +133,22 @@ fn invoke_scalar(geom: &Wkb, index: usize, writer: &mut
impl std::io::Write) ->
mod tests {
use datafusion_common::ScalarValue;
use rstest::rstest;
- use sedona_schema::datatypes::{WKB_GEOMETRY_ITEM_CRS, WKB_VIEW_GEOMETRY};
+ use sedona_schema::datatypes::{WKB_GEOGRAPHY_ITEM_CRS,
WKB_GEOMETRY_ITEM_CRS};
use sedona_testing::testers::ScalarUdfTester;
use sedona_testing::{compare::assert_array_equal, create::create_array};
use super::*;
#[rstest]
- fn udf(#[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type: SedonaType)
{
+ fn udf(#[values(WKB_GEOMETRY, WKB_GEOGRAPHY)] sedona_type: SedonaType) {
let tester = ScalarUdfTester::new(
st_geometryn_udf().into(),
vec![
sedona_type.clone(),
SedonaType::Arrow(arrow_schema::DataType::Int64),
],
);
- tester.assert_return_type(WKB_GEOMETRY);
+ tester.assert_return_type(sedona_type.clone());
let input_wkt = create_array(
&[
Review Comment:
This test is parameterized over geometry and geography, but the `input_wkt`
(and expected output) arrays in the body are still constructed with
`WKB_GEOMETRY`. For the geography case the tester is configured for geography
input/return types; build both input and expected arrays using `sedona_type` so
the geography path is actually exercised.
##########
rust/sedona-functions/src/st_pointn.rs:
##########
@@ -144,12 +154,14 @@ mod tests {
}
#[rstest]
- fn udf(#[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type: SedonaType)
{
+ fn udf(#[values(WKB_GEOMETRY, WKB_GEOGRAPHY)] sedona_type: SedonaType) {
let tester_pointn = ScalarUdfTester::new(
st_pointn_udf().into(),
vec![sedona_type.clone(), SedonaType::Arrow(DataType::Int64)],
);
+ tester_pointn.assert_return_type(sedona_type.clone());
+
Review Comment:
This test is parameterized over `WKB_GEOMETRY` and `WKB_GEOGRAPHY` and
asserts the return type matches `sedona_type`, but the expected arrays later in
the test still use a hard-coded `WKB_GEOMETRY` Sedona type. That will fail for
geography inputs; build expected arrays with `sedona_type`.
##########
c/sedona-geos/src/st_boundary.rs:
##########
@@ -287,16 +294,16 @@ fn collect_boundary_components(
mod tests {
use rstest::rstest;
use sedona_expr::scalar_udf::SedonaScalarUDF;
- use sedona_schema::datatypes::{WKB_GEOMETRY, WKB_GEOMETRY_ITEM_CRS,
WKB_VIEW_GEOMETRY};
+ use sedona_schema::datatypes::{WKB_GEOGRAPHY_ITEM_CRS,
WKB_GEOMETRY_ITEM_CRS};
Review Comment:
The test module uses `WKB_GEOMETRY`/`WKB_GEOGRAPHY` in `#[values(...)]` but
only imports `*_ITEM_CRS`. This will not compile; import `WKB_GEOMETRY` and
`WKB_GEOGRAPHY` (or qualify them) in the test module.
##########
rust/sedona-functions/src/st_force_dim.rs:
##########
@@ -451,9 +450,9 @@ mod tests {
}
#[rstest]
- fn udf_2d(#[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type:
SedonaType) {
+ fn udf_2d(#[values(WKB_GEOMETRY, WKB_GEOGRAPHY)] sedona_type: SedonaType) {
let tester = ScalarUdfTester::new(st_force2d_udf().into(),
vec![sedona_type.clone()]);
- tester.assert_return_type(WKB_GEOMETRY);
+ tester.assert_return_type(sedona_type.clone());
let points = create_array(
&[
Review Comment:
These tests are now parameterized over geography as well as geometry, but
the expected arrays in the test bodies are still constructed with
`&WKB_GEOMETRY`. Since the UDFs now return the same type as the input, the
geography variants will mismatch; build expected arrays using `sedona_type`
throughout.
##########
rust/sedona-functions/src/st_flipcoordinates.rs:
##########
@@ -133,31 +131,13 @@ mod tests {
assert_eq!(udf.name(), "st_flipcoordinates");
}
- #[test]
- fn udf_return_type() {
- let tester = ScalarUdfTester::new(st_flipcoordinates_udf().into(),
vec![WKB_GEOGRAPHY]);
- tester.assert_return_type(WKB_GEOGRAPHY);
-
- let tester = ScalarUdfTester::new(st_flipcoordinates_udf().into(),
vec![WKB_GEOMETRY]);
- tester.assert_return_type(WKB_GEOMETRY);
-
- let tester = ScalarUdfTester::new(st_flipcoordinates_udf().into(),
vec![WKB_VIEW_GEOMETRY]);
- tester.assert_return_type(WKB_GEOMETRY);
-
- let tester = ScalarUdfTester::new(
- st_flipcoordinates_udf().into(),
- vec![Wkb(Edges::Planar, lnglat())],
- );
- tester.assert_return_type(Wkb(Edges::Planar, lnglat()));
- }
-
#[rstest]
- fn udf_invoke(
- #[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY, WKB_GEOGRAPHY)] sedona_type:
SedonaType,
- ) {
+ fn udf(#[values(WKB_GEOMETRY, WKB_GEOGRAPHY)] sedona_type: SedonaType) {
let tester =
ScalarUdfTester::new(st_flipcoordinates_udf().into(),
vec![sedona_type.clone()]);
+ tester.assert_return_type(sedona_type.clone());
+
let result = tester.invoke_scalar("POINT (1 3)").unwrap();
tester.assert_scalar_result_equals(result, "POINT (3 1)");
Review Comment:
This test now asserts `st_flipcoordinates` returns the same type as the
input, but the expected array later in the test body is still constructed with
a hard-coded `WKB_GEOMETRY` Sedona type. That will fail for `WKB_GEOGRAPHY`;
build the expected array with `sedona_type`.
##########
c/sedona-geos/src/st_normalize.rs:
##########
@@ -89,17 +96,19 @@ mod tests {
use geos::{Geom, Geometry};
use rstest::rstest;
use sedona_expr::scalar_udf::SedonaScalarUDF;
- use sedona_schema::datatypes::{WKB_GEOMETRY, WKB_GEOMETRY_ITEM_CRS,
WKB_VIEW_GEOMETRY};
+ use sedona_schema::datatypes::{
+ WKB_GEOGRAPHY, WKB_GEOGRAPHY_ITEM_CRS, WKB_GEOMETRY,
WKB_GEOMETRY_ITEM_CRS,
+ };
use sedona_testing::testers::ScalarUdfTester;
use super::*;
#[rstest]
- fn udf(#[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type: SedonaType)
{
+ fn udf(#[values(WKB_GEOMETRY, WKB_GEOGRAPHY)] sedona_type: SedonaType) {
let udf = SedonaScalarUDF::from_impl("st_normalize",
st_normalize_impl());
let tester = ScalarUdfTester::new(udf.into(),
vec![sedona_type.clone()]);
- tester.assert_return_type(WKB_GEOMETRY);
+ tester.assert_return_type(sedona_type.clone());
let input_wkt = "POLYGON((1 1, 1 0, 0 0, 0 1, 1 1))";
let expected_wkt = "POLYGON ((0 0, 0 1, 1 1, 1 0, 0 0))";
Review Comment:
The test is now parameterized over geometry and geography and asserts the
return type matches `sedona_type`, but the batch `expected` array is still
constructed with `&WKB_GEOMETRY`. For the geography case this expected type
will not match; build the expected array using `sedona_type`.
##########
rust/sedona-functions/src/st_start_point.rs:
##########
@@ -190,12 +201,15 @@ mod tests {
}
#[rstest]
- fn udf(#[values(WKB_GEOMETRY, WKB_VIEW_GEOMETRY)] sedona_type: SedonaType)
{
+ fn udf(#[values(WKB_GEOMETRY, WKB_GEOGRAPHY)] sedona_type: SedonaType) {
let tester_start_point =
ScalarUdfTester::new(st_start_point_udf().into(),
vec![sedona_type.clone()]);
let tester_end_point =
ScalarUdfTester::new(st_end_point_udf().into(),
vec![sedona_type.clone()]);
+ tester_start_point.assert_return_type(sedona_type.clone());
+ tester_end_point.assert_return_type(sedona_type.clone());
+
Review Comment:
This test is now parameterized over `WKB_GEOMETRY` and `WKB_GEOGRAPHY` and
asserts the return type matches `sedona_type`, but the expected arrays in the
body are still constructed with a hard-coded `WKB_GEOMETRY` Sedona type. That
will make the geography variant fail (and won’t validate the output type);
build expected arrays using `sedona_type`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
