You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "paleolimbot (via GitHub)" <gi...@apache.org> on 2026/02/13 19:28:31 UTC
[PR] chore(docs/reference/sql): Migrate function documentation defined in Rust into Markdown [sedona-db]
paleolimbot opened a new pull request, #616:
URL: https://github.com/apache/sedona-db/pull/616
This PR removes documentation defined in rust/sedona-functions and places it in docs/reference/sql (as the one source of truth for function documentation).
This was done by a combination of Opus and Codex, although I had to review all the descriptions to ensure we didn't have trivially repeated sections and the examples ran.
I will follow up with a PR to make the documentation accessible from Rust using a `sedona-function-definitions`, which will also do some cross checking and make the documentation available from SQL.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(docs/reference/sql): Migrate function documentation defined in Rust into Markdown [sedona-db]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #616:
URL: https://github.com/apache/sedona-db/pull/616#discussion_r2806319346
##########
rust/sedona-functions/src/st_geomfromwkb.rs:
##########
@@ -72,51 +70,13 @@ pub fn st_geogfromwkb_udf() -> SedonaScalarUDF {
out_type: WKB_VIEW_GEOGRAPHY,
})],
Volatility::Immutable,
- Some(doc("ST_GeogFromWKB", "Geography")),
- )
-}
-
-fn doc(name: &str, out_type_name: &str) -> Documentation {
- Documentation::builder(
- DOC_SECTION_OTHER,
- format!("Construct a {out_type_name} from WKB"),
- format!("{name} (Wkb: Binary)"),
- )
- .with_argument(
- "WKB",
- format!(
- "binary: Well-known binary representation of the {}",
- out_type_name.to_lowercase()
- ),
+ None,
)
- .with_sql_example(format!("SELECT {name}([01 02 00 00 00 02 00 00 00 00 00 00 00 84 D6 00 C0 00 00 00 00 80 B5 D6 BF 00 00 00 60 E1 EF F7 BF 00 00 00 80 07 5D E5 BF])"))
- .with_related_udf("ST_AsText")
- .build()
}
-/// Documentation for `ST_GeomFromWKBUnchecked()`.
+/// for `ST_GeomFromWKBUnchecked()`.
///
/// Parameterized for reuse if `ST_GeogFromWKBUnchecked()` is implemented in the future.
-fn doc_unchecked(name: &str, out_type_name: &str) -> Documentation {
- Documentation::builder(
- DOC_SECTION_OTHER,
- format!(
- "Construct a {out_type_name} from WKB without validation. Invalid WKB input may result in undefined behavior."
- ),
- format!("{name} (Wkb: Binary)"),
- )
- .with_argument(
- "WKB",
- format!(
- "binary: Well-known binary representation of the {}",
- out_type_name.to_lowercase()
- ),
- )
- .with_sql_example(format!("SELECT {name}([01 02 00 00 00 02 00 00 00 00 00 00 00 84 D6 00 C0 00 00 00 00 80 B5 D6 BF 00 00 00 60 E1 EF F7 BF 00 00 00 80 07 5D E5 BF])"))
- .with_related_udf("ST_AsText")
- .build()
-}
-
#[derive(Debug)]
Review Comment:
This rustdoc comment fragment (`/// for ST_GeomFromWKBUnchecked()`) no longer describes what follows and reads like a leftover from the removed Documentation helpers. Consider rewriting it to describe `STGeomFromWKB` (e.g., that it is the shared kernel for the WKB constructors) to avoid confusing generated docs.
##########
docs/reference/sql/st_geomfromwkb.qmd:
##########
@@ -17,19 +17,43 @@
# under the License.
title: ST_GeomFromWKB
-description: Construct a Geometry from WKB.
+description: Construct a Geometry from Well-Known Binary (WKB).
kernels:
- returns: geometry
args:
- name: wkb
type: binary
+ - returns: geometry
+ args:
+ - name: wkb
+ type: binary
+ - name: srid
+ type: integer
Review Comment:
The second kernel’s `srid` argument is declared as `type: integer`, but the page text/examples indicate it can accept an SRID *or* CRS string (e.g., 'OGC:CRS27'). Consider using a CRS-capable type label here (e.g., `crs` or `string`) and/or documenting both accepted forms to keep the rendered Usage/Arguments accurate.
##########
docs/reference/sql/st_knn.qmd:
##########
@@ -21,14 +21,24 @@ description: Return true if geomA finds k nearest neighbors from geomB.
kernels:
- returns: boolean
args:
- - geometry
+ - name: geomA
+ type: geometry
+ description: The geometry around which to search.
+ - name: geomb
+ type: geometry
+ description: Column containing candidate geometries.
- geometry
- name: k
type: integer
+ description: The number of nearest neighbours to return.
- name: use_spheroid
- type: boolean
+ type: true to use spherical distance, false for Euclidean
Review Comment:
The YAML frontmatter kernel signature looks inconsistent with the intended ST_KNN call signature: there is an extra shorthand `- geometry` arg (which will render as an unexpected third geometry parameter), `geomb` should likely be `geomB`, and `use_spheroid`'s `type` field currently contains prose instead of a type (e.g., `boolean`). Please update the `args` list so it matches `ST_KNN(geomA, geomB, k, use_spheroid)` and move the prose into `description` fields.
##########
docs/reference/sql/st_point.qmd:
##########
@@ -27,8 +27,18 @@ kernels:
type: double
---
+## Description
+
+Constructs a Point geometry from X and Y coordinates. An optional SRID or CRS can be provided as a third argument.
+
Review Comment:
The Description/Examples mention an optional third SRID/CRS argument, but the frontmatter `kernels` for this page only documents the 2-arg form (x, y). Please add a kernel variant for the 3-argument overload so generated Usage/Arguments match what’s described here.
##########
docs/reference/sql/st_geogpoint.qmd:
##########
@@ -27,6 +27,11 @@ kernels:
type: double
---
+## Description
+
+Creates a geography Point from longitude and latitude coordinates. An optional
+SRID or CRS can be provided as a third argument.
+
Review Comment:
This Description states an optional third SRID/CRS argument is supported, but the frontmatter `kernels` only defines the 2-arg signature. Please add a kernel variant including the SRID/CRS argument so the rendered Usage/Arguments reflect the documented behavior.
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(docs/reference/sql): Migrate function documentation defined in Rust into Markdown [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot merged PR #616:
URL: https://github.com/apache/sedona-db/pull/616
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(docs/reference/sql): Migrate function documentation defined in Rust into Markdown [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on PR #616:
URL: https://github.com/apache/sedona-db/pull/616#issuecomment-3916039811
> Sorry for making a conflict on st_force_dim.rs
No problem! Thanks!
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(docs/reference/sql): Migrate function documentation defined in Rust into Markdown [sedona-db]
Posted by "yutannihilation (via GitHub)" <gi...@apache.org>.
yutannihilation commented on PR #616:
URL: https://github.com/apache/sedona-db/pull/616#issuecomment-3903109307
Sorry for making a conflict on `st_force_dim.rs`. I created a pull request to resolve (but feel free to ignore if it's easier to do by yourself).
https://github.com/paleolimbot/sedona-db/pull/1
--
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: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org