Copilot commented on code in PR #2876:
URL: https://github.com/apache/sedona/pull/2876#discussion_r3172344966
##########
docker/test-notebooks.sh:
##########
@@ -179,13 +194,15 @@ PYTHON_CLEANUP
continue
fi
- # Run the full Python script with timeout (600 seconds = 10 minutes)
- echo " Running Python script (600 second timeout)..."
+ # Run the full Python script with timeout (900 seconds = 15 minutes).
+ # Bumped from 600s to absorb network variance in notebooks that hit STAC
+ # or remote object stores. Local-data notebooks finish well under 60s.
+ echo " Running Python script (900 second timeout)..."
cd "$EXAMPLES_DIR/.."
# Use timeout with progress reporting
START_TIME=$(date +%s)
- if timeout 600 python3 "$PYTHON_FILE" | tee /tmp/notebook_output_$$.log;
then
+ if timeout 900 python3 "$PYTHON_FILE" | tee /tmp/notebook_output_$$.log;
then
END_TIME=$(date +%s)
Review Comment:
The test command pipes `timeout python3 ...` into `tee`, but the script does
not enable `set -o pipefail` (nor does it check `PIPESTATUS`). In Bash this
means the pipeline exit status is `tee`'s, so a failing/timeouting notebook run
can still be reported as passed. Enable `set -o pipefail` (ideally near `set
-e`) or explicitly check the python/timeout exit code to ensure failures are
detected.
##########
docs/usecases/00-quickstart.ipynb:
##########
@@ -0,0 +1,219 @@
+{
+ "cells": [
+ {
+ "cell_type": "markdown",
+ "metadata": {},
+ "source": [
+ "<!--\n",
+ " Licensed to the Apache Software Foundation (ASF) under one\n",
+ " or more contributor license agreements. See the NOTICE file\n",
+ " distributed with this work for additional information\n",
+ " regarding copyright ownership. The ASF licenses this file\n",
+ " to you under the Apache License, Version 2.0 (the\n",
+ " \"License\"); you may not use this file except in compliance\n",
+ " with the License. You may obtain a copy of the License at\n",
+ "\n",
+ " http://www.apache.org/licenses/LICENSE-2.0\n";,
+ "\n",
+ " Unless required by applicable law or agreed to in writing,\n",
+ " software distributed under the License is distributed on an\n",
+ " \"AS IS\" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY\n",
+ " KIND, either express or implied. See the License for the\n",
+ " specific language governing permissions and limitations\n",
+ " under the License.\n",
+ "-->\n",
+ "\n",
+ "# Sedona quickstart\n",
+ "\n",
+ "Spatial analytics in ten cells. We answer:\n",
+ "\n",
+ "> **Which countries have the most airports per million people?**\n",
+ "\n",
+ "Along the way we read two shapefiles, run a distributed spatial join,
aggregate, write **GeoParquet 1.1** with auto covering-bbox metadata, read it
back, and render an interactive map with **SedonaKepler**.\n",
+ "\n",
+ "Data ships with the docker image (Natural Earth 50m admin and airports).
No network required.\n",
+ "\n",
+ "Once you've finished here, head to:\n",
+ "\n",
+ "- `01-mobility-pulse.ipynb` — vector at scale: NYC taxi pickups by hour
and neighborhood.\n",
+ "- `02-vegetation-change.ipynb` — full raster pipeline: NDVI change
detection between two Sentinel-2 dates.\n",
+ "- `03-fire-risk-fusion.ipynb` — raster + vector fusion: wildfire risk
score per building.\n",
+ "- `04-flood-snapshot.ipynb` — STAC + Sentinel-1 to flag buildings inside
a recent flood extent.\n",
+ "- `05-geopandas-on-spark.ipynb` — scale your GeoPandas notebook to
billions of rows."
Review Comment:
The notebook links to `01-mobility-pulse.ipynb` through
`05-geopandas-on-spark.ipynb`, but those notebooks are not present under
`docs/usecases/` in this PR. Until they exist, these references will be dead
links in the docker image and in-repo browsing. Consider removing the list for
now, or replacing it with a generic "coming soon" note (or links to existing
content).
```suggestion
"Additional use-case notebooks are planned and will be added here in a
future update."
```
##########
docs/usecases/00-quickstart.ipynb:
##########
@@ -0,0 +1,219 @@
+{
+ "cells": [
+ {
+ "cell_type": "markdown",
+ "metadata": {},
+ "source": [
+ "<!--\n",
+ " Licensed to the Apache Software Foundation (ASF) under one\n",
+ " or more contributor license agreements. See the NOTICE file\n",
+ " distributed with this work for additional information\n",
+ " regarding copyright ownership. The ASF licenses this file\n",
+ " to you under the Apache License, Version 2.0 (the\n",
+ " \"License\"); you may not use this file except in compliance\n",
+ " with the License. You may obtain a copy of the License at\n",
+ "\n",
+ " http://www.apache.org/licenses/LICENSE-2.0\n";,
+ "\n",
+ " Unless required by applicable law or agreed to in writing,\n",
+ " software distributed under the License is distributed on an\n",
+ " \"AS IS\" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY\n",
+ " KIND, either express or implied. See the License for the\n",
+ " specific language governing permissions and limitations\n",
+ " under the License.\n",
+ "-->\n",
+ "\n",
+ "# Sedona quickstart\n",
+ "\n",
+ "Spatial analytics in ten cells. We answer:\n",
+ "\n",
+ "> **Which countries have the most airports per million people?**\n",
+ "\n",
+ "Along the way we read two shapefiles, run a distributed spatial join,
aggregate, write **GeoParquet 1.1** with auto covering-bbox metadata, read it
back, and render an interactive map with **SedonaKepler**.\n",
Review Comment:
This intro says "Spatial analytics in ten cells", but the notebook currently
contains 9 cells total. Update the wording (or adjust the notebook) so the
description matches the actual structure.
--
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]
