Copilot commented on code in PR #585: URL: https://github.com/apache/sedona-db/pull/585#discussion_r2800097235
########## docs/reference/sql/st_iscollection.qmd: ########## @@ -0,0 +1,43 @@ +--- +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +title: ST_IsCollection +description: Returns TRUE if the geometry type of the input is a geometry collection type. +kernels: + - returns: boolean + args: [geometry] +--- + +## Description + +Returns TRUE if the geometry type of the input is a geometry collection type. Collection types are the following: + +- GEOMETRYCOLLECTION +- MULTIPOINT +- MULTIPOLYGON +- MULTILINESTRING + +## Examples + +```sql +SELECT ST_IsCollection(ST_GeomFromText('MULTIPOINT(0 0), (6 6)')); Review Comment: This WKT is invalid for `MULTIPOINT` (the parentheses/comma placement is wrong). Update the example to a valid `MULTIPOINT`, e.g. `MULTIPOINT(0 0, 6 6)` or `MULTIPOINT((0 0), (6 6))`, so the example will actually render/run successfully. ```suggestion SELECT ST_IsCollection(ST_GeomFromText('MULTIPOINT(0 0, 6 6)')); ``` ########## mkdocs.yml: ########## @@ -43,7 +43,7 @@ nav: - Programming Guide: programming-guide.md - API: - Python: reference/python.md - - SQL: reference/sql.md + - SQL: reference/sql/index.md Review Comment: MkDocs nav now points to `reference/sql/index.md`, but this PR also deletes `docs/reference/sql.md` and doesn’t show an added/committed `docs/reference/sql/index.md`. If `index.md` is generated by Quarto, ensure it is generated as part of the docs build before `mkdocs build` runs (and that CI produces it in the expected location); otherwise add the missing `index.qmd`/`index.md` entrypoint. ```suggestion - SQL: reference/sql.md ``` ########## docs/reference/sql/st_transform.qmd: ########## @@ -0,0 +1,46 @@ +--- +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +title: ST_Transform +description: Transforms a geometry from one coordinate reference system to another. +kernels: + - returns: geometry + args: + - geometry + - name: target_crs + type: string + - returns: geometry + args: + - geometry + - name: source_crs + type: string + - name: target_crs + type: string +--- + +## Description + +Transforms the coordinate reference system of a geometry between EPSG reference systems. If provided with 2 arguments, target_crs can be specified as an EPSG code (e.g., 'EPSG:4326') or as a PROJ string. + +When a 3 arguments are provided, a source_crs can be used to specify the source CRS in case the input geometry doesn't have an SRID defined. The source_crs takes precedence over the geometry's SRID. Review Comment: Grammar: change 'When a 3 arguments are provided' to 'When 3 arguments are provided' (or 'When three arguments are provided') for readability. ```suggestion When 3 arguments are provided, a source_crs can be used to specify the source CRS in case the input geometry doesn't have an SRID defined. The source_crs takes precedence over the geometry's SRID. ``` ########## docs/reference/sql/st_translate.qmd: ########## @@ -0,0 +1,35 @@ +--- +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +title: ST_Translate +description: Returns a geometry with coordinates translated by deltaX and deltaY. +kernels: + - returns: geometry + args: + - geometry + - name: deltaX + type: double + - name: deltaY + type: double Review Comment: The previous monolithic `sql.md` (removed in this PR) documented `ST_Translate` as supporting an optional `deltaZ`. This new per-function page only documents `deltaX`/`deltaY`. Please confirm the actual function signature(s) and either (a) add the `deltaZ` overload to `kernels`, or (b) update/remove any prior references to `deltaZ` behavior to keep the docs consistent with the implementation. ########## ci/scripts/build-docs.sh: ########## @@ -47,6 +47,11 @@ else exit 1 fi +if grep -e "-- Example failed to render:" *.md; then Review Comment: This only scans `*.md` in the current directory and will miss nested outputs if Quarto writes markdown into subdirectories. Consider switching to a recursive search (and `-q` to avoid noisy logs), e.g. searching via `grep -R` over the rendered output directory. ```suggestion if grep -R -q -- "-- Example failed to render:" .; then ``` ########## docs/reference/sql/st_azimuth.qmd: ########## @@ -0,0 +1,30 @@ +--- +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +title: ST_Azimuth +description: Returns Azimuth for two given points in radians null or otherwise. Review Comment: The description is grammatically unclear. Consider rephrasing to something like 'Returns the azimuth between two points in radians, or NULL if not available.' ```suggestion description: Returns the azimuth between two points in radians, or NULL if not available. ``` ########## docs/reference/sql/st_crs.qmd: ########## @@ -0,0 +1,30 @@ +--- +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +title: ST_CRS +description: Returns the Coordinate Reference System (CRS) metadata associated with a geometry or geography object. +kernels: + - returns: string + args: [geometry] +--- + +## Examples + +```sql +SELECT ST_CRS(ST_Point(0.25, 0.25, 4326)) as crs_info; Review Comment: The example passes 3 arguments to `ST_Point`, but the `ST_Point` docs in this PR only define `(x, y)`. If CRS/SRID should be set, update the example to use `ST_SetSRID(ST_Point(...), 4326)` or `ST_SetCRS(...)` (whichever is correct for the SQL API), to avoid an example that may fail during rendering. ```suggestion SELECT ST_CRS(ST_SetSRID(ST_Point(0.25, 0.25), 4326)) as crs_info; ``` ########## .github/workflows/packaging.yml: ########## @@ -93,13 +97,47 @@ jobs: - uses: actions/checkout@v6 with: fetch-depth: 0 + submodules: 'true' - uses: actions/setup-python@v6 with: python-version: "3.x" - uses: quarto-dev/quarto-actions/setup@v2 + - name: Clone vcpkg + uses: actions/checkout@v6 + with: + repository: microsoft/vcpkg + ref: ${{ env.VCPKG_REF }} + path: vcpkg + + - name: Set up environment variables and bootstrap vcpkg + env: + VCPKG_ROOT: ${{ github.workspace }}/vcpkg + CMAKE_TOOLCHAIN_FILE: ${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake + run: | + cd vcpkg + ./bootstrap-vcpkg.sh + cd .. + + echo "VCPKG_ROOT=$VCPKG_ROOT" >> $GITHUB_ENV + echo "PATH=$VCPKG_ROOT:$PATH" >> $GITHUB_ENV + echo "CMAKE_TOOLCHAIN_FILE=$CMAKE_TOOLCHAIN_FILE" >> $GITHUB_ENV + + - name: Cache vcpkg binaries + id: cache-vcpkg + uses: actions/cache@v5 + with: + path: vcpkg/packages + # Bump the number at the end of this line to force a new dependency build + key: vcpkg-installed-${{ runner.os }}-${{ runner.arch }}-${{ env.VCPKG_REF }}-3 + + - name: Install vcpkg dependencies + if: steps.cache-vcpkg.outputs.cache-hit != 'true' + run: | + ./vcpkg/vcpkg install abseil openssl Review Comment: On a cache hit, the workflow skips `vcpkg install`, but the cache only stores `vcpkg/packages` (not `vcpkg/installed`). That can leave the build without installed headers/libs even though the cache hit. Fix by either caching `vcpkg/installed` (and/or using vcpkg binary caching explicitly), or always running `vcpkg install ...` and letting the cache speed it up. -- 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]
