paleolimbot commented on code in PR #38:
URL: https://github.com/apache/sedona-db/pull/38#discussion_r2333968002
##########
python/sedonadb/tests/functions/test_transforms.py:
##########
@@ -64,3 +80,20 @@ def test_st_setcrs_sedonadb(eng, geom, crs, expected_srid):
result = eng.execute_and_collect(f"SELECT ST_SetCrs({geom_or_null(geom)},
'{crs}')")
df = eng.result_to_pandas(result)
assert df.crs.to_epsg() == expected_srid
+
+
[email protected]("eng", [SedonaDB])
[email protected](
+ ("geom", "crs", "expected_crs"),
+ [
+ ("POINT (1 1)", "'EPSG:26920'", "EPSG:26920"),
+ ("POINT (1 1)", f"'{pyproj.CRS('EPSG:26920').to_json()}'",
"EPSG:26920"),
+ ("POINT (1 1)", None, None),
Review Comment:
optional nit: These parameters become part of the test name when we run
`pytest`, which in this case is a very long string for the second parameter! It
may make sense to have multiple `eng.assert_query_result()` instead.
##########
rust/sedona-functions/src/st_srid.rs:
##########
@@ -94,6 +118,52 @@ impl SedonaScalarKernel for StSrid {
}
}
+#[derive(Debug)]
+struct StCrs {}
+
+impl SedonaScalarKernel for StCrs {
+ fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> {
+ let matcher = ArgMatcher::new(
+ vec![ArgMatcher::is_geometry_or_geography()],
+ SedonaType::Arrow(DataType::Utf8),
Review Comment:
A `Utf8View` is probably what we want to return here (it more effectively
handles repeated values). We can change that in the future if it causes an
error in some internal somewhere (string views are a bit newer).
##########
rust/sedona-functions/src/st_srid.rs:
##########
@@ -94,6 +118,52 @@ impl SedonaScalarKernel for StSrid {
}
}
+#[derive(Debug)]
+struct StCrs {}
+
+impl SedonaScalarKernel for StCrs {
+ fn return_type(&self, args: &[SedonaType]) -> Result<Option<SedonaType>> {
+ let matcher = ArgMatcher::new(
+ vec![ArgMatcher::is_geometry_or_geography()],
+ SedonaType::Arrow(DataType::Utf8),
+ );
+
+ matcher.match_args(args)
+ }
+
+ fn invoke_batch(
+ &self,
+ arg_types: &[SedonaType],
+ args: &[ColumnarValue],
+ ) -> Result<ColumnarValue> {
+ let executor = WkbExecutor::new(arg_types, args);
+ let preallocate_bytes = "EPSG:4326".len() * executor.num_iterations();
+ let mut builder =
+ StringBuilder::with_capacity(executor.num_iterations(),
preallocate_bytes);
+ let crs_opt: Option<String> = match &arg_types[0] {
+ SedonaType::Wkb(_, Some(crs)) | SedonaType::WkbView(_, Some(crs))
=> {
+ match crs.to_authority_code()? {
+ Some(auth_code) => Some(auth_code),
+ None => Some(crs.to_json()),
+ }
Review Comment:
While the authority:code output is definitely a lot nicer to look at, I
think we want to return `crs.to_json()` here because it does not loose
information. I think that `ST_SRID()` will support most uses where the user
wants a code. In some future we could also have a "crs" type that prints out
nicely but keeps the JSON under the hood.
--
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]
