-
-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FOSS4G 2024 Belem workshop #244
Conversation
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: pgRouting-workshop/develop Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/index/
Currently translated at 100.0% (141 of 141 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (145 of 145 strings) Translated using Weblate (Spanish) Currently translated at 77.5% (31 of 40 strings) Translated using Weblate (Spanish) Currently translated at 92.3% (48 of 52 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (141 of 141 strings) Translated using Weblate (Spanish) Currently translated at 91.2% (136 of 149 strings) Translated using Weblate (Spanish) Currently translated at 84.6% (44 of 52 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (158 of 158 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (145 of 145 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (104 of 104 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (137 of 137 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (158 of 158 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (111 of 111 strings) Co-authored-by: Celia Virginia Vergara Castillo <vicky@erosion.dev> Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/basic-graph_views/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/basic-pedestrian/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/basic-plpgsql_function/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/basic-sql_function/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/basic-vehicle/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/interactions-chapter-10/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/interactions-chapter-9/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/un_sdg-sdg3-health/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/un_sdg-sdg7-energy/es/ Translation: pgRouting workshop/basic-graph_views Translation: pgRouting workshop/basic-pedestrian Translation: pgRouting workshop/basic-plpgsql_function Translation: pgRouting workshop/basic-sql_function Translation: pgRouting workshop/basic-vehicle Translation: pgRouting workshop/interactions-chapter-10 Translation: pgRouting workshop/interactions-chapter-9 Translation: pgRouting workshop/un_sdg-sdg3-health Translation: pgRouting workshop/un_sdg-sdg7-energy
Currently translated at 100.0% (9 of 9 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (9 of 9 strings) Co-authored-by: Anonymous <noreply@weblate.org> Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/index/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/index/ja/ Translation: pgRouting workshop/index
…index Translations update from OSGeo Weblate
Fix links and locale update
Fix typo
Fixing typo
Currently translated at 100.0% (137 of 137 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (86 of 86 strings) Translated using Weblate (Spanish) Currently translated at 52.1% (37 of 71 strings) Translated using Weblate (Spanish) Currently translated at 83.0% (44 of 53 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (158 of 158 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (86 of 86 strings) Translated using Weblate (Spanish) Currently translated at 99.3% (145 of 146 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (104 of 104 strings) Translated using Weblate (Spanish) Currently translated at 87.5% (35 of 40 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (15 of 15 strings) Translated using Weblate (Spanish) Currently translated at 57.1% (20 of 35 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (18 of 18 strings) Translated using Weblate (Spanish) Currently translated at 92.3% (48 of 52 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (55 of 55 strings) Translated using Weblate (Spanish) Currently translated at 100.0% (38 of 38 strings) Translated using Weblate (Spanish) Currently translated at 84.6% (44 of 52 strings) Co-authored-by: Celia Virginia Vergara Castillo <vicky@erosion.dev> Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/advanced-chapter-12/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/appendix-appendix-2/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/appendix-appendix-3/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/basic-data/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/basic-graph_views/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/basic-pedestrian/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/basic-plpgsql_function/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/basic-sql_function/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/basic-vehicle/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/general-intro-introduction/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/interactions-chapter-10/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/interactions-chapter-9/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/un_sdg-introduction/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/un_sdg-sdg11-cities/es/ Translate-URL: https://weblate.osgeo.org/projects/pgrouting-workshop/un_sdg-sdg7-energy/es/ Translation: pgRouting workshop/advanced-chapter-12 Translation: pgRouting workshop/appendix-appendix-2 Translation: pgRouting workshop/appendix-appendix-3 Translation: pgRouting workshop/basic-data Translation: pgRouting workshop/basic-graph_views Translation: pgRouting workshop/basic-pedestrian Translation: pgRouting workshop/basic-plpgsql_function Translation: pgRouting workshop/basic-sql_function Translation: pgRouting workshop/basic-vehicle Translation: pgRouting workshop/general-intro-introduction Translation: pgRouting workshop/interactions-chapter-10 Translation: pgRouting workshop/interactions-chapter-9 Translation: pgRouting workshop/un_sdg-introduction Translation: pgRouting workshop/un_sdg-sdg11-cities Translation: pgRouting workshop/un_sdg-sdg7-energy
…index Translations update from OSGeo Weblate
WalkthroughThis pull request introduces a comprehensive update to the pgRouting Workshop documentation, transitioning from the Prizren 2.9 version to the Belém 3.0 version. The changes encompass multiple aspects of the project, including workflow configurations, documentation, localization files, and project metadata. Key modifications include updating the workshop location from Prizren to Belém, upgrading dependency management, refactoring documentation structure, and enhancing locale support. Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 22
🔭 Outside diff range comments (3)
.github/scripts/remove_obsolete_entries.sh (1)
Line range hint
10-15
: Improve script robustness for handling special characters in filenames.The current implementation using
find
with afor
loop is fragile and may fail with filenames containing spaces or special characters.Apply this diff to make the script more robust:
-for file in $(find locale -type f -name "*.po"); do - if grep -q '#~' $file; then - perl -pi -0777 -e 's/#~.*//s' $file - git add $file - fi -done +find locale -type f -name "*.po" -print0 | while IFS= read -r -d '' file; do + if grep -q '#~' "$file"; then + perl -pi -0777 -e 's/#~.*//s' "$file" + git add "$file" + fi +doneThis change:
- Uses
-print0
withread -d ''
to handle special characters- Properly quotes variables to prevent word splitting
- Addresses the shellcheck warning SC2044
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 10-10: For loops over find output are fragile. Use find -exec or a while read loop.
(SC2044)
docs/interactions/chapter-11.rst (1)
Line range hint
165-262
: Add error handling for WMS requests.The OpenLayers code should include error handling for WMS requests.
Add error handling to the WMS layer creation:
result = new ol.layer.Image({ source: new ol.source.ImageWMS({ url: 'http://localhost:8082/geoserver/pgrouting/wms', - params: params + params: params, + imageLoadFunction: function(image, src) { + image.getImage().src = src; + image.getImage().onerror = function() { + console.error('Failed to load WMS layer'); + // Optionally show user-friendly error message + alert('Failed to load routing result. Please try again.'); + }; + } }) });docs/scripts/basic/chapter_7/all_sections.sql (1)
Line range hint
302-347
: Fix potential geometry direction bug in wrk_dijkstra function.The function uses source_osm for determining geometry direction, but it should use source for consistency with the rest of the codebase.
CASE - WHEN node = source_osm THEN the_geom + WHEN node = source THEN the_geom ELSE ST_Reverse(the_geom) END AS route_geom
🧹 Nitpick comments (62)
locale/en/LC_MESSAGES/examples/boost_dijkstra.po (1)
Line range hint
27-27
: Fix typo in message string.There's a typo in the message string: "tranlasted" should be "translated".
-msgid "Here is the tranlasted SQL commands." +msgid "Here is the translated SQL commands."docs/scripts/get_data/process_osgeolive_data.sh (1)
Line range hint
1-45
: Consider adding error handling for database operations.The script uses
set -e
which is good for failing fast, but could benefit from more specific error handling for database operations.Consider wrapping database operations in functions with error handling:
+handle_db_error() { + echo "Error: Database operation failed: $1" >&2 + exit 1 +} +create_extensions() { + psql city_routing << EOF || handle_db_error "Failed to create extensions" + CREATE EXTENSION postgis; + CREATE EXTENSION pgrouting; +EOF +}docs/scripts/un_sdg/sdg11/all_exercises_sdg11.sql (6)
3-8
: Consider adding constraints and indexes for the geometry columns.The table structure looks good, but could benefit from:
- A PRIMARY KEY constraint on the id column
- NOT NULL constraints where appropriate
- Spatial indexes on geom and city_buffer columns
CREATE TABLE bangladesh ( - id BIGINT, + id BIGINT PRIMARY KEY, - name TEXT, + name TEXT NOT NULL, geom geometry, city_buffer geometry ); +CREATE INDEX ON bangladesh USING GIST (geom); +CREATE INDEX ON bangladesh USING GIST (city_buffer);
36-42
: Consider combining DELETE operations and documenting OSM IDs.The separate DELETE operations can be combined for better maintainability. Also, consider adding comments explaining why these specific OSM IDs are being removed.
+-- Remove disconnected or irrelevant waterway segments DELETE FROM waterways_ways WHERE osm_id IN ( 721133202, 908102930, 749173392, 652172284, - 126774195, 720395312); + 126774195, 720395312, 815893446 +); - -DELETE FROM waterways_ways WHERE osm_id = 815893446;
46-59
: Add error handling and optimize vertex updates.The vertex extraction and updates look correct but could benefit from:
- Error handling for cases where the_geom is NULL
- Combining coordinate updates into a single operation
SELECT * INTO waterways.waterways_vertices FROM pgr_extractVertices( 'SELECT gid AS id, source, target FROM waterways.waterways_ways ORDER BY id'); +-- Ensure the_geom is not NULL before updates UPDATE waterways_vertices SET geom = ST_startPoint(the_geom) -FROM waterways_ways WHERE source = id; +FROM waterways_ways WHERE source = id AND the_geom IS NOT NULL; UPDATE waterways_vertices SET geom = ST_endPoint(the_geom) -FROM waterways_ways WHERE geom IS NULL AND target = id; +FROM waterways_ways WHERE geom IS NULL AND target = id AND the_geom IS NOT NULL; -UPDATE waterways_vertices set (x,y) = (ST_X(geom), ST_Y(geom)); +UPDATE waterways_vertices +SET (x,y) = (ST_X(geom), ST_Y(geom)) +WHERE geom IS NOT NULL;
82-89
: Add input validation to get_city_buffer function.The function should handle cases where the city_id doesn't exist.
CREATE OR REPLACE FUNCTION get_city_buffer(city_id INTEGER) RETURNS geometry AS $BODY$ + SELECT CASE + WHEN EXISTS (SELECT 1 FROM bangladesh WHERE id = city_id) + THEN (SELECT city_buffer FROM bangladesh WHERE id = city_id) + ELSE NULL + END; - SELECT city_buffer FROM bangladesh WHERE id = city_id; $BODY$ LANGUAGE SQL;
91-95
: Consider adding an index hint for spatial join performance.The spatial join could benefit from explicitly using the spatial index.
SELECT DISTINCT component FROM bangladesh JOIN waterways.waterways_ways -ON (ST_Intersects(the_geom, get_city_buffer(5))); +ON (ST_Intersects(the_geom, get_city_buffer(5))) +WHERE id = 5; -- Add WHERE clause to limit bangladesh rows
96-105
: Consider creating a function for buffer calculations.The hardcoded buffer size (0.005) is used in multiple places. Consider creating a function to maintain consistency.
+CREATE OR REPLACE FUNCTION get_buffer_size() RETURNS float AS +$BODY$ + SELECT 0.005::float; -- Approximately 500m at the equator +$BODY$ +LANGUAGE SQL IMMUTABLE; ALTER TABLE waterways_ways ADD COLUMN rain_zone geometry; UPDATE waterways.waterways_ways -SET rain_zone = ST_Buffer((the_geom),0.005) +SET rain_zone = ST_Buffer((the_geom), get_buffer_size()) WHERE ST_Intersects(the_geom, get_city_buffer(5));docs/scripts/un_sdg/sdg7/all_exercises_sdg7.sql (2)
12-25
: Consider creating a shared function for vertex extraction.This vertex extraction logic is duplicated in both SDG scripts. Consider creating a shared function to reduce duplication.
+-- Create in a shared script: +CREATE OR REPLACE FUNCTION setup_vertices_table( + schema text, + ways_table text, + vertices_table text +) RETURNS void AS +$BODY$ +BEGIN + EXECUTE format(' + SELECT * INTO %I.%I + FROM pgr_extractVertices( + ''SELECT gid AS id, source, target + FROM %I.%I ORDER BY id''); + + UPDATE %I SET geom = ST_startPoint(the_geom) + FROM %I WHERE source = id; + + UPDATE %I SET geom = ST_endPoint(the_geom) + FROM %I WHERE geom IS NULL AND target = id; + + UPDATE %I set (x,y) = (ST_X(geom), ST_Y(geom)); + ', + schema, vertices_table, + schema, ways_table, + vertices_table, ways_table, + vertices_table, ways_table, + vertices_table + ); +END; +$BODY$ +LANGUAGE plpgsql; +-- Then use it here: +SELECT setup_vertices_table('roads', 'roads_ways', 'roads_vertices');
29-68
: Add documentation for the component analysis process.The component analysis uses complex CTEs to identify and remove disconnected components. Consider adding comments to explain the process for workshop participants.
+-- Add component columns to track connected subgraphs ALTER TABLE roads_ways ADD COLUMN component BIGINT; ALTER TABLE roads_vertices ADD COLUMN component BIGINT; +-- Identify connected components using pgr_connectedComponents UPDATE roads_vertices SET component = c.component FROM ( SELECT * FROM pgr_connectedComponents( 'SELECT gid as id, source, target, cost, reverse_cost FROM roads_ways') ) AS c WHERE id = node; +-- Propagate component information to ways table UPDATE roads_ways SET component = v.component FROM (SELECT id, component FROM roads_vertices) AS v WHERE source = v.id; +-- Find and keep only the largest connected component WITH all_components AS (SELECT component, count(*) FROM roads_vertices GROUP BY component), max_component AS (SELECT max(count) from all_components), the_component AS (SELECT component FROM all_components WHERE count = (SELECT max FROM max_component)) DELETE FROM roads_vertices WHERE component != (SELECT component FROM the_component);locale/en/LC_MESSAGES/basic/sql_function.po (3)
4-5
: Ensure placeholders are updated or removed if not needed.
The “FIRST AUTHOR” placeholder and email address are typically meant to be replaced with actual values.
219-219
: Fix typographical error in “leng.”
Please correct “leng” to “length.”- "The leng in meters of the segment." + "The length in meters of the segment."
558-558
: Resolve spelling/grammar for “heavely.”
Replace “heavely” with “heavily.”- "pgRouting uses heavely function overloading:" + "pgRouting uses heavily function overloading:"locale/en/LC_MESSAGES/appendix/appendix-2.po (1)
90-90
: Improve message formatting.Consider removing the unnecessary line break before "restart PostgreSQL server" for better readability.
-" restart PostgreSQL server with ``sudo service postgresql restart``." +" restart PostgreSQL server with ``sudo service postgresql restart``."locale/en/LC_MESSAGES/appendix/appendix-3.po (1)
45-45
: Enhance clarity of technical limitations.The description of osm2pgrouting's memory limitations could be clearer. Consider restructuring the sentence to better explain the memory constraints.
-"The way to handle large data sets is to current version of osm2pgrouting needs to load all data into memory" +"The current version of osm2pgrouting needs to load all data into memory to handle large datasets"locale/es/LC_MESSAGES/appendix/appendix-2.po (2)
45-50
: Simplify message string formatting.The Geoserver message translation is unnecessarily split across multiple lines. Consider keeping it as a single line for better maintainability.
-"`Geoserver " -"<https://live.osgeo.org/en/quickstart/geoserver_quickstart.html>`__ for " -"the routing application" +"`Geoserver <https://live.osgeo.org/en/quickstart/geoserver_quickstart.html>`__ for the routing application"
104-110
: Consolidate split message strings.The PostgreSQL configuration instructions are split across multiple lines unnecessarily. Consider consolidating them for better readability and maintenance.
-"Para evitar errores de permiso denegado para los usuarios locales, se puede " -"ajustar el método de conexión a ``trust`` en ``/etc/postgresql/<version>/" -"main/pg_hba.conf`` y reiniciar el servidor PostgreSQL con ``sudo service " -"postgresql restart``." +"Para evitar errores de permiso denegado para los usuarios locales, se puede ajustar el método de conexión a ``trust`` en ``/etc/postgresql/<version>/main/pg_hba.conf`` y reiniciar el servidor PostgreSQL con ``sudo service postgresql restart``."locale/en/LC_MESSAGES/general-intro/introduction.po (1)
114-114
: Fix typo in "Acknowledgments".There's a typo in the section heading.
-msgid "Aknowledments" +msgid "Acknowledgments"locale/en/LC_MESSAGES/advanced/chapter-12.po (1)
241-243
: Fix the extra spaces in the msgid string.The string contains extra spaces that should be removed for consistency.
Apply this diff to fix the formatting:
-"Now we are ready for our first routing query with `pgr_dijkstra " -"<https://docs.pgrouting.org/latest/en/pgr_dijkstra.html>`__ or any other " -"pgRouting query." +"Now we are ready for our first routing query with `pgr_dijkstra" +"<https://docs.pgrouting.org/latest/en/pgr_dijkstra.html>`__ or any other" +"pgRouting query."docs/conf.py (1)
184-186
: Consider documenting the empty sidebar decision.While using an empty sidebar is valid, consider adding a comment explaining this design choice.
html_sidebars = { - '**': [] + # Using empty sidebar for cleaner documentation layout + '**': [] }locale/en/LC_MESSAGES/un_sdg/introduction.po (2)
44-44
: Fix grammatical error in the text.The sentence structure needs improvement.
-"the pgRouting workshop by addressing Three of the Seventeen Sustainable " +"the pgRouting workshop by addressing three of the seventeen Sustainable "
Line range hint
220-220
: Fix typo in the word "which".-" wich is in a lower altitude of the city, are to be removed from the " +" which is in a lower altitude of the city, are to be removed from the "locale/en/LC_MESSAGES/un_sdg/sdg11-cities.po (1)
234-235
: Improve message formatting consistency.The line wrapping in the message is inconsistent with the rest of the file.
-"Using ``cost_s`` (line **6**) and ``reverse_cost_s`` (line **7**) " -"columns, which are in unit ``seconds``." +"Using ``cost_s`` (line **6**) and ``reverse_cost_s`` (line **7**) columns, " +"which are in unit ``seconds``."locale/es/LC_MESSAGES/advanced/chapter-12.po (1)
158-160
: Maintain consistent translation of technical terms.The translation of "Routing Network Topology" varies within the text. It should be consistent throughout the document.
-"Asegurarse de que los datos proporcionan una correcta `Red Topológica de " -"ruteo` que consta de información sobre los identificadores de origen y " +"Asegurarse de que los datos proporcionan una correcta `Topología de Red de " +"Ruteo` que consta de información sobre los identificadores de origen y "locale/en/LC_MESSAGES/basic/vehicle.po (2)
396-398
: Remove redundant empty line.The empty line after the message is unnecessary and inconsistent with the file's formatting.
msgid "``penalty < 0`` makes the road not to be included in the graph." msgstr "" -
464-465
: Fix unnecessary line break.The short message doesn't need to be split across multiple lines.
-msgid "The costs do not change proportionally." -msgstr "" +msgid "The costs do not change proportionally." +msgstr ""docs/images/CMakeLists.txt (1)
4-8
: Well-structured image directory organization!Good separation of image categories into logical subdirectories.
Consider adding a comment describing the purpose of each subdirectory for better maintainability.
Readme.md (1)
46-46
: Document supporter changes.The addition of erosion as a supporter is good, but consider adding their role or contribution type for context.
-* [erosion](https://www.erosion.dev) +* [erosion](https://www.erosion.dev) - Development & Infrastructure Supportdocs/scripts/basic/chapter_7/images.sql (1)
59-69
: Document template variables usage.The views use template variables (@CH7_OSMID_1@, @CH7_OSMID_2@) without explanation.
Add comments explaining:
- What these variables represent
- How they are populated during build
- Expected value ranges
.github/scripts/update_locale.sh (1)
34-37
: Consider simplifying the complex awk/perl pipeline.The current implementation using multiple perl commands could be simplified for better maintainability.
-awk '/^Update|^Create/{print $2}' build/docs/locale_changes.txt > build/docs/locale_changes_po.txt # .po files -cp build/docs/locale_changes_po.txt build/docs/locale_changes_po_pot.txt -perl -ne '/\/en\// && print' build/docs/locale_changes_po.txt | \ - perl -pe 's/(.*)en\/LC_MESSAGES(.*)/$1pot$2t/' >> build/docs/locale_changes_po_pot.txt # .pot files +awk '/^Update|^Create/{print $2}' build/docs/locale_changes.txt | tee build/docs/locale_changes_po.txt | \ + awk '/\/en\//{sub(/en\/LC_MESSAGES/, "pot"); print $0"t"}' >> build/docs/locale_changes_po_pot.txtdocs/scripts/get_data/CMakeLists.txt (1)
28-37
: Consider adding timeout for download operations.The execute_process command should include a timeout to prevent hanging on slow downloads.
execute_process( OUTPUT_FILE ${CMAKE_SOURCE_DIR}/downloads/${PGR_WORKSHOP_DOWNLOAD_DIR}/${PGR_WORKSHOP_CITY_FILE}.osm COMMAND bash ${CMAKE_CURRENT_BINARY_DIR}/get_all_data.sh WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/downloads/${PGR_WORKSHOP_DOWNLOAD_DIR} INPUT_FILE ${CMAKE_CURRENT_BINARY_DIR}/get_all_data.sh + TIMEOUT 300 )
docs/scripts/basic/chapter_5/images.sql (1)
47-53
: Consider adding an index for performance optimization.The query might benefit from an index on the length_m column for better performance.
CREATE INDEX IF NOT EXISTS idx_ways_length_m ON ways(length_m);docs/CMakeLists.txt (1)
89-92
: Consider maintaining warning visibility.Setting WARNINGS_TO_ERRORS to empty for all languages might hide important issues. Consider keeping warnings enabled but not treating them as errors.
- set(WARNINGS_TO_ERRORS "") + set(WARNINGS_TO_ERRORS "-W").github/workflows/locale.yml (2)
8-14
: Consider optimizing workflow triggers.The workflow could benefit from more specific path filters to trigger only when relevant files change:
on: workflow_dispatch: push: + paths: + - 'docs/**' + - '.github/workflows/locale.yml' + - '.github/scripts/update_locale.sh' + - '.github/scripts/remove_obsolete_entries.sh'
91-98
: Add concurrency control to prevent race conditions.Consider adding concurrency settings to prevent conflicts when multiple workflow runs attempt to update the repository simultaneously:
name: Update Locale + +concurrency: + group: locale-${{ github.ref }} + cancel-in-progress: truedocs/scripts/basic/chapter_8/all-sections-8.sql (1)
Line range hint
71-89
: Function signature and implementation improvements.The wrk_Nearest function has better error handling and uses a more consistent naming convention.
Consider adding input validation for lat/lon parameters to ensure they are within valid ranges:
CREATE OR REPLACE FUNCTION wrk_Nearest( IN vertex_table REGCLASS, IN lat NUMERIC, IN lon NUMERIC) RETURNS BIGINT AS $BODY$ DECLARE result BIGINT; BEGIN + -- Validate latitude range + IF lat < -90 OR lat > 90 THEN + RAISE EXCEPTION 'Invalid latitude: %. Must be between -90 and 90.', lat; + END IF; + -- Validate longitude range + IF lon < -180 OR lon > 180 THEN + RAISE EXCEPTION 'Invalid longitude: %. Must be between -180 and 180.', lon; + END IF;docs/scripts/basic/chapter_5/all_exercises_5.sql (1)
65-78
: Enhanced query structure with VALUES clause.Good use of VALUES clause for defining source-target combinations.
Consider adding a comment explaining the VALUES clause usage:
SELECT * FROM pgr_dijkstra( ' SELECT gid AS id, source, target, length_m / 1.3 / 60 AS cost FROM ways ', + -- Using VALUES clause to define multiple source-target pairs + -- Each row represents a (source, target) combination to route between 'SELECT * FROM (VALUES (@ID_1@, @ID_4@), (@ID_2@, @ID_5@)) AS combinations (source, target)', directed := false);docs/general-intro/osgeolive.rst (2)
55-56
: Add memory requirement warning.The memory size recommendation of 4096MB should include a warning about minimum requirements.
Add a note about memory requirements:
+ .. warning:: + The recommended memory size of 4096MB is optimal for workshop exercises. + A minimum of 2048MB is required for basic functionality, but may result + in degraded performance.
157-164
: Remove duplicate note about credentials.The user credentials note appears twice (lines 157 and 164).
Remove the duplicate note:
.. note:: OSGeoLive's account is ``user`` and password is ``user`` - .. Note:: OSGeoLive's account is ``user`` and password is ``user``
docs/scripts/un_sdg/sdg3/all_exercises_sdg3.sql (1)
Line range hint
56-73
: Optimize population calculation function.The population calculation function can be improved for better maintainability and performance.
Consider using a lookup table approach:
-CREATE OR REPLACE FUNCTION population(tag_id INTEGER, area INTEGER) +CREATE OR REPLACE FUNCTION population(tag_id INTEGER, area INTEGER) RETURNS INTEGER AS $BODY$ DECLARE -population INTEGER; + population INTEGER; + density_factors CONSTANT jsonb := '{ + "negligible": {"max_id": 100, "min_pop": 1, "factor": 0}, + "very_sparse": {"max_id": 200, "min_pop": 2, "factor": 0.0002}, + "sparse": {"max_id": 300, "min_pop": 3, "factor": 0.002}, + "moderate": {"max_id": 400, "min_pop": 5, "factor": 0.05}, + "dense": {"max_id": 500, "min_pop": 7, "factor": 0.7}, + "very_dense": {"max_id": null, "min_pop": 10, "factor": 1} + }'; BEGIN - IF tag_id <= 100 THEN population = 1; -- Negligible - ELSIF 100 < tag_id AND tag_id <= 200 THEN population = GREATEST(2, area * 0.0002); -- Very Sparse - ELSIF 200 < tag_id AND tag_id <= 300 THEN population = GREATEST(3, area * 0.002); -- Sparse - ELSIF 300 < tag_id AND tag_id <= 400 THEN population = GREATEST(5, area * 0.05); -- Moderate - ELSIF 400 < tag_id AND tag_id <= 500 THEN population = GREATEST(7, area * 0.7); -- Dense - ELSIF tag_id > 500 THEN population = GREATEST(10,area * 1); -- Very Dense - ELSE population = 1; - END IF; + SELECT + GREATEST( + (density->>'min_pop')::integer, + area * (density->>'factor')::float + ) + INTO population + FROM jsonb_each(density_factors) AS d(level, density) + WHERE tag_id <= COALESCE((density->>'max_id')::integer, 2147483647) + ORDER BY (density->>'max_id')::integer + LIMIT 1; + + RETURN COALESCE(population, 1); END; $BODY$ LANGUAGE plpgsql;Benefits:
- More maintainable with density factors in a single place
- Easier to modify or add new density levels
- Better performance with indexed lookup
docs/un_sdg/data.rst (1)
178-179
: Improve clarity of Bangladesh data instructions.The transition to Bangladesh data section could be clearer.
Enhance the section introduction:
-Now download the data for an area in Bangladesh by following the same steps like -that of Mumbai. +# Bangladesh Data Preparation + +This section guides you through downloading and preparing data for an area in Bangladesh. +The process is similar to the Mumbai data preparation, but with Bangladesh-specific +configurations and data sources.docs/scripts/basic/chapter_7/all_sections.sql (3)
7-31
: Add error handling for geometry updates.The sequence of geometry updates should include error handling to catch and log any failures. This is particularly important for the geom, x, and y column updates.
UPDATE ways_vertices SET geom = ST_startPoint(the_geom) +FROM ways WHERE source_osm = id; + +DO $$ +BEGIN + IF EXISTS (SELECT 1 FROM ways_vertices WHERE geom IS NULL) THEN + RAISE WARNING 'Some vertices failed geometry update'; + END IF; +END $$;
33-63
: Consider adding indexes for performance optimization.Adding indexes on the component columns and join keys would improve query performance, especially for large datasets.
ALTER TABLE ways ADD COLUMN component BIGINT; ALTER TABLE ways_vertices ADD COLUMN component BIGINT; +CREATE INDEX ON ways(component); +CREATE INDEX ON ways_vertices(component); +CREATE INDEX ON ways(source_osm); +CREATE INDEX ON ways(target_osm);
65-124
: Add view documentation and consider parameterizing cost multipliers.The network views lack documentation explaining their purpose and assumptions. Additionally, the taxi cost multiplier is hardcoded.
+-- vehicle_net: Base network for vehicle routing +-- Excludes pedestrian-only paths CREATE VIEW vehicle_net AS WITH all_components AS (SELECT component, count(*) FROM ways GROUP BY component), max_component AS (SELECT max(count) from all_components), ... +-- taxi_net: Network for taxi routing with increased costs +-- Uses a 1.10 multiplier to account for taxi service premium CREATE VIEW taxi_net AS + WITH config AS ( + SELECT 1.10 as taxi_cost_multiplier -- Can be adjusted as needed + ) SELECT id, source, target, - cost * 1.10 AS cost, reverse_cost * 1.10 AS reverse_cost, + cost * taxi_cost_multiplier AS cost, + reverse_cost * taxi_cost_multiplier AS reverse_cost, name, length, geom - FROM vehicle_net + FROM vehicle_net, configdocs/basic/vehicle.rst (1)
363-370
: Improve documentation formatting consistency.The collapse directive is followed by unnecessary blank lines that should be removed for consistency with other similar sections.
.. collapse:: Query results - - .. literalinclude:: ../scripts/basic/chapter_5/exercise_5_5.txtdocs/basic/pedestrian.rst (1)
244-245
: Maintain consistent exercise numbering format.The exercise title format differs from other exercises in the document.
-Exercise 5: Combination of routes -............................................................................... +Exercise 5: Combination of routes +--------------------------------------------------------------------------------docs/un_sdg/sdg11-cities.rst (1)
358-359
: Improve variable naming in SQL examples.The column name 'rain_zone' could be more descriptive to indicate it stores buffer geometry.
- :start-after: get_rain_zone1.txt - :end-before: get_rain_zone2.txt + :start-after: get_rain_impact_zone1.txt + :end-before: get_rain_impact_zone2.txtdocs/basic/plpgsql_function.rst (1)
31-31
: Fix typo in the sentence.Change "if" to "it" in the sentence: "Returns a route that includes a geometry so that if can be displayed".
-Returns a route that includes a geometry so that if can be displayed, for example, in QGIS. +Returns a route that includes a geometry so that it can be displayed, for example, in QGIS.docs/basic/sql_function.rst (1)
67-67
: Fix typo in the column description.Change "leng" to "length" in the description.
-``length`` FLOAT The leng in meters of the segment. +``length`` FLOAT The length in meters of the segment.docs/basic/graph_views.rst (1)
62-64
: Consider enhancing the function description.The description could be more explicit about the function's purpose and usage. Consider adding a brief example or use case.
docs/un_sdg/sdg3-health.rst (2)
190-192
: Fix typo in data type name.Change "LINGSTING" to "LINESTRING".
-The table ``buildings_ways`` contains the buildings in ``LINGSTING`` type. +The table ``buildings_ways`` contains the buildings in ``LINESTRING`` type.
587-588
: Standardize speed unit notation.For consistency, use either "m/s" or "mts/sec" throughout the document. The document uses both notations.
docs/scripts/basic/chapter_6/images.sql (4)
2-10
: Consider parameterizing the SQL query.The SQL query string is hardcoded. Consider extracting it into a variable or parameter for better maintainability.
- ' SELECT gid AS id, source, target, cost, reverse_cost FROM ways ', + query := 'SELECT gid AS id, source, target, cost, reverse_cost FROM ways'; + SELECT * FROM pgr_dijkstra(query, @ID_1@, @ID_3@, directed := true)
44-52
: Consider adding an index for performance optimization.The JOIN operation between
ways
andconfiguration
tables might benefit from an index on thetag_id
column.CREATE INDEX IF NOT EXISTS idx_ways_tag_id ON ways(tag_id); CREATE INDEX IF NOT EXISTS idx_configuration_tag_id ON configuration(tag_id);
54-73
: Improve readability of the CASE expressions.The nested CASE expressions make the code hard to read. Consider creating a mapping table or using a more maintainable structure.
- CASE WHEN start_vid = @ID_1@ THEN '@PLACE_1@' WHEN start_vid = @ID_2@ THEN '@PLACE_2@' - WHEN start_vid = @ID_3@ THEN '@PLACE_3@' WHEN start_vid = @ID_4@ THEN '@PLACE_4@' - WHEN start_vid = @ID_5@ THEN '@PLACE_5@' END + CASE start_vid + WHEN @ID_1@ THEN '@PLACE_1@' + WHEN @ID_2@ THEN '@PLACE_2@' + WHEN @ID_3@ THEN '@PLACE_3@' + WHEN @ID_4@ THEN '@PLACE_4@' + WHEN @ID_5@ THEN '@PLACE_5@' + END
94-95
: Document the tag_id values.The hardcoded
tag_id
values (119, 122, 114, 118) lack documentation explaining what they represent.-SELECT * FROM ways where tag_id in (119, 122, 114, 118); +-- tag_ids: 119=steps, 122=footway, 114=pedestrian, 118=cycleway +SELECT * FROM ways where tag_id in ( + 119, -- steps + 122, -- footway + 114, -- pedestrian + 118 -- cycleway +);.github/workflows/website.yml (1)
75-75
: Consider using environment variables for CMake configuration.The CMake configuration could be more flexible by using environment variables.
- cmake -DES=ON .. + cmake -DES="${ES:-ON}" ..docs/scripts/basic/chapter_6/all_exercises.sql (3)
3-5
: Add explanation for negative cost check.The purpose of checking for negative costs should be explained in a comment.
+-- Check for invalid negative costs that could affect routing algorithms SELECT count(*) FROM ways WHERE cost < 0; -- line 3
52-57
: Document the speed conversion formula.The conversion formula
cost_s / 3600 * 100
needs explanation.+ -- Convert seconds to hours and scale by 100 for better readability cost_s / 3600 * 100 AS cost, -- line 6 reverse_cost_s / 3600 * 100 AS reverse_cost -- line 7
129-148
: Consider adding error handling.The nested query structure might benefit from error handling for invalid results.
+ -- Add error handling for no route found + WITH RECURSIVE route AS ( SELECT * FROM pgr_dijkstra( $$ SELECT gid AS id, source, target, cost_s AS cost, reverse_cost_s AS reverse_cost FROM ( -- penalized query SELECT edge AS gid FROM pgr_dijkstra( ' SELECT gid AS id, source, target, cost_s * penalty AS cost, reverse_cost_s * penalty AS reverse_cost FROM ways JOIN configuration USING (tag_id) ', @ID_3@, @ID_1@) ) AS edges_in_route JOIN ways USING (gid) $$, - @ID_3@, @ID_1@); + @ID_3@, @ID_1@) + ) + SELECT CASE + WHEN NOT EXISTS (SELECT 1 FROM route) THEN + RAISE EXCEPTION 'No route found between points % and %', @ID_3@, @ID_1@ + ELSE + SELECT * FROM route + END;docs/general-intro/introduction.rst (2)
74-75
: Fix typo in engineer title.There's a typo in the word "engineer".
-* *Ramón Ríos* Is a freelance developer from Mexico. Lead engenieer for +* *Ramón Ríos* Is a freelance developer from Mexico. Lead engineer for
65-65
: Fix incorrect alt text.The alt text for the Erosion logo incorrectly references Paragon Corporation.
- :alt: Paragon Corporation + :alt: Erosion
🛑 Comments failed to post (22)
locale/en/LC_MESSAGES/examples/boost_dijkstra.po (2)
12-12:
⚠️ Potential issueFix the POT creation date.
The POT creation date is set to November 2024, which is in the future. This could cause issues with build tools or version control systems.
-"POT-Creation-Date: 2024-11-04 21:10-0600\n" +"POT-Creation-Date: 2024-01-15 21:10-0600\n"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements."POT-Creation-Date: 2024-01-15 21:10-0600\n"
10-21: 💡 Codebase verification
Minor version string inconsistency found in Spanish localization
The localization files are mostly consistent, but there's one file that needs attention:
locale/es/LC_MESSAGES/basic/graph_views.po
is missing "Belém" in its Project-Id-Version string🔗 Analysis chain
Verify localization file updates across the project.
The metadata updates look good and align with the workshop rebranding. However, let's verify that these changes are consistent across all localization files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for consistency in version and dates across all PO files echo "Checking PO files for consistency..." find . -name "*.po" -type f -exec grep -H "Project-Id-Version:" {} \; find . -name "*.po" -type f -exec grep -H "POT-Creation-Date:" {} \;Length of output: 11508
docs/scripts/get_data/process_osgeolive_data.sh (1)
43-45: 🛠️ Refactor suggestion
Add logging and error handling for data cleanup operation.
While removing faulty ways is important for data integrity, the current implementation could be improved with better logging and error handling.
Consider this enhanced implementation:
-echo remove_faulty_ways_start -psql -c 'DELETE FROM ways WHERE length_m IS NULL;' -d city_routing -echo remove_faulty_ways_end +echo "Starting cleanup of ways with NULL length_m..." +psql -d city_routing << EOF + \set QUIET on + \t on + WITH deleted AS ( + DELETE FROM ways + WHERE length_m IS NULL + RETURNING * + ) + SELECT 'Removed ' || COUNT(*) || ' faulty ways with NULL length_m' + FROM deleted; +EOF +echo "Cleanup completed."This implementation:
- Provides clear count of affected rows
- Uses heredoc for better SQL readability
- Maintains atomic operation with better feedback
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.echo "Starting cleanup of ways with NULL length_m..." psql -d city_routing << EOF \set QUIET on \t on WITH deleted AS ( DELETE FROM ways WHERE length_m IS NULL RETURNING * ) SELECT 'Removed ' || COUNT(*) || ' faulty ways with NULL length_m' FROM deleted; EOF echo "Cleanup completed."
docs/basic/images/chapter5/CMakeLists.txt (1)
6-10: 💡 Codebase verification
Inconsistent image naming conventions detected
The new images exist but use a different naming convention (underscores) compared to other chapters:
- Chapter 5: Uses underscores (e.g.,
pedestrian_route1.png
)- Chapters 7-8: Use hyphens (e.g.,
ch7-e3.png
,ch8-walknet.png
)Consider standardizing the naming convention across all chapters to maintain consistency.
🔗 Analysis chain
Verify new images and naming consistency.
The naming convention has been updated from hyphen-separated to underscore-separated. Please ensure:
- All new image files exist
- Naming convention is consistent across all chapters
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check image existence and naming consistency cd docs/basic/images # Check if new images exist cd chapter5 for img in pedestrian_combinations.png pedestrian_route{1,2,4,5}.png; do if [ ! -f "$img" ]; then echo "Missing image in chapter5: $img" fi done # Check naming consistency across chapters cd .. find . -type f -name "*-*.png" | while read -r file; do echo "Found hyphenated filename: $file" doneLength of output: 2030
docs/images/logos/CMakeLists.txt (1)
10-11:
⚠️ Potential issueRemove duplicate entry for osm2pgrouting.png.
The file
osm2pgrouting.png
is listed twice inPGR_WORKSHOP_IMG_FILES
. Please remove one instance.osm_logo.png osm2pgrouting.png - osm2pgrouting.png pgrouting.png
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.osm2pgrouting.png
docs/scripts/get_data/get_all_data.sh (2)
6-7: 🛠️ Refactor suggestion
Add error handling and file verification.
The script should verify downloaded files and handle potential failures:
- No error handling for wget failures
- No verification of downloaded files
- URLs could be variables for easier maintenance
Consider this improved implementation:
+# Define URLs as variables for easier maintenance +OSGEOLIVE_URL="https://download.osgeo.org/livedvd/17/osm" +PGROUTING_URL="https://download.osgeo.org/pgrouting/workshops" + +# Function to download and verify +download_and_verify() { + local url="$1" + local file="$2" + echo "Downloading ${file}..." + if ! wget -N --progress=dot:mega "${url}/${file}.osm.bz2"; then + echo "Failed to download ${file}" + exit 1 + fi + if [ ! -f "${file}.osm.bz2" ]; then + echo "Downloaded file ${file}.osm.bz2 not found" + exit 1 + fi + echo "Extracting ${file}..." + bunzip2 -f "${file}.osm.bz2" + if [ ! -f "${file}.osm" ]; then + echo "Extracted file ${file}.osm not found" + exit 1 + fi +} + +# Download and extract files +download_and_verify "$OSGEOLIVE_URL" "@PGR_WORKSHOP_CITY_FILE@" +download_and_verify "$PGROUTING_URL" "mumbai" +download_and_verify "$PGROUTING_URL" "bangladesh" -wget -N --progress=dot:mega "https://download.osgeo.org/livedvd/17/osm/@PGR_WORKSHOP_CITY_FILE@.osm.bz2" -bunzip2 -f "@PGR_WORKSHOP_CITY_FILE@.osm.bz2" -wget -N --progress=dot:mega "https://download.osgeo.org/pgrouting/workshops/mumbai.osm.bz2" -bunzip2 -f "mumbai.osm.bz2" -wget -N --progress=dot:mega "https://download.osgeo.org/pgrouting/workshops/bangladesh.osm.bz2" -bunzip2 -f "bangladesh.osm.bz2"Also applies to: 11-12, 16-17
5-5: 🛠️ Refactor suggestion
Remove unused variable.
The
CITY
variable is defined but never used in the script.-CITY="@PGR_WORKSHOP_CITY_FILE@"
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 5-5: CITY appears unused. Verify use (or export if used externally).
(SC2034)
docs/scripts/basic/chapter_5/CMakeLists.txt (1)
8-8: 🛠️ Refactor suggestion
Add missing dependencies for images.sql.
The script
images.sql
has been added to the build process, but:
- Any output files generated by
images.sql
should be listed in BYPRODUCTSimages.sql
should be added to the DEPENDS list to ensure proper build dependenciesCOMMAND psql -d city_routing -f all_exercises_5.sql COMMAND psql -d city_routing -f images.sql COMMENT "running chapter 5 scripts" -DEPENDS all_exercises_5.sql +DEPENDS all_exercises_5.sql images.sqlAlso applies to: 29-29
docs/scripts/basic/chapter_7/CMakeLists.txt (1)
7-7: 🛠️ Refactor suggestion
Add missing dependency and align with chapter_5 structure.
The script
images.sql
has been added, but:
images.sql
should be added to the DEPENDS list- The file configuration approach differs from chapter_5's structure
COMMAND psql -d city_routing -f all_sections.sql COMMAND psql -d city_routing -f images.sql COMMENT "running chapter 7 scripts" -DEPENDS all_sections.sql +DEPENDS all_sections.sql images.sqlConsider using the same structure as chapter_5 for consistency:
-configure_file(all_sections.sql all_sections.sql) -configure_file(images.sql images.sql) +set(PGR_WORKSHOP_FILES + "all_sections.sql" + "images.sql" +) + +foreach (f ${PGR_WORKSHOP_FILES}) + if (PGR_WORKSHOP_VERBOSE_DEBUG) + message(STATUS " Adding file ${f}") + endif() + configure_file(${f} ${f}) +endforeach()Also applies to: 29-29
docs/scripts/basic/chapter_6/CMakeLists.txt (1)
30-33:
⚠️ Potential issueUpdate the DEPENDS clause to match new SQL files.
The DEPENDS clause still references
section-6.1.sql
, but this file appears to have been replaced byall_exercises.sql
andimages.sql
.Update the dependency:
-DEPENDS section-6.1.sql) +DEPENDS all_exercises.sql images.sql)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.COMMAND psql -d city_routing -f all_exercises.sql COMMAND psql -d city_routing -f images.sql COMMENT "running chapter 6 scripts" DEPENDS all_exercises.sql images.sql)
Readme.md (1)
24-29: 🛠️ Refactor suggestion
Enhance database setup instructions.
The build instructions include dropping the database but don't mention creating it.
Add the database creation step:
dropdb city_routing +createdb city_routing +psql -d city_routing -c 'CREATE EXTENSION postgis; CREATE EXTENSION pgrouting;' mkdir build cd build cmake ..📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.dropdb city_routing createdb city_routing psql -d city_routing -c 'CREATE EXTENSION postgis; CREATE EXTENSION pgrouting;' mkdir build cd build cmake .. make html cd ..
docs/scripts/basic/chapter_7/images.sql (1)
28-30:
⚠️ Potential issueReview SQL injection vulnerability in dynamic query.
The dynamic SQL construction using string concatenation could be vulnerable to SQL injection.
Consider using
format()
with proper type casting:-'SELECT * FROM ' || edges_subset, +format('SELECT * FROM %s', edges_subset),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.FROM pgr_dijkstra( format('SELECT * FROM %s', edges_subset), source, target)
.github/scripts/update_locale.sh (1)
40-41: 🛠️ Refactor suggestion
Add safety checks for file removal operations.
The xargs commands should include additional safety checks to prevent accidental removal of unintended files.
-git status locale/es --porcelain | awk 'match($1, "?"){print $2}' | xargs -r rm -rf -git status locale/ja --porcelain | awk 'match($1, "?"){print $2}' | xargs -r rm -rf +git status locale/es --porcelain | awk 'match($1, "?"){print $2}' | grep -E '^locale/es/' | xargs -r rm -rf +git status locale/ja --porcelain | awk 'match($1, "?"){print $2}' | grep -E '^locale/ja/' | xargs -r rm -rf📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.git status locale/es --porcelain | awk 'match($1, "?"){print $2}' | grep -E '^locale/es/' | xargs -r rm -rf git status locale/ja --porcelain | awk 'match($1, "?"){print $2}' | grep -E '^locale/ja/' | xargs -r rm -rf
docs/scripts/basic/chapter_5/images.sql (1)
69-76:
⚠️ Potential issueFix potential SQL injection vulnerability.
The VALUES clause in the subquery should use proper parameter binding instead of direct string interpolation.
- 'SELECT * FROM (VALUES (@ID_1@, @ID_4@), (@ID_2@, @ID_5@)) AS combinations (source, target)', + 'SELECT source, target FROM (VALUES ($1, $2), ($3, $4)) AS combinations (source, target)', + ARRAY[@ID_1@, @ID_4@, @ID_2@, @ID_5@],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.CREATE VIEW pedestrian_combinations AS WITH dijkstra AS ( SELECT * FROM pgr_dijkstra( 'SELECT gid AS id, source, target, length_m / 1.3 / 60 AS cost FROM ways', 'SELECT source, target FROM (VALUES ($1, $2), ($3, $4)) AS combinations (source, target)', ARRAY[@ID_1@, @ID_4@, @ID_2@, @ID_5@], directed := false) ) SELECT seq, start_vid, end_vid, the_geom AS geom FROM dijkstra JOIN ways ON(edge = gid);
.github/workflows/locale.yml (2)
29-41: 🛠️ Refactor suggestion
Fix shell script best practices.
The shell commands should follow best practices to prevent word splitting and globbing issues:
- pgver=$(psql --version | grep -Po '(?<=psql \(PostgreSQL\) )[^;]+(?=\.\d+ \()') - echo "PGVER=${pgver}" >> $GITHUB_ENV + pgver="$(psql --version | grep -Po '(?<=psql \(PostgreSQL\) )[^;]+(?=\.\d+ \()')" + echo "PGVER=${pgver}" >> "$GITHUB_ENV" - branch=${GITHUB_REF#refs/heads/} - git_hash=$(git rev-parse --short "$GITHUB_SHA") - echo "GIT_HASH=$git_hash" >> $GITHUB_ENV + branch="${GITHUB_REF#refs/heads/}" + git_hash="$(git rev-parse --short "$GITHUB_SHA")" + echo "GIT_HASH=${git_hash}" >> "$GITHUB_ENV"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.- name: Get postgres version run: | sudo service postgresql start pgver="$(psql --version | grep -Po '(?<=psql \(PostgreSQL\) )[^;]+(?=\.\d+ \()')" echo "PGVER=${pgver}" >> "$GITHUB_ENV" echo "PGIS=3" >> $GITHUB_ENV - name: Extract branch name and commit hash run: | branch="${GITHUB_REF#refs/heads/}" git_hash="$(git rev-parse --short "$GITHUB_SHA")" echo "GIT_HASH=${git_hash}" >> "$GITHUB_ENV"
🧰 Tools
🪛 actionlint (1.7.4)
30-30: shellcheck reported issue in this script: SC2086:info:3:26: Double quote to prevent globbing and word splitting
(shellcheck)
30-30: shellcheck reported issue in this script: SC2086:info:4:18: Double quote to prevent globbing and word splitting
(shellcheck)
37-37: shellcheck reported issue in this script: SC2034:warning:1:1: branch appears unused. Verify use (or export if used externally)
(shellcheck)
37-37: shellcheck reported issue in this script: SC2086:info:3:30: Double quote to prevent globbing and word splitting
(shellcheck)
71-74: 🛠️ Refactor suggestion
Enhance security by using GitHub secrets.
Consider using GitHub secrets for database passwords instead of hardcoding them:
- sudo -u postgres psql -c 'CREATE ROLE "runner" SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN PASSWORD $$runner$$;' -d setup - echo :5432:*:runner:runner >> .pgpass - sudo -u postgres psql -c 'CREATE ROLE "user" SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN PASSWORD $$user$$;' -d setup - echo :5432:*:user:user >> .pgpass + sudo -u postgres psql -c "CREATE ROLE \"runner\" SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN PASSWORD '${RUNNER_PASSWORD}';" -d setup + echo ":5432:*:runner:${RUNNER_PASSWORD}" >> .pgpass + sudo -u postgres psql -c "CREATE ROLE \"user\" SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN PASSWORD '${USER_PASSWORD}';" -d setup + echo ":5432:*:user:${USER_PASSWORD}" >> .pgpassAdd these secrets to your GitHub repository settings:
RUNNER_PASSWORD
USER_PASSWORD
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.sudo -u postgres createdb -p ${POSTGRES_PORT} setup sudo -u postgres psql -c "CREATE ROLE \"runner\" SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN PASSWORD '${RUNNER_PASSWORD}';" -d setup echo ":5432:*:runner:${RUNNER_PASSWORD}" >> .pgpass sudo -u postgres psql -c "CREATE ROLE \"user\" SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN PASSWORD '${USER_PASSWORD}';" -d setup echo ":5432:*:user:${USER_PASSWORD}" >> .pgpass
docs/general-intro/osgeolive.rst (1)
37-43: 🛠️ Refactor suggestion
Update package repository instructions for security.
The instructions for adding VirtualBox repository should use HTTPS and include verification steps.
Add the following security-related steps:
- deb https://download.virtualbox.org/virtualbox/debian <mydist> contrib + # 1. Add the repository + deb [arch=amd64 signed-by=/usr/share/keyrings/oracle-virtualbox-2016.gpg] https://download.virtualbox.org/virtualbox/debian <mydist> contrib + + # 2. Download and verify the key + wget -O- https://www.virtualbox.org/download/oracle_vbox_2016.asc | sudo gpg --dearmor --yes --output /usr/share/keyrings/oracle-virtualbox-2016.gpg📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Add the following line to your /etc/apt/sources.list. According to your distribution, replace `<mydist>` with your distribution name. :: # 1. Add the repository deb [arch=amd64 signed-by=/usr/share/keyrings/oracle-virtualbox-2016.gpg] https://download.virtualbox.org/virtualbox/debian <mydist> contrib # 2. Download and verify the key wget -O- https://www.virtualbox.org/download/oracle_vbox_2016.asc | sudo gpg --dearmor --yes --output /usr/share/keyrings/oracle-virtualbox-2016.gpg
docs/scripts/un_sdg/sdg3/all_exercises_sdg3.sql (2)
15-27: 🛠️ Refactor suggestion
Consider using transaction for table alterations.
Multiple column drops should be wrapped in a transaction for atomicity.
Wrap the ALTER TABLE operations in a transaction:
+BEGIN; ALTER TABLE buildings.buildings_ways DROP source, DROP target, DROP source_osm, DROP target_osm, DROP length, DROP length_m, DROP cost, DROP reverse_cost, DROP cost_s, DROP reverse_cost_s, DROP one_way, DROP oneway, DROP priority, DROP osm_id, DROP rule, DROP x1, DROP x2, DROP y1, DROP y2, DROP maxspeed_forward, DROP maxspeed_backward; +COMMIT;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.\o clean_buildings.txt BEGIN; ALTER TABLE buildings.buildings_ways DROP source, DROP target, DROP source_osm, DROP target_osm, DROP length, DROP length_m, DROP cost, DROP reverse_cost, DROP cost_s, DROP reverse_cost_s, DROP one_way, DROP oneway, DROP priority, DROP osm_id, DROP rule, DROP x1, DROP x2, DROP y1, DROP y2, DROP maxspeed_forward, DROP maxspeed_backward; COMMIT;
223-232: 🛠️ Refactor suggestion
Add error handling for population update.
The population update query lacks error handling for invalid edge_id values.
Add error handling:
+DO $$ +BEGIN UPDATE roads_ways SET population = SUM FROM ( SELECT edge_id, SUM(population) FROM buildings_ways GROUP BY edge_id ) AS subquery WHERE gid = edge_id; +IF NOT FOUND THEN + RAISE WARNING 'No roads were updated with population data'; +END IF; +END $$;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.\o add_road_population2.txt DO $$ BEGIN UPDATE roads_ways SET population = SUM FROM ( SELECT edge_id, SUM(population) FROM buildings_ways GROUP BY edge_id ) AS subquery WHERE gid = edge_id; IF NOT FOUND THEN RAISE WARNING 'No roads were updated with population data'; END IF; END $$;
docs/un_sdg/sdg7-energy.rst (1)
180-181:
⚠️ Potential issueFix incorrect file path in documentation.
The file path in the literalinclude directive points to sdg3 instead of sdg7.
- .. literalinclude:: ../scripts/un_sdg/sdg3/only_connected7.txt + .. literalinclude:: ../scripts/un_sdg/sdg7/only_connected7.txt📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements... literalinclude:: ../scripts/un_sdg/sdg7/only_connected7.txt
.github/workflows/website.yml (2)
20-20:
⚠️ Potential issueUpdate the checkout action version.
The runner version is too old for
actions/checkout@v3
. Update to the latest version.- uses: actions/checkout@v3 + uses: actions/checkout@v4📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.uses: actions/checkout@v4
🧰 Tools
🪛 actionlint (1.7.4)
20-20: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
27-27:
⚠️ Potential issueFix shellcheck issues in the script.
The script has potential globbing and word splitting issues.
- pgver=$(psql --version | grep -Po '(?<=psql \(PostgreSQL\) )[^;]+(?=\.\d+ \()') + pgver="$(psql --version | grep -Po '(?<=psql \(PostgreSQL\) )[^;]+(?=\.\d+ \()')"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.pgver="$(psql --version | grep -Po '(?<=psql \(PostgreSQL\) )[^;]+(?=\.\d+ \()')"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🔭 Outside diff range comments (5)
locale/en/LC_MESSAGES/interactions/chapter-9.po (1)
Line range hint
42-42
: Update QGIS version reference.The instructions reference QGIS 2.14 Essen, which is significantly outdated. Consider updating to match the current QGIS version available in OSGeoLive.
.github/scripts/remove_obsolete_entries.sh (1)
Line range hint
10-15
: Improve robustness of file handling.The current implementation using
find
with a pipe tofor
loop is fragile and may break with filenames containing spaces or special characters.Consider using one of these more robust approaches:
-for file in $(find locale -type f -name "*.po"); do - if grep -q '#~' $file; then - perl -pi -0777 -e 's/#~.*//s' $file - git add $file - fi -done +# Option 1: Using find -exec +find locale -type f -name "*.po" -exec sh -c ' + if grep -q "#~" "$1"; then + perl -pi -0777 -e "s/#~.*//s" "$1" + git add "$1" + fi +' sh {} \; + +# Option 2: Using while read +find locale -type f -name "*.po" -print0 | while IFS= read -r -d "" file; do + if grep -q '#~' "$file"; then + perl -pi -0777 -e 's/#~.*//s' "$file" + git add "$file" + fi +done🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 10-10: For loops over find output are fragile. Use find -exec or a while read loop.
(SC2044)
docs/scripts/un_sdg/sdg3/all_exercises_sdg3.sql (2)
Line range hint
54-82
: Population function needs input validation.The population calculation function should validate its inputs.
Add input validation:
CREATE OR REPLACE FUNCTION population(tag_id INTEGER, area INTEGER) RETURNS INTEGER AS $BODY$ DECLARE population INTEGER; BEGIN + IF area < 0 THEN + RAISE EXCEPTION 'Area cannot be negative: %', area; + END IF; + IF tag_id IS NULL THEN + RAISE EXCEPTION 'tag_id cannot be null'; + END IF; IF tag_id <= 100 THEN population = 1; -- Negligible ELSIF 100 < tag_id AND tag_id <= 200 THEN population = GREATEST(2, area * 0.0002); -- Very Sparse ELSIF 200 < tag_id AND tag_id <= 300 THEN population = GREATEST(3, area * 0.002); -- Sparse ELSIF 300 < tag_id AND tag_id <= 400 THEN population = GREATEST(5, area * 0.05); -- Moderate ELSIF 400 < tag_id AND tag_id <= 500 THEN population = GREATEST(7, area * 0.7); -- Dense ELSIF tag_id > 500 THEN population = GREATEST(10,area * 1); -- Very Dense ELSE population = 1; END IF; RETURN population; END; $BODY$ LANGUAGE plpgsql;
Line range hint
203-265
: Improve error handling in component comparison.The component comparison logic should handle edge cases better.
Add error handling and validation:
WITH results AS ( SELECT seq, edge AS id, node, cost AS seconds FROM pgr_dijkstra('SELECT * FROM vehicle_net', @CH7_OSMID_1@, @CH7_OSMID_2@) ), compare AS ( SELECT seq, id, lead(seq) over(ORDER BY seq) AS next_seq, ST_AsText(ST_endPoint(geom)) AS id_end, ST_AsText(ST_startPoint(lead(geom) over(ORDER BY seq))) AS next_id_start + FROM results + LEFT JOIN vehicle_net USING (id) + WHERE id IS NOT NULL -- Handle missing edges + ORDER BY seq ) -FROM results LEFT JOIN vehicle_net USING (id) -ORDER BY seq) SELECT * FROM compare WHERE id_end != next_id_start;docs/scripts/basic/chapter_7/all_sections.sql (1)
Line range hint
302-348
: Improve wrk_dijkstra function performance and safety.The function needs input validation and performance optimization.
Enhance the function:
CREATE OR REPLACE FUNCTION wrk_dijkstra( IN edges_subset REGCLASS, IN source BIGINT, IN target BIGINT, OUT seq INTEGER, OUT id BIGINT, OUT seconds FLOAT, OUT name TEXT, OUT length_m FLOAT, OUT route_readable TEXT, OUT route_geom geometry, OUT azimuth FLOAT ) RETURNS SETOF record AS $BODY$ +DECLARE + query text; +BEGIN + IF edges_subset IS NULL THEN + RAISE EXCEPTION 'edges_subset cannot be null'; + END IF; + + query := format('SELECT * FROM %I', edges_subset); + WITH results AS ( SELECT seq, edge AS id, cost AS seconds, node FROM pgr_dijkstra( - 'SELECT * FROM ' || edges_subset, + query, source, target) ), additional AS ( SELECT seq, id, seconds, name, length_m, CASE WHEN node = source THEN ST_AsText(the_geom) ELSE ST_AsText(ST_Reverse(the_geom)) END AS route_readable, CASE WHEN node = source_osm THEN the_geom ELSE ST_Reverse(the_geom) END AS route_geom FROM results LEFT JOIN ways ON (gid = id) ) SELECT *, degrees(ST_azimuth(ST_StartPoint(route_geom), ST_EndPoint(route_geom))) AS azimuth FROM additional ORDER BY seq; +END; $BODY$ -LANGUAGE 'sql'; +LANGUAGE plpgsql;
🧹 Nitpick comments (35)
locale/en/LC_MESSAGES/basic/sql_function.po (2)
219-219
: Fix the typo in “The leng in meters of the segment.”Change “leng” to “length”.
- msgid "The leng in meters of the segment." + msgid "The length in meters of the segment."
558-558
: Minor grammar correction for “pgRouting uses heavely function overloading.”Replace “heavely” with “heavily.”
- msgid "pgRouting uses heavely function overloading:" + msgid "pgRouting uses heavily function overloading:"locale/en/LC_MESSAGES/appendix/appendix-3.po (1)
45-45
: Fix the grammar “a lot or memory.”Use “a lot of memory” instead.
- which makes it fast but also requires a lot or memory for large datasets. + which makes it fast but also requires a lot of memory for large datasets.locale/en/LC_MESSAGES/general-intro/introduction.po (1)
138-140
: Update presenter information.Consider adding brief professional descriptions for both presenters to maintain consistency in the documentation.
Also applies to: 142-143
locale/es/LC_MESSAGES/basic/data.po (1)
177-180
: Fix formatting in documentation reference.There's a formatting issue in the Spanish translation where the documentation reference is not properly closed.
-"adicional sobre ``osm2pgrouting :doc:`../appendix/appendix-3`" +"adicional sobre ``osm2pgrouting`` en :doc:`../appendix/appendix-3`"locale/en/LC_MESSAGES/un_sdg/sdg11-cities.po (3)
219-221
: Fix typo in the word "which"There's a typo in the word "wich" on line 220.
-" wich is in a lower altitude of the city, are to be removed from the " +" which is in a lower altitude of the city, are to be removed from the "
255-257
: Fix typo in the word "explained"There's a typo in the word "explaind" on line 256.
-"this task and its explaind with more detail in " +"this task and it's explained with more detail in "
324-326
: Fix typo in the word "exercise"There's a typo in the word "excercise" on lines 324 and 325.
-"In this excercise the area , where if it rains, the city would be " -"affected, is calculated. This area is called ``rain zone`` in the " -"excercise" +"In this exercise the area, where if it rains, the city would be " +"affected, is calculated. This area is called ``rain zone`` in the " +"exercise"locale/es/LC_MESSAGES/advanced/chapter-12.po (1)
160-162
: Improve Spanish translation flowThe Spanish translation could be improved for better readability.
-"destino para cada vínculo de carretera. Los resultados anteriores " -"muestran que la topología de red no tiene ninguna información de origen y" -" destino." +"destino para cada enlace de carretera. Los resultados anteriores " +"muestran que la topología de red carece de información de origen y" +" destino."locale/en/LC_MESSAGES/basic/vehicle.po (1)
198-205
: Improve clarity of cost explanationThe explanation of costs could be clearer by using consistent terminology.
-"Use ``cost_s`` (line **6**) and ``reverse_cost_s`` (line **7**) columns, " -"in units seconds." +"Use ``cost_s`` (line **6**) and ``reverse_cost_s`` (line **7**) columns, " +"which represent costs in seconds."locale/en/LC_MESSAGES/basic/pedestrian.po (2)
451-452
: Fix line wrapping in cost explanationThe line wrapping in the cost explanation could be improved.
-"The cost to be in minutes, with a walking speed s = 1.3 m/s and t = d/s " -"(line **6**)." +"The cost to be in minutes, with a walking speed s = 1.3 m/s and t = d/s" +" (line **6**)."
458-459
: Update location names for consistencyEnsure location names are consistently formatted with quotes.
-"depart from \"Instituto Federal do Pará, Campus Belém\" than from " -"\"Hangar Convention Center\"" +"depart from \"|place_2|\" than from " +"\"|place_1|\""docs/scripts/get_data/get_all_data.sh (1)
11-12
: Consider adding error handling for downloads.The script could benefit from error handling for failed downloads or corrupted archives.
Consider adding checks:
+download_and_extract() { + local url="$1" + local file="${url##*/}" + echo "Downloading $file..." + if ! wget -N --progress=dot:mega "$url"; then + echo "Failed to download $file" >&2 + return 1 + fi + echo "Extracting $file..." + if ! bunzip2 -f "$file"; then + echo "Failed to extract $file" >&2 + return 1 + fi +} -wget -N --progress=dot:mega "https://download.osgeo.org/pgrouting/workshops/mumbai.osm.bz2" -bunzip2 -f "mumbai.osm.bz2" +download_and_extract "https://download.osgeo.org/pgrouting/workshops/mumbai.osm.bz2" -wget -N --progress=dot:mega "https://download.osgeo.org/pgrouting/workshops/bangladesh.osm.bz2" -bunzip2 -f "bangladesh.osm.bz2" +download_and_extract "https://download.osgeo.org/pgrouting/workshops/bangladesh.osm.bz2"Also applies to: 16-17
Readme.md (1)
24-29
: Add error handling to database operations.Consider adding error handling for the database drop operation to prevent failures if the database doesn't exist.
-dropdb city_routing +dropdb --if-exists city_routingdocs/scripts/get_data/process_osgeolive_data.sh (1)
43-45
: LGTM! Data cleanup step properly added.Good addition of the cleanup step to remove faulty ways. Consider adding error handling to verify the deletion operation's success.
echo remove_faulty_ways_start -psql -c 'DELETE FROM ways WHERE length_m IS NULL;' -d city_routing +psql -v ON_ERROR_STOP=1 -c 'DELETE FROM ways WHERE length_m IS NULL;' -d city_routing echo remove_faulty_ways_end.github/scripts/update_locale.sh (1)
33-37
: Consider using safer file handling practices.The script assumes the existence of files and directories without validation.
+# Check if locale_changes.txt exists +[ ! -f build/docs/locale_changes.txt ] && { echo "locale_changes.txt not found"; exit 1; } + # List all the files that needs to be committed in build/docs/locale_changes.txt awk '/^Update|^Create/{print $2}' build/docs/locale_changes.txt > build/docs/locale_changes_po.txt # .po files cp build/docs/locale_changes_po.txt build/docs/locale_changes_po_pot.txt perl -ne '/\/en\// && print' build/docs/locale_changes_po.txt | \ perl -pe 's/(.*)en\/LC_MESSAGES(.*)/$1pot$2t/' >> build/docs/locale_changes_po_pot.txt # .pot filesdocs/scripts/basic/chapter_5/images.sql (1)
69-76
: New view for pedestrian combinations uses efficient approach.The new view effectively:
- Uses VALUES clause for source-target pairs
- Maintains consistency with other views
- Provides clear output structure
However, consider adding a comment explaining the purpose of this view.
+-- View for calculating routes between specific source-target combinations +-- Used for analyzing multiple pedestrian paths simultaneously CREATE VIEW pedestrian_combinations ASdocs/scripts/un_sdg/sdg11/all_exercises_sdg11.sql (2)
3-8
: Consider adding table constraints.The bangladesh table would benefit from:
- Primary key constraint on id
- NOT NULL constraints where appropriate
- UNIQUE constraint on name
CREATE TABLE bangladesh ( - id BIGINT, - name TEXT, + id BIGINT PRIMARY KEY, + name TEXT NOT NULL UNIQUE, geom geometry, city_buffer geometry );
63-64
: Good use of connected components analysis.The implementation correctly:
- Adds component columns to both tables
- Uses pgr_connectedComponents for analysis
- Updates components based on topology
Consider adding an index on the component columns for better query performance.
ALTER TABLE waterways_ways ADD COLUMN component BIGINT; ALTER TABLE waterways_vertices ADD COLUMN component BIGINT; +CREATE INDEX idx_waterways_ways_component ON waterways_ways(component); +CREATE INDEX idx_waterways_vertices_component ON waterways_vertices(component);Also applies to: 68-72
docs/scripts/un_sdg/sdg7/all_exercises_sdg7.sql (1)
83-84
: Consider adding error handling for length calculations.The length calculations should handle NULL values and invalid geometries.
-SELECT sum(cost)/1000 AS material_km +SELECT COALESCE(sum(CASE WHEN cost IS NOT NULL AND cost > 0 THEN cost ELSE 0 END), 0)/1000 AS material_km FROM pgr_kruskalDFS('edges', 91); -SELECT sum(length_m)/1000 AS total_km +SELECT COALESCE(sum(CASE WHEN length_m IS NOT NULL AND length_m > 0 THEN length_m ELSE 0 END), 0)/1000 AS total_km FROM roads_ways;Also applies to: 88-89
docs/scripts/basic/chapter_6/images.sql (1)
94-95
: Document the tag_ids in the pedestrian_only_roads view.The view uses specific tag_ids (119, 122, 114, 118) without documentation explaining what they represent.
Consider adding a comment explaining what each tag_id represents:
CREATE VIEW pedestrian_only_roads AS +-- tag_ids: 119 (footway), 122 (steps), 114 (pedestrian), 118 (cycleway) SELECT * FROM ways where tag_id in (119, 122, 114, 118);
docs/general-intro/introduction.rst (1)
74-74
: Fix typo in engineer title.Correct the spelling from "engenieer" to "engineer".
-* *Ramón Ríos* Is a freelance developer from Mexico. Lead engenieer for +* *Ramón Ríos* Is a freelance developer from Mexico. Lead engineer fordocs/scripts/un_sdg/sdg3/all_exercises_sdg3.sql (1)
146-151
: Optimize closest_vertex function.The function could benefit from using an index for better performance.
Add index hint:
CREATE OR REPLACE FUNCTION closest_vertex(geom GEOMETRY) RETURNS BIGINT AS $BODY$ -SELECT id FROM roads_vertices ORDER BY geom <-> $1 LIMIT 1; +SELECT id FROM roads_vertices ORDER BY geom <-> $1::geometry LIMIT 1; $BODY$ LANGUAGE SQL;Also, consider adding an index if not already present:
CREATE INDEX IF NOT EXISTS idx_roads_vertices_geom ON roads_vertices USING GIST(geom);docs/scripts/basic/chapter_7/all_sections.sql (1)
9-12
: Add index for vertex extraction performance.The vertex extraction query could benefit from an index on the id columns.
Add indices before vertex extraction:
CREATE INDEX IF NOT EXISTS idx_ways_source_osm ON ways(source_osm); CREATE INDEX IF NOT EXISTS idx_ways_target_osm ON ways(target_osm);docs/un_sdg/sdg7-energy.rst (3)
Line range hint
58-65
: Improve documentation clarity.The introduction could be more specific about the data requirements and prerequisites.
Consider adding:
- Required data format
- Expected schema structure
- Prerequisites for following the exercises
225-231
: Add error handling examples in SQL queries.The SQL examples should include error handling patterns.
Consider adding error handling examples:
DO $$ BEGIN -- Existing query with error handling IF NOT EXISTS (SELECT 1 FROM ways LIMIT 1) THEN RAISE EXCEPTION 'No data found in ways table'; END IF; -- ... rest of the query ... EXCEPTION WHEN OTHERS THEN RAISE NOTICE 'Error executing query: %', SQLERRM; RAISE; END $$;
281-284
: Add performance metrics to the output section.The output section could benefit from additional performance metrics.
Consider adding:
- Query execution time
- Memory usage estimates
- Index usage statistics
docs/basic/graph_views.rst (3)
89-90
: Fix typo in documentation.There's a missing "to" in the sentence.
-have the set of vertices defined. +to have the set of vertices defined.
158-160
: Improve sentence structure.The sentence structure can be improved for better readability.
-The importance of counting the information on this workshop is to make sure that -the same data is used and consequently the results are same. -Also, some of the rows can be seen to understand the structure of the table and +The importance of counting the information in this workshop is to ensure that +the same data is used and consequently the results are consistent. +Additionally, some of the rows can be examined to understand the structure of the table and
Line range hint
636-639
: Fix typo and improve clarity.There are typos and the sentence can be clearer.
-It is noticable from the output figure that some of the roads which are near to -the hospital are not highlighted. For example, to roads in the north of the -hospital. This is because the only one edge per road vertex was selected by the +It is noticeable from the output figure that some of the roads which are near to +the hospital are not highlighted. For example, the roads in the north of the +hospital. This is because only one edge per road vertex was selected by thedocs/un_sdg/sdg3-health.rst (3)
198-199
: Fix typo in documentation.There's a typo in the word "buildings".
-columns that are not related to a **buidling** concept are dropped from +columns that are not related to a **building** concept are dropped from
673-679
: Improve paragraph structure and clarity.The paragraph can be restructured for better readability.
-Now the next step is to estimate the dependant population. Official source of -population is Census conducted by the government. But for this exercise, population -will be estimated from the ``area`` as well as the ``category`` of the building. -This area will be stored in the nearest roads. Following steps explain this -process in detail. +The next step is to estimate the dependent population. While the official source of +population data is the government census, for this exercise, population +will be estimated based on the ``area`` and ``category`` of each building. +This population estimate will be stored in the nearest roads. The following steps explain this +process in detail.
772-774
: Fix sentence structure.The sentence can be improved for better clarity.
-Final step is to find the total population served by the hospital based on -travel time. +The final step is to calculate the total population served by the hospital based on +travel time.docs/basic/vehicle.rst (1)
347-377
: Well-structured new exercise with clear progression.The new Exercise 6 is well-organized and logically follows from Exercise 5, providing a clear connection between penalized routes and their actual time costs. Consider adding a brief explanation of why this calculation is important for route planning.
Add a brief introduction explaining the practical importance of calculating actual time costs for penalized routes, such as:
Exercise 6: Time in seconds of penalized route ............................................................................... + +Understanding the actual time costs of penalized routes is crucial for realistic route planning, +as it helps validate whether the penalization achieves the desired effect without +making routes impractically slow. + .. rubric:: Problem:docs/un_sdg/sdg11-cities.rst (1)
Line range hint
60-67
: Clear explanation of city selection with good context.The explanation for choosing Munshigang effectively establishes the context for the exercises. Consider adding specific geographical coordinates in the introduction for immediate reference.
Add coordinates in the introduction:
For this exercise, Munshigang city from Bangladesh is chosen. This city has multiple rivers in its proximity which makes it an apt location to demonstrate this exercise. +Located at approximately <latitude>, <longitude>, Munshigang's geographical position +makes it particularly suitable for this analysis. The exercise will try to find the areas, where if it rains the city will be affected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (28)
docs/basic/images/chapter5/pedestrian-route1.png
is excluded by!**/*.png
docs/basic/images/chapter5/pedestrian-route2.png
is excluded by!**/*.png
docs/basic/images/chapter5/pedestrian-route4.png
is excluded by!**/*.png
docs/basic/images/chapter5/pedestrian-route5.png
is excluded by!**/*.png
docs/basic/images/chapter5/pedestrian_combinations.png
is excluded by!**/*.png
docs/basic/images/chapter5/pedestrian_route1.png
is excluded by!**/*.png
docs/basic/images/chapter5/pedestrian_route2.png
is excluded by!**/*.png
docs/basic/images/chapter5/pedestrian_route4.png
is excluded by!**/*.png
docs/basic/images/chapter5/pedestrian_route5.png
is excluded by!**/*.png
docs/basic/images/chapter5/route.png
is excluded by!**/*.png
docs/basic/images/chapter6/ad11.png
is excluded by!**/*.png
docs/basic/images/chapter6/ad7.png
is excluded by!**/*.png
docs/basic/images/chapter6/ad8.png
is excluded by!**/*.png
docs/basic/images/chapter6/detailofroute9.png
is excluded by!**/*.png
docs/basic/images/chapter6/pedestrian-only-roads.png
is excluded by!**/*.png
docs/basic/images/chapter6/pedestrian_only_roads.png
is excluded by!**/*.png
docs/basic/images/chapter6/route_using_pedestrian.png
is excluded by!**/*.png
docs/basic/images/chapter7/ch7-e1.png
is excluded by!**/*.png
docs/basic/images/chapter7/ch7-e2.png
is excluded by!**/*.png
docs/basic/images/chapter7/ch7-e4.png
is excluded by!**/*.png
docs/basic/images/chapter7/ch7-e6.png
is excluded by!**/*.png
docs/basic/images/chapter7/ch7-e8.png
is excluded by!**/*.png
docs/basic/images/chapter7/taxi_net.png
is excluded by!**/*.png
docs/basic/images/chapter7/vehicle_net.png
is excluded by!**/*.png
docs/basic/images/chapter7/walk-net.png
is excluded by!**/*.png
docs/basic/images/chapter7/walk_net.png
is excluded by!**/*.png
docs/images/logos/erosion.png
is excluded by!**/*.png
docs/un_sdg/images/sdg11/remove_waterways.png
is excluded by!**/*.png
📒 Files selected for processing (88)
.github/dependabot.yml
(1 hunks).github/scripts/remove_obsolete_entries.sh
(1 hunks).github/scripts/update_locale.sh
(1 hunks).github/workflows/locale.yml
(1 hunks).github/workflows/ubuntu.yml
(2 hunks).github/workflows/website.yml
(3 hunks)CMakeLists.txt
(4 hunks)REQUIREMENTS.txt
(1 hunks)Readme.md
(1 hunks)docs/CMakeLists.txt
(2 hunks)docs/advanced/chapter-12.rst
(1 hunks)docs/appendix/appendix-2.rst
(1 hunks)docs/appendix/appendix-3.rst
(1 hunks)docs/basic/CMakeLists.txt
(1 hunks)docs/basic/appendix.rst
(0 hunks)docs/basic/data.rst
(4 hunks)docs/basic/graph_views.rst
(1 hunks)docs/basic/images/chapter5/CMakeLists.txt
(1 hunks)docs/basic/images/chapter6/CMakeLists.txt
(1 hunks)docs/basic/images/chapter7/CMakeLists.txt
(1 hunks)docs/basic/pedestrian.rst
(14 hunks)docs/basic/plpgsql_function.rst
(9 hunks)docs/basic/sql_function.rst
(7 hunks)docs/basic/vehicle.rst
(9 hunks)docs/conf.py
(6 hunks)docs/general-intro/introduction.rst
(2 hunks)docs/general-intro/osgeolive.rst
(1 hunks)docs/general-intro/overview.rst
(2 hunks)docs/images/_dsZWQsq
(1 hunks)docs/images/logos/CMakeLists.txt
(1 hunks)docs/index.rst
(2 hunks)docs/interactions/chapter-9.rst
(1 hunks)docs/scripts/basic/chapter_5/all_exercises_5.sql
(2 hunks)docs/scripts/basic/chapter_5/images.sql
(4 hunks)docs/scripts/basic/chapter_6/CMakeLists.txt
(1 hunks)docs/scripts/basic/chapter_6/all_exercises.sql
(6 hunks)docs/scripts/basic/chapter_6/images.sql
(2 hunks)docs/scripts/basic/chapter_7/all_sections.sql
(3 hunks)docs/scripts/basic/chapter_7/images.sql
(1 hunks)docs/scripts/basic/chapter_8/all-sections-8.sql
(5 hunks)docs/scripts/get_data/get_all_data.sh
(1 hunks)docs/scripts/get_data/process_osgeolive_data.sh
(1 hunks)docs/scripts/un_sdg/sdg11/all_exercises_sdg11.sql
(1 hunks)docs/scripts/un_sdg/sdg3/all_exercises_sdg3.sql
(2 hunks)docs/scripts/un_sdg/sdg7/all_exercises_sdg7.sql
(1 hunks)docs/un_sdg/CMakeLists.txt
(0 hunks)docs/un_sdg/appendix.rst
(0 hunks)docs/un_sdg/data.rst
(1 hunks)docs/un_sdg/images/sdg11/CMakeLists.txt
(1 hunks)docs/un_sdg/introduction.rst
(5 hunks)docs/un_sdg/sdg11-cities.rst
(3 hunks)docs/un_sdg/sdg3-health.rst
(9 hunks)docs/un_sdg/sdg7-energy.rst
(4 hunks)locale/en/LC_MESSAGES/advanced/chapter-12.po
(3 hunks)locale/en/LC_MESSAGES/appendix/appendix-2.po
(3 hunks)locale/en/LC_MESSAGES/appendix/appendix-3.po
(3 hunks)locale/en/LC_MESSAGES/appendix/appendix-4.po
(1 hunks)locale/en/LC_MESSAGES/basic/appendix.po
(0 hunks)locale/en/LC_MESSAGES/basic/data.po
(3 hunks)locale/en/LC_MESSAGES/basic/graph_views.po
(1 hunks)locale/en/LC_MESSAGES/basic/pedestrian.po
(5 hunks)locale/en/LC_MESSAGES/basic/plpgsql_function.po
(3 hunks)locale/en/LC_MESSAGES/basic/sql_function.po
(3 hunks)locale/en/LC_MESSAGES/basic/vehicle.po
(4 hunks)locale/en/LC_MESSAGES/examples/boost_dijkstra.po
(1 hunks)locale/en/LC_MESSAGES/examples/hanoslav.po
(1 hunks)locale/en/LC_MESSAGES/examples/wiki_example.po
(1 hunks)locale/en/LC_MESSAGES/general-intro/introduction.po
(6 hunks)locale/en/LC_MESSAGES/general-intro/osgeolive.po
(3 hunks)locale/en/LC_MESSAGES/general-intro/overview.po
(6 hunks)locale/en/LC_MESSAGES/index.po
(3 hunks)locale/en/LC_MESSAGES/interactions/chapter-10.po
(2 hunks)locale/en/LC_MESSAGES/interactions/chapter-11.po
(2 hunks)locale/en/LC_MESSAGES/interactions/chapter-9.po
(3 hunks)locale/en/LC_MESSAGES/un_sdg/appendix.po
(0 hunks)locale/en/LC_MESSAGES/un_sdg/data.po
(8 hunks)locale/en/LC_MESSAGES/un_sdg/introduction.po
(5 hunks)locale/en/LC_MESSAGES/un_sdg/sdg11-cities.po
(6 hunks)locale/en/LC_MESSAGES/un_sdg/sdg3-health.po
(5 hunks)locale/en/LC_MESSAGES/un_sdg/sdg7-energy.po
(6 hunks)locale/es/LC_MESSAGES/advanced/chapter-12.po
(5 hunks)locale/es/LC_MESSAGES/appendix/appendix-2.po
(5 hunks)locale/es/LC_MESSAGES/appendix/appendix-3.po
(4 hunks)locale/es/LC_MESSAGES/appendix/appendix-4.po
(1 hunks)locale/es/LC_MESSAGES/basic/appendix.po
(0 hunks)locale/es/LC_MESSAGES/basic/data.po
(3 hunks)locale/es/LC_MESSAGES/basic/graph_views.po
(1 hunks)locale/es/LC_MESSAGES/basic/pedestrian.po
(12 hunks)
💤 Files with no reviewable changes (6)
- docs/basic/appendix.rst
- locale/en/LC_MESSAGES/basic/appendix.po
- docs/un_sdg/CMakeLists.txt
- locale/en/LC_MESSAGES/un_sdg/appendix.po
- docs/un_sdg/appendix.rst
- locale/es/LC_MESSAGES/basic/appendix.po
✅ Files skipped from review due to trivial changes (18)
- docs/appendix/appendix-2.rst
- docs/images/logos/CMakeLists.txt
- docs/interactions/chapter-9.rst
- locale/es/LC_MESSAGES/appendix/appendix-4.po
- .github/dependabot.yml
- docs/un_sdg/data.rst
- docs/appendix/appendix-3.rst
- docs/un_sdg/introduction.rst
- docs/general-intro/osgeolive.rst
- locale/en/LC_MESSAGES/appendix/appendix-2.po
- locale/en/LC_MESSAGES/interactions/chapter-10.po
- locale/en/LC_MESSAGES/examples/wiki_example.po
- locale/en/LC_MESSAGES/examples/boost_dijkstra.po
- locale/en/LC_MESSAGES/appendix/appendix-4.po
- locale/en/LC_MESSAGES/un_sdg/data.po
- locale/en/LC_MESSAGES/interactions/chapter-11.po
- docs/images/_dsZWQsq
- locale/en/LC_MESSAGES/examples/hanoslav.po
🧰 Additional context used
🪛 Shellcheck (0.10.0)
.github/scripts/remove_obsolete_entries.sh
[warning] 10-10: For loops over find output are fragile. Use find -exec or a while read loop.
(SC2044)
🪛 actionlint (1.7.4)
.github/workflows/locale.yml
30-30: shellcheck reported issue in this script: SC2086:info:3:26: Double quote to prevent globbing and word splitting
(shellcheck)
30-30: shellcheck reported issue in this script: SC2086:info:4:18: Double quote to prevent globbing and word splitting
(shellcheck)
37-37: shellcheck reported issue in this script: SC2034:warning:1:1: branch appears unused. Verify use (or export if used externally)
(shellcheck)
37-37: shellcheck reported issue in this script: SC2086:info:3:30: Double quote to prevent globbing and word splitting
(shellcheck)
55-55: shellcheck reported issue in this script: SC2086:info:3:14: Double quote to prevent globbing and word splitting
(shellcheck)
55-55: shellcheck reported issue in this script: SC2086:info:3:31: Double quote to prevent globbing and word splitting
(shellcheck)
55-55: shellcheck reported issue in this script: SC2086:info:4:14: Double quote to prevent globbing and word splitting
(shellcheck)
55-55: shellcheck reported issue in this script: SC2086:info:4:31: Double quote to prevent globbing and word splitting
(shellcheck)
55-55: shellcheck reported issue in this script: SC2086:info:5:14: Double quote to prevent globbing and word splitting
(shellcheck)
67-67: shellcheck reported issue in this script: SC2086:info:4:30: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (81)
locale/en/LC_MESSAGES/basic/sql_function.po (3)
2-5
: Update to Belém references looks correct.No issues found with the project name, version, and copyrights.
10-12
: Project metadata changes are consistent.The updated POT-Creation-Date and Babel version align with the new release.
Also applies to: 21-21
53-82
: New lines explaining output columns are clear.These updated lines describing routing output columns (e.g.,
seq
,id
,name
, etc.) appear logically consistent and well formatted.locale/en/LC_MESSAGES/index.po (3)
2-5
: Belém workshop references are consistent.No concerns regarding updated year, project name, and version.
10-12
: Project metadata updates are correct.The new POT-Creation-Date and Babel version are coherent with the workshop’s updated timeline.
Also applies to: 21-21
43-43
: No issues with updated headings.All headings for “Interaction with other software,” “Examples from the Internet,” and “Appendices” look consistent.
Also applies to: 47-47, 51-51
locale/en/LC_MESSAGES/appendix/appendix-3.po (2)
2-5
: Belém version references look good.No concerns about the updated project version or copyright.
10-12
: Metadata updates are consistent.The new creation date and Babel version align with the broader changes.
Also applies to: 21-21
locale/es/LC_MESSAGES/appendix/appendix-2.po (3)
11-11
: LGTM: Metadata updates are consistent.The updates to project version, dates, and technical specifications are correctly reflected in the Spanish localization file.
Also applies to: 13-14, 20-20, 23-24
44-50
: Verify translation accuracy for Geoserver reference.The translation maintains technical accuracy while being properly localized to Spanish. However, verify that "aplicación de ruteo" is the preferred translation in your localization guide for "routing application".
66-70
: Translations maintain technical accuracy.The Spanish translations for installation instructions and configuration steps are accurate and maintain technical terminology appropriately.
Also applies to: 104-110
locale/en/LC_MESSAGES/general-intro/introduction.po (2)
2-5
: LGTM: Metadata updates are consistent.The updates to copyright year, project version, and technical specifications are correctly reflected.
Also applies to: 10-12, 21-21
33-35
: Verify workshop content references.Ensure that the referenced sections 'Basic' and 'Advanced' exist in the documentation structure.
✅ Verification successful
References to 'Basic' and 'Advanced' sections are valid
Both referenced sections exist in the documentation structure:
- 'Basic' has a dedicated section with multiple chapters
- 'Advanced' content is included under "Interaction with other software" (chapter-12.rst)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the referenced sections exist fd -t f "index.rst|basic|advanced" docs/Length of output: 57
Script:
#!/bin/bash # Check index.rst content and search for workshop level files echo "=== Content of index.rst ===" cat docs/index.rst echo -e "\n=== Searching for workshop level files ===" fd -e rst . | grep -i "basic\|advanced" echo -e "\n=== Searching for RST section markers ===" rg -i "^[[:space:]]*basic[[:space:]]*$|^[[:space:]]*advanced[[:space:]]*$" -g "*.rst"Length of output: 2776
locale/en/LC_MESSAGES/basic/data.po (2)
88-92
: Verify OSM data snapshot date.The text references a snapshot from Sep 2024, but the current date is January 2025. Consider updating to the latest available snapshot.
158-159
: Update file path reference.The file path
~/Desktop/workshop/BELEM_BR.osm
should be verified to ensure it matches the actual path in the OSGeoLive distribution.✅ Verification successful
File path is correctly documented
The path
~/Desktop/workshop/BELEM_BR.osm
is consistent with the standard workshop directory structure used throughout the documentation. This location is explicitly created during workshop setup (mkdir ~/Desktop/workshop
) and is used consistently for all workshop data files.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the file exists in the expected location fd -t f "BELEM_BR.osm" ~/Desktop/workshop/Length of output: 175
Script:
#!/bin/bash # Search for any mentions of BELEM_BR.osm in the codebase rg -i "BELEM_BR.osm" # Find the source RST file fd -e rst "data.rst" # Look for any path patterns with Desktop/workshop rg -i "desktop/workshop"Length of output: 4201
locale/en/LC_MESSAGES/interactions/chapter-9.po (1)
Line range hint
69-74
: LGTM: Database connection parameters are correct.The database connection parameters are consistent with the workshop setup instructions.
locale/es/LC_MESSAGES/appendix/appendix-3.po (3)
12-25
: LGTM! Metadata updates are consistent with project changes.The updates to project version, dates, and tool versions align with the transition from Prizren 2.9 to Belém 3.0.
33-41
: LGTM! Translation improvements enhance clarity.The Spanish translation has been improved for better readability while maintaining the original meaning.
53-65
: LGTM! Consistent terminology in translations.The translation maintains consistent terminology for technical terms and improves the flow of the text.
locale/en/LC_MESSAGES/general-intro/overview.po (2)
2-10
: LGTM! Metadata updates reflect current project state.The copyright year and project version updates are consistent with the transition to Belém 3.0.
30-32
: LGTM! Improved clarity in content.The text has been revised for better clarity while maintaining accuracy.
locale/en/LC_MESSAGES/general-intro/osgeolive.po (1)
97-97
: Verify OSGeoLive version references.Multiple references to OSGeoLive version 17 need verification to ensure they match the actual release version.
Also applies to: 118-118, 138-138, 222-222
✅ Verification successful
All OSGeoLive version 17 references are consistent
All references to OSGeoLive version 17 in the translation files match the version defined in CMakeLists.txt (17.0). No inconsistencies found across English, Spanish, and Japanese translations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify OSGeoLive version references # Test: Search for version references to ensure consistency rg -i "osgeolive.*17|17.*osgeolive"Length of output: 3466
locale/es/LC_MESSAGES/basic/data.po (1)
104-111
: Verify data snapshot date and location.The reference to Belém data snapshot from September 2024 needs verification to ensure accuracy.
✅ Verification successful
Translation and data reference are accurate and consistent
The translation correctly reflects the source text, and the September 2024 snapshot date is consistently referenced across the workshop materials. This appears to be an intentional future date, as the workshop materials explicitly acknowledge that using different data versions may cause variations in results.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify data snapshot references # Test: Search for date and location references rg -i "belem.*2024|2024.*belem"Length of output: 33
Script:
#!/bin/bash # Search in .po and .pot files for Belém/Belem references fd -e po -e pot -x rg -i "bel[eé]m" {} # Search for date references fd -e po -e pot -x rg -i "sep.*2024|2024.*sep|september.*2024|2024.*september" {}Length of output: 10606
locale/en/LC_MESSAGES/advanced/chapter-12.po (2)
2-5
: LGTM! Metadata updates are consistent.The updates to copyright year, project name, version, and generation information are consistent with the migration from Prizren 2.9 to Belém 3.0.
Also applies to: 10-12, 21-21
241-243
: Documentation link improvement is valuable.The addition of a direct link to the pgr_dijkstra documentation enhances accessibility and user experience.
locale/en/LC_MESSAGES/un_sdg/sdg7-energy.po (2)
102-107
: Content improvement enhances clarity.The updated description of data preprocessing provides clearer context and better explains the importance of data quality checks.
122-122
: Exercise title and description are more precise.The exercise title and description have been updated to be more specific about the task (setting the search path).
Also applies to: 127-128
docs/conf.py (3)
124-126
: Documentation link improvement in navbar.The addition of a direct link to the pgRouting documentation in the navbar enhances navigation.
144-146
: Verify Bootstrap version compatibility.The change to Bootstrap version 2 needs verification for theme compatibility.
✅ Verification successful
Bootstrap version 2 configuration is correct and compatible ✅
The
sphinx_bootstrap_theme>=0.4
package explicitly supports Bootstrap 2, and the configuration follows the documented pattern.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the Bootstrap 2 theme files are present and properly referenced find . -type f -name "*.html" -exec grep -l "bootstrap.*\.css" {} \;Length of output: 73
Script:
#!/bin/bash # Check theme configuration and requirements echo "=== Checking conf.py for theme settings ===" rg -A 5 "html_theme" docs/conf.py echo -e "\n=== Checking for requirements files ===" fd "requirements.*\.(txt|in)" --type f --exec cat {} echo -e "\n=== Checking for Bootstrap-related Python packages ===" pip list | grep -i "bootstrap\|sphinx.*theme"Length of output: 992
32-33
: Verify sphinx_collapse extension compatibility.The addition of the sphinx_collapse extension needs verification for compatibility with the current Sphinx version.
✅ Verification successful
sphinx_collapse extension is compatible with project requirements ✅
The extension requires Sphinx >= 3, which is satisfied by the project's Sphinx>4.0.0 requirement. The extension is also properly listed in REQUIREMENTS.txt.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if sphinx_collapse is listed in requirements.txt and verify its version compatibility grep -i "sphinx-collapse" requirements.txt pip index versions sphinx-collapseLength of output: 326
Script:
#!/bin/bash # Look for requirements files in docs directory and check sphinx version specification in conf.py fd requirements.txt docs/ fd requirements docs/ cat docs/conf.py | grep -i "sphinx" pip show sphinx-collapseLength of output: 927
Script:
#!/bin/bash # Look for Python package management files and check sphinx_collapse metadata fd -e toml -e txt -e in "requirements|pyproject|setup.py|setup.cfg" . curl -s https://pypi.org/pypi/sphinx-collapse/json | jq -r '.info.requires_dist[], .info.requires_python'Length of output: 297
Script:
#!/bin/bash # Check Sphinx version in REQUIREMENTS.txt cat REQUIREMENTS.txt | grep -i "sphinx"Length of output: 108
locale/en/LC_MESSAGES/un_sdg/introduction.po (1)
44-44
: Content clarity improvements.The text updates improve clarity and remove unnecessary spaces, particularly in the gender equality section.
Also applies to: 198-198
locale/es/LC_MESSAGES/basic/pedestrian.po (4)
53-58
: Improved translation clarity for algorithm description.The translation has been enhanced to better explain that Dijkstra's algorithm requires only identifiers and weights, making it more understandable for Spanish readers.
120-127
: Updated location references for the workshop.The location references have been updated to point to the correct area in Belém:
https://www.openstreetmap.org/#map=14/-1.44228/-48.46069
415-445
: New exercises added for route combinations.Exercise 5 has been added to cover route combinations, enhancing the workshop content with more practical examples.
Line range hint
12-25
: Verify metadata consistency across localization files.The metadata has been updated to reflect the new workshop location and version:
- Project version: "Workshop FOSS4G Belém 3.0"
- POT creation date: "2024-11-10"
- PO revision date: "2024-12-16"
- Generator version: "Weblate 5.4.3"
- Babel version: "2.16.0"
✅ Verification successful
Metadata consistency verified across localization files
The project version "Workshop FOSS4G Belém 3.0" is consistently used across all files. Variations in timestamps and tool versions are expected and reflect normal translation workflow patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistency in metadata across all locale files # Test: All locale files should have the same metadata values echo "Checking metadata consistency across locale files..." for po_file in $(find locale -name "*.po"); do echo "=== $po_file ===" grep -A 4 "Project-Id-Version:" "$po_file" grep "POT-Creation-Date:" "$po_file" grep "X-Generator:" "$po_file" grep "Generated-By:" "$po_file" doneLength of output: 44947
locale/en/LC_MESSAGES/basic/plpgsql_function.po (2)
Line range hint
2-21
: Update copyright and version information.The metadata has been updated with:
- New copyright year: 2024
- New project version: "Workshop FOSS4G Belém 3.0"
- Updated POT creation date
580-582
: Function name change requires codebase verification.The function name has been changed from
wrk_NearestOSM
towrk_nearest
. This change needs to be verified across the codebase to ensure all references are updated.locale/en/LC_MESSAGES/basic/graph_views.po (1)
1-662
: New localization file for graph views documentation.The new file provides comprehensive English translations for graph views documentation, including exercises and examples. The content is well-structured and complete.
locale/en/LC_MESSAGES/un_sdg/sdg3-health.po (3)
Line range hint
2-21
: Updated metadata for SDG3 health documentation.The metadata has been updated to reflect the new workshop version and dates.
116-194
: Improved structure with PostgreSQL basics section.The content has been restructured to include a PostgreSQL basics section, making it more accessible for beginners.
473-763
: Enhanced exercise flow and clarity.The exercises have been reorganized and clarified, with better progression from basic to advanced concepts. Exercise numbering has been updated to maintain consistency.
locale/es/LC_MESSAGES/basic/graph_views.po (1)
1-732
: Well-structured and accurate Spanish translations!The translations effectively convey technical concepts while maintaining natural Spanish language flow. Technical terms like "graph", "vertices", and "edges" are consistently translated, and exercise instructions are clear.
REQUIREMENTS.txt (1)
3-4
: Verify sphinx-collapse compatibility.While adding sphinx-collapse is good for enhanced documentation features, we should ensure we're using a compatible version.
✅ Verification successful
sphinx-collapse dependency is compatible ✓
sphinx-collapse 0.1.3 has permissive requirements (Sphinx >= 3) and is actively maintained.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check PyPI for latest versions and compatibility curl -s https://pypi.org/pypi/sphinx-collapse/json | jq -r '.info | {version: .version, requires_python: .requires_python}' # Check if there are any known issues with Sphinx>4.0.0 curl -s https://pypi.org/pypi/sphinx-collapse/json | jq -r '.releases[] | select(.requires_dist != null) | select(.requires_dist[] | contains("sphinx"))'Length of output: 409
Script:
#!/bin/bash # Get sphinx-collapse dependencies with correct JSON path curl -s https://pypi.org/pypi/sphinx-collapse/json | jq -r '.info.requires_dist[]' # Check project's Python version requirement cat *requirements*.txt | grep -i "python" cat setup.py 2>/dev/null | grep -i "python_requires" cat pyproject.toml 2>/dev/null | grep -i "python"Length of output: 353
docs/un_sdg/images/sdg11/CMakeLists.txt (1)
8-8
: LGTM! New image file added correctly.The addition of
remove_waterways.png
follows the existing pattern and will be properly handled by the build system.docs/basic/images/chapter6/CMakeLists.txt (1)
9-10
: LGTM! Improved file naming convention.The new image files use a more consistent naming convention with underscores, which is better for maintainability.
docs/basic/images/chapter7/CMakeLists.txt (1)
13-15
: LGTM! More descriptive image names.The new image names (
vehicle_net.png
,walk_net.png
,taxi_net.png
) are more descriptive and follow a consistent naming pattern.Please verify that these new image files exist in the directory and that they contain the correct content replacing
ch7-e1.png
andch7-e2.png
.✅ Verification successful
All new image files are present and valid
The three new PNG files exist in the correct directory and have appropriate file sizes indicating they contain valid image content.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence and size of new image files for img in vehicle_net.png walk_net.png taxi_net.png; do if [ -f "docs/basic/images/chapter7/$img" ]; then ls -l "docs/basic/images/chapter7/$img" else echo "Missing: $img" fi doneLength of output: 749
docs/basic/images/chapter5/CMakeLists.txt (1)
6-10
: LGTM! Consistent naming convention.The changes improve consistency by:
- Using underscores instead of hyphens in filenames
- Adding a new
pedestrian_combinations.png
for enhanced documentationPlease verify the existence and content of the updated image files:
✅ Verification successful
All image files verified successfully
All referenced image files exist in the correct location with appropriate sizes and permissions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence and size of updated image files for img in pedestrian_combinations.png pedestrian_route{1,2,4,5}.png; do if [ -f "docs/basic/images/chapter5/$img" ]; then ls -l "docs/basic/images/chapter5/$img" else echo "Missing: $img" fi doneLength of output: 1467
docs/scripts/get_data/get_all_data.sh (1)
6-7
: LGTM! Updated to latest OSGeo Live DVD version.The URL has been updated to version 17 of the OSGeo Live DVD data.
docs/basic/CMakeLists.txt (1)
12-12
: LGTM! Consider documenting file order dependency.The addition of
graph_views.rst
and removal ofappendix.rst
align with the documentation restructuring.Since the comment states "Files order is important", please document why the order matters:
✅ Verification successful
Document the pedagogical order of RST files
The file order in
CMakeLists.txt
follows the tutorial's chapter progression (4 through 8), making it important for the learning experience. While there are no technical dependencies between files, consider adding a comment inCMakeLists.txt
explaining that the order reflects the tutorial's pedagogical structure:# Files are ordered by tutorial progression: # Ch4: Data setup # Ch5-6: Basic concepts (pedestrian, vehicle) # Ch7-8: Advanced concepts (graphs, functions)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any explicit dependencies between RST files for file in data.rst pedestrian.rst vehicle.rst graph_views.rst sql_function.rst plpgsql_function.rst; do echo "Checking references in $file:" if [ -f "docs/basic/$file" ]; then grep -n "include::" "docs/basic/$file" || echo "No includes found" grep -n ".. _.*:" "docs/basic/$file" || echo "No labels found" else echo "File not found: $file" fi doneLength of output: 12898
Script:
#!/bin/bash # Check for toctree directives and documentation structure echo "Checking index.rst for toctree:" if [ -f "docs/basic/index.rst" ]; then grep -A 5 "toctree::" "docs/basic/index.rst" || echo "No toctree found" fi echo -e "\nChecking conf.py for documentation structure:" if [ -f "docs/conf.py" ]; then grep -A 5 "toctree_" "docs/conf.py" || echo "No toctree settings found" fiLength of output: 334
docs/scripts/basic/chapter_6/CMakeLists.txt (1)
22-28
: LGTM! New byproduct files properly declared.The additions to the BYPRODUCTS list follow the existing naming convention and are properly formatted.
docs/scripts/basic/chapter_7/images.sql (1)
1-2
: LGTM! Improved script idempotency.Good addition of IF EXISTS clauses to prevent errors when objects don't exist.
docs/index.rst (1)
36-36
: LGTM! Documentation structure updated.The new entries are properly placed in their respective sections. Please verify that the referenced RST files exist and are properly formatted.
Also applies to: 63-63
✅ Verification successful
RST files verified successfully ✓
Both new documentation files exist and follow proper RST formatting conventions with appropriate section markers.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new RST files exist and are properly formatted echo "Checking for new RST files..." for file in "docs/basic/graph_views.rst" "docs/advanced/chapter-12.rst"; do if [ -f "$file" ]; then echo "Found $file" # Check for common RST formatting issues grep -n "^=\{3,\}$\|^-\{3,\}$\|^#\{3,\}$" "$file" || echo "No heading markers found in $file" else echo "Missing file: $file" fi doneLength of output: 2104
.github/workflows/ubuntu.yml (1)
14-14
: LGTM! Action versions updated.The updates to GitHub Actions versions look good:
Also applies to: 31-31
docs/scripts/basic/chapter_5/images.sql (2)
18-18
: Parameter standardization looks good.The change from @ID_5@ to @ID_1@ as the source vertex makes the parameter usage more consistent across views.
Also applies to: 28-28
58-58
: Improved column selection in pedestrian_route5.The explicit column selection with rounding is better than SELECT * as it:
- Makes the output more predictable
- Improves readability
- Controls decimal precision
docs/scripts/un_sdg/sdg7/all_exercises_sdg7.sql (2)
72-74
: Good use of PREPARE statement.The PREPARE statement helps with:
- Query plan caching
- Code reusability
- Performance optimization
1-4
: 🛠️ Refactor suggestionConsider using transaction blocks.
For data consistency, wrap related operations in transactions.
+BEGIN; \o set_path.txt SET search_path TO roads,public,contrib,postgis; \o show_path2.txt SHOW search_path; +COMMIT;Likely invalid or redundant comment.
docs/CMakeLists.txt (2)
70-70
: LGTM! Improved locale generation flexibility.The change from hardcoded 'en' to ${SPHINXINTL_LANGUAGE} enhances the flexibility of locale generation, allowing for better language handling.
89-92
: Verify the impact of disabling warnings-as-errors.The change consistently disables warnings-as-errors for all languages. While this might make the build more permissive, it could potentially hide documentation issues.
Consider if some critical warnings should still be treated as errors to maintain documentation quality.
docs/scripts/basic/chapter_6/images.sql (1)
29-29
: LGTM! Enhanced penalty configuration for pedestrian routing.The addition of 'cycleway' to the penalty configuration aligns with the existing treatment of pedestrian ways.
docs/scripts/basic/chapter_6/all_exercises.sql (1)
5-5
: LGTM! Enhanced SQL readability with improved comments.The added comments and consistent formatting improve code readability and maintainability.
Also applies to: 12-12, 19-24, 37-38, 52-53, 86-89, 117-122
.github/workflows/website.yml (1)
1-1
: LGTM! Improved workflow organization and dependency versions.The changes effectively:
- Separate website updates from locale handling
- Upgrade to latest action versions
- Simplify the build configuration
Also applies to: 12-12, 47-47, 75-75
docs/general-intro/introduction.rst (1)
64-67
: LGTM! New sponsor logo added.The Erosion logo has been properly added with appropriate width and target URL.
docs/scripts/basic/chapter_8/all-sections-8.sql (2)
15-41
: LGTM! Improved vertex management.The use of
pgr_extractVertices
and subsequent updates provides a more robust way to handle network vertices.
Line range hint
71-92
: Verify the function signature change impact.The
wrk_Nearest
function has been updated to use REGCLASS parameter. Ensure all calling code has been updated accordingly.✅ Verification successful
Function signature change is compatible with all existing calls ✅
All calls to
wrk_Nearest
pass string literals which PostgreSQL automatically casts to REGCLASS type, making them compatible with the updated function signature. No changes to the calling code are required.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all calls to wrk_Nearest to verify parameter compatibility rg -A 2 "wrk_Nearest\(" --type sqlLength of output: 1547
docs/general-intro/overview.rst (1)
19-20
: LGTM! Improved clarity in documentation.The text has been restructured to better explain the relationship between FOSS geospatial tools.
docs/scripts/basic/chapter_5/all_exercises_5.sql (2)
82-92
: LGTM! Improved query formatting.The SQL query formatting has been improved while maintaining the original functionality.
65-78
: Verify the function replacement impact.The change from
pgr_dijkstraCost
topgr_dijkstra
may affect the output format. Ensure all dependent code handles the new output structure correctly.✅ Verification successful
Function replacement is safe and intentional
The change from
pgr_dijkstraCost
topgr_dijkstra
in the example query is intentional, providing more detailed path information while maintaining the same routing logic. Both functions continue to coexist in the file for different educational purposes.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for code that processes the output of these functions rg -A 5 "pgr_dijkstra|pgr_dijkstraCost" --type sqlLength of output: 32479
docs/basic/data.rst (2)
30-35
: LGTM! Improved installation instructions.The installation steps have been reorganized for better clarity and user experience.
136-145
: LGTM! Added cleanup section.New cleanup section with appropriate script references improves the completeness of the documentation.
docs/advanced/chapter-12.rst (1)
166-168
: Documentation improvement: Better linking and flexibility.The changes improve the documentation by:
- Using a direct link to the pgr_dijkstra documentation
- Adding flexibility by mentioning other pgRouting queries
CMakeLists.txt (3)
Line range hint
224-294
: Review language mapping completeness.The language mapping appears comprehensive but should be verified against the locale files.
✅ Verification successful
Language mapping is correctly implemented and verified ✓
The locale directory structure confirms that all supported languages (es, ja) have corresponding locale directories with proper gettext structure. While Japanese translations are partial, the language mapping itself is correctly implemented.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if all supported languages have corresponding locale files for lang in es ja; do if [ ! -d "locale/$lang" ]; then echo "Warning: Missing locale directory for $lang" fi doneLength of output: 93
Script:
#!/bin/bash # First, find the locale directory echo "=== Searching for locale directory ===" fd -t d "^locale$" echo -e "\n=== Listing contents of locale directory if found ===" for dir in $(fd -t d "^locale$"); do echo "Contents of $dir:" ls -la "$dir" echo -e "\nSubdirectories with .po files:" find "$dir" -name "*.po" -type f doneLength of output: 3535
26-29
: Verify version numbering scheme.Please verify that the version increment from 2.9 to 3.0 follows the project's semantic versioning guidelines, especially since this appears to be a significant version jump.
71-86
: Validate coordinate boundaries.The coordinate values for Belém need verification to ensure they cover the intended area correctly.
✅ Verification successful
All coordinates are within valid Belém boundaries
All coordinate pairs in the file fall within the expected ranges for Belém (latitude: -1.49 to -1.36, longitude: -48.52 to -48.4), representing valid locations within the city limits.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify coordinates are within valid ranges for Belém echo "Validating coordinates..." LAT="-1.422066" LON="-48.45665" python3 << EOF lat, lon = float("$LAT"), float("$LON") if not (-1.49 <= lat <= -1.36 and -48.52 <= lon <= -48.4): print(f"Warning: Coordinates ({lat}, {lon}) may be outside Belém's expected range") EOFLength of output: 102
Script:
#!/bin/bash echo "Validating all coordinates..." python3 << EOF # Define all coordinates coords = [ ("-1.422066", "-48.45665", "Main"), ("-1.455829", "-48.446044", "POINT1"), ("-1.453448", "-48.447142", "POINT2") ] # Validate each coordinate pair for lat_str, lon_str, name in coords: lat, lon = float(lat_str), float(lon_str) if not (-1.49 <= lat <= -1.36 and -48.52 <= lon <= -48.4): print(f"Warning: {name} coordinates ({lat}, {lon}) may be outside Belém's expected range") # Known landmarks in Belém for reference landmarks = { "Hangar Convention Center": (-1.4375, -48.4778), "IFPA Campus Belém": (-1.4359, -48.4556), "Forte do Castelo": (-1.4561, -48.5047), "Estação das Docas": (-1.4497, -48.4997) } print("\nLandmark proximity check:") for coord_lat_str, coord_lon_str, name in coords: coord_lat, coord_lon = float(coord_lat_str), float(coord_lon_str) for landmark_name, (lm_lat, lm_lon) in landmarks.items(): # Calculate rough distance (simplified) dist = ((coord_lat - lm_lat) ** 2 + (coord_lon - lm_lon) ** 2) ** 0.5 if dist < 0.01: # roughly 1km print(f"{name} coordinates are close to {landmark_name}") EOFLength of output: 105
docs/basic/vehicle.rst (2)
36-36
: LGTM! Correct RST formatting for CO2 subscript.The chemical formula CO2 is properly formatted using RST subscript syntax:
CO\ :sub:
2\ emissions
.
Line range hint
1-377
: Well-structured documentation with clear progression.The document maintains excellent pedagogical flow, with each exercise building upon previous concepts and clear connections between sections.
docs/basic/pedestrian.rst (2)
44-44
: Improved function signature clarity.The updated return signature of
pgr_dijkstra
now correctly showsstart_vid
andend_vid
as mandatory return values, which better reflects their importance in route identification.
244-273
: Well-structured new exercise on route combinations.The new Exercise 5 effectively demonstrates how to handle multiple independent routes simultaneously, with clear visualization and SQL implementation.
docs/basic/plpgsql_function.rst (1)
197-197
: Improved function naming.The change from
wrk_NearestOSM
towrk_nearest
follows better naming conventions by being more concise while maintaining clarity of purpose.docs/un_sdg/sdg11-cities.rst (1)
Line range hint
71-397
: Well-structured exercises with clear progression.The exercises are logically organized, with each step building upon previous ones and clear rubrics guiding the user through the process.
docs/basic/sql_function.rst (1)
49-70
: Clear and well-structured parameter documentation.The parameter documentation uses clear table formatting and provides comprehensive descriptions for both input parameters and output columns.
|
||
### Building PDF | ||
|
||
Install prerequisite: | ||
```bash | ||
sudo apt-get install texlive-latex-extra |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove or update empty PDF section.
The "Building PDF" section contains installation prerequisites but no build instructions, which could confuse users.
Either remove the section entirely or add the necessary build instructions.
pushd "${DIR}" > /dev/null || exit 1 | ||
|
||
mkdir -p build | ||
pushd build > /dev/null || exit 1 | ||
cmake -DLOCALE=ON .. | ||
|
||
make locale | ||
popd > /dev/null || exit 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for CMake configuration and build steps.
The script should check the return status of critical operations.
pushd "${DIR}" > /dev/null || exit 1
mkdir -p build
pushd build > /dev/null || exit 1
-cmake -DLOCALE=ON ..
+cmake -DLOCALE=ON .. || { echo "CMake configuration failed"; exit 1; }
-make locale
+make locale || { echo "make locale failed"; exit 1; }
popd > /dev/null || exit 1
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
pushd "${DIR}" > /dev/null || exit 1 | |
mkdir -p build | |
pushd build > /dev/null || exit 1 | |
cmake -DLOCALE=ON .. | |
make locale | |
popd > /dev/null || exit 1 | |
pushd "${DIR}" > /dev/null || exit 1 | |
mkdir -p build | |
pushd build > /dev/null || exit 1 | |
cmake -DLOCALE=ON .. || { echo "CMake configuration failed"; exit 1; } | |
make locale || { echo "make locale failed"; exit 1; } | |
popd > /dev/null || exit 1 |
run: | | ||
sudo service postgresql start | ||
pgver=$(psql --version | grep -Po '(?<=psql \(PostgreSQL\) )[^;]+(?=\.\d+ \()') | ||
echo "PGVER=${pgver}" >> $GITHUB_ENV | ||
echo "PGIS=3" >> $GITHUB_ENV |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix shell script quoting issues.
The script has potential word splitting issues due to unquoted variables.
Apply these fixes:
- pgver=$(psql --version | grep -Po '(?<=psql \(PostgreSQL\) )[^;]+(?=\.\d+ \()')
- echo "PGVER=${pgver}" >> $GITHUB_ENV
+ pgver="$(psql --version | grep -Po '(?<=psql \(PostgreSQL\) )[^;]+(?=\.\d+ \()')"
+ echo "PGVER=${pgver}" >> "$GITHUB_ENV"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
run: | | |
sudo service postgresql start | |
pgver=$(psql --version | grep -Po '(?<=psql \(PostgreSQL\) )[^;]+(?=\.\d+ \()') | |
echo "PGVER=${pgver}" >> $GITHUB_ENV | |
echo "PGIS=3" >> $GITHUB_ENV | |
run: | | |
sudo service postgresql start | |
pgver="$(psql --version | grep -Po '(?<=psql \(PostgreSQL\) )[^;]+(?=\.\d+ \()')" | |
echo "PGVER=${pgver}" >> "$GITHUB_ENV" | |
echo "PGIS=3" >> $GITHUB_ENV |
🧰 Tools
🪛 actionlint (1.7.4)
30-30: shellcheck reported issue in this script: SC2086:info:3:26: Double quote to prevent globbing and word splitting
(shellcheck)
30-30: shellcheck reported issue in this script: SC2086:info:4:18: Double quote to prevent globbing and word splitting
(shellcheck)
sudo -u postgres createdb -p ${POSTGRES_PORT} setup | ||
sudo -u postgres psql -c 'CREATE ROLE "runner" SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN PASSWORD $$runner$$;' -d setup | ||
echo :5432:*:runner:runner >> .pgpass | ||
sudo -u postgres psql -c 'CREATE ROLE "user" SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN PASSWORD $$user$$;' -d setup | ||
echo :5432:*:user:user >> .pgpass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance PostgreSQL security configuration.
The PostgreSQL setup uses hardcoded passwords and superuser privileges, which is not recommended for security.
Consider:
- Using environment variables for passwords
- Limiting user privileges to only what's necessary
- Using connection pooling for better security
Example:
- sudo -u postgres psql -c 'CREATE ROLE "runner" SUPERUSER CREATEDB CREATEROLE INHERIT LOGIN PASSWORD $$runner$$;' -d setup
+ sudo -u postgres psql -c "CREATE ROLE \"runner\" LOGIN PASSWORD '${RUNNER_PASSWORD}' INHERIT;" -d setup
+ sudo -u postgres psql -c "GRANT CONNECT ON DATABASE setup TO runner;" -d setup
Committable suggestion skipped: line range outside the PR's diff.
\o clean_buildings.txt | ||
ALTER TABLE buildings.buildings_ways | ||
DROP source, DROP target, | ||
DROP source_osm, DROP target_osm, | ||
DROP length, DROP length_m, | ||
DROP cost, DROP reverse_cost, | ||
DROP cost_s, DROP reverse_cost_s, | ||
DROP one_way, DROP oneway, | ||
DROP priority, DROP osm_id, DROP rule, | ||
DROP x1, DROP x2, | ||
DROP y1, DROP y2, | ||
DROP maxspeed_forward, | ||
DROP maxspeed_backward; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider using transaction for table cleanup.
The ALTER TABLE operations should be wrapped in a transaction for atomicity.
Add transaction handling:
+BEGIN;
ALTER TABLE buildings.buildings_ways
DROP source, DROP target,
DROP source_osm, DROP target_osm,
DROP length, DROP length_m,
DROP cost, DROP reverse_cost,
DROP cost_s, DROP reverse_cost_s,
DROP one_way, DROP oneway,
DROP priority, DROP osm_id, DROP rule,
DROP x1, DROP x2,
DROP y1, DROP y2,
DROP maxspeed_forward,
DROP maxspeed_backward;
+COMMIT;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
\o clean_buildings.txt | |
ALTER TABLE buildings.buildings_ways | |
DROP source, DROP target, | |
DROP source_osm, DROP target_osm, | |
DROP length, DROP length_m, | |
DROP cost, DROP reverse_cost, | |
DROP cost_s, DROP reverse_cost_s, | |
DROP one_way, DROP oneway, | |
DROP priority, DROP osm_id, DROP rule, | |
DROP x1, DROP x2, | |
DROP y1, DROP y2, | |
DROP maxspeed_forward, | |
DROP maxspeed_backward; | |
\o clean_buildings.txt | |
BEGIN; | |
ALTER TABLE buildings.buildings_ways | |
DROP source, DROP target, | |
DROP source_osm, DROP target_osm, | |
DROP length, DROP length_m, | |
DROP cost, DROP reverse_cost, | |
DROP cost_s, DROP reverse_cost_s, | |
DROP one_way, DROP oneway, | |
DROP priority, DROP osm_id, DROP rule, | |
DROP x1, DROP x2, | |
DROP y1, DROP y2, | |
DROP maxspeed_forward, | |
DROP maxspeed_backward; | |
COMMIT; |
\o show_schemas.txt | ||
\dn | ||
|
||
-- Show the current search path | ||
\o show_path1.txt | ||
SHOW search_path; | ||
|
||
-- Set the search path | ||
SET search_path TO roads,buildings,public; | ||
\o set_path.txt | ||
SET search_path TO roads,buildings,public,contrib,postgis; | ||
\o show_path2.txt | ||
SHOW search_path; | ||
|
||
|
||
-- Enumerate all the tables | ||
\o enumerate_tables.txt | ||
\dt | ||
|
||
|
||
\o exercise_5.txt | ||
-- Counting the number of Edges of roads | ||
\o count1.txt | ||
SELECT COUNT(*) FROM roads_ways; | ||
|
||
-- Counting the number of Vertices of roads | ||
SELECT COUNT(*) FROM roads_ways_vertices_pgr; | ||
|
||
-- Counting the number of buildings | ||
\o count2.txt | ||
SELECT COUNT(*) FROM buildings_ways; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Schema management needs error handling.
The script should handle potential errors when setting the search path and performing initial setup.
Add error handling:
-SET search_path TO roads,buildings,public,contrib,postgis;
+DO $$
+BEGIN
+ PERFORM set_config('search_path', 'roads,buildings,public,contrib,postgis', false);
+EXCEPTION WHEN OTHERS THEN
+ RAISE NOTICE 'Error setting search_path: %', SQLERRM;
+ RAISE;
+END $$;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
\o show_schemas.txt | |
\dn | |
-- Show the current search path | |
\o show_path1.txt | |
SHOW search_path; | |
-- Set the search path | |
SET search_path TO roads,buildings,public; | |
\o set_path.txt | |
SET search_path TO roads,buildings,public,contrib,postgis; | |
\o show_path2.txt | |
SHOW search_path; | |
-- Enumerate all the tables | |
\o enumerate_tables.txt | |
\dt | |
\o exercise_5.txt | |
-- Counting the number of Edges of roads | |
\o count1.txt | |
SELECT COUNT(*) FROM roads_ways; | |
-- Counting the number of Vertices of roads | |
SELECT COUNT(*) FROM roads_ways_vertices_pgr; | |
-- Counting the number of buildings | |
\o count2.txt | |
SELECT COUNT(*) FROM buildings_ways; | |
\o show_schemas.txt | |
\dn | |
\o show_path1.txt | |
SHOW search_path; | |
\o set_path.txt | |
DO $$ | |
BEGIN | |
PERFORM set_config('search_path', 'roads,buildings,public,contrib,postgis', false); | |
EXCEPTION WHEN OTHERS THEN | |
RAISE NOTICE 'Error setting search_path: %', SQLERRM; | |
RAISE; | |
END $$; | |
\o show_path2.txt | |
SHOW search_path; | |
\o enumerate_tables.txt | |
\dt | |
\o count1.txt | |
SELECT COUNT(*) FROM roads_ways; | |
\o count2.txt | |
SELECT COUNT(*) FROM buildings_ways; |
CREATE VIEW vehicle_net AS | ||
SELECT | ||
gid AS id, | ||
source_osm AS source, target_osm AS target, | ||
cost_s AS cost, reverse_cost_s AS reverse_cost, | ||
name, length_m, the_geom | ||
FROM ways JOIN configuration AS c | ||
USING (tag_id) | ||
WHERE c.tag_value NOT IN ('steps','footway','path'); | ||
|
||
-- Verification1 | ||
|
||
WITH | ||
all_components AS (SELECT component, count(*) FROM ways GROUP BY component), -- line 6 | ||
max_component AS (SELECT max(count) from all_components), | ||
the_component AS ( | ||
SELECT component FROM all_components | ||
WHERE count = (SELECT max FROM max_component)) | ||
|
||
SELECT | ||
gid AS id, | ||
source_osm AS source, target_osm AS target, -- line 14 | ||
cost_s AS cost, reverse_cost_s AS reverse_cost, | ||
name, length_m AS length, the_geom AS geom | ||
FROM ways JOIN the_component USING (component) JOIN configuration USING (tag_id) | ||
WHERE tag_value NOT IN ('steps','footway','path','cycleway'); -- line 18 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Optimize vehicle network view.
The vehicle network view could be more efficient with materialization and proper indexing.
Convert to materialized view with indices:
-CREATE VIEW vehicle_net AS
+CREATE MATERIALIZED VIEW vehicle_net AS
WITH
all_components AS (
SELECT component, count(*) FROM ways GROUP BY component
),
max_component AS (
SELECT max(count) from all_components
),
the_component AS (
SELECT component FROM all_components
WHERE count = (SELECT max FROM max_component)
)
SELECT
gid AS id,
source_osm AS source, target_osm AS target,
cost_s AS cost, reverse_cost_s AS reverse_cost,
name, length_m AS length, the_geom AS geom
FROM ways
JOIN the_component USING (component)
JOIN configuration USING (tag_id)
WHERE tag_value NOT IN ('steps','footway','path','cycleway');
+CREATE INDEX idx_vehicle_net_id ON vehicle_net(id);
+CREATE INDEX idx_vehicle_net_source ON vehicle_net(source);
+CREATE INDEX idx_vehicle_net_target ON vehicle_net(target);
+CREATE INDEX idx_vehicle_net_geom ON vehicle_net USING GIST(geom);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
CREATE VIEW vehicle_net AS | |
SELECT | |
gid AS id, | |
source_osm AS source, target_osm AS target, | |
cost_s AS cost, reverse_cost_s AS reverse_cost, | |
name, length_m, the_geom | |
FROM ways JOIN configuration AS c | |
USING (tag_id) | |
WHERE c.tag_value NOT IN ('steps','footway','path'); | |
-- Verification1 | |
WITH | |
all_components AS (SELECT component, count(*) FROM ways GROUP BY component), -- line 6 | |
max_component AS (SELECT max(count) from all_components), | |
the_component AS ( | |
SELECT component FROM all_components | |
WHERE count = (SELECT max FROM max_component)) | |
SELECT | |
gid AS id, | |
source_osm AS source, target_osm AS target, -- line 14 | |
cost_s AS cost, reverse_cost_s AS reverse_cost, | |
name, length_m AS length, the_geom AS geom | |
FROM ways JOIN the_component USING (component) JOIN configuration USING (tag_id) | |
WHERE tag_value NOT IN ('steps','footway','path','cycleway'); -- line 18 | |
CREATE MATERIALIZED VIEW vehicle_net AS | |
WITH | |
all_components AS (SELECT component, count(*) FROM ways GROUP BY component), | |
max_component AS (SELECT max(count) from all_components), | |
the_component AS ( | |
SELECT component FROM all_components | |
WHERE count = (SELECT max FROM max_component)) | |
SELECT | |
gid AS id, | |
source_osm AS source, target_osm AS target, | |
cost_s AS cost, reverse_cost_s AS reverse_cost, | |
name, length_m AS length, the_geom AS geom | |
FROM ways JOIN the_component USING (component) JOIN configuration USING (tag_id) | |
WHERE tag_value NOT IN ('steps','footway','path','cycleway'); | |
CREATE INDEX idx_vehicle_net_id ON vehicle_net(id); | |
CREATE INDEX idx_vehicle_net_source ON vehicle_net(source); | |
CREATE INDEX idx_vehicle_net_target ON vehicle_net(target); | |
CREATE INDEX idx_vehicle_net_geom ON vehicle_net USING GIST(geom); |
Description of the function can be found in `pgr_extractVertices | ||
<https://docs.pgrouting.org/latest/en/pgr_connectedComponents.html>`__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix incorrect documentation link.
The link to pgr_extractVertices
documentation points to pgr_connectedComponents
instead.
-<https://docs.pgrouting.org/latest/en/pgr_connectedComponents.html>`__
+<https://docs.pgrouting.org/latest/en/pgr_extractVertices.html>`__
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Description of the function can be found in `pgr_extractVertices | |
<https://docs.pgrouting.org/latest/en/pgr_connectedComponents.html>`__ | |
Description of the function can be found in `pgr_extractVertices | |
<https://docs.pgrouting.org/latest/en/pgr_extractVertices.html>`__ |
pgr_drivingDistance(edges_sql, start_vids, distance [, directed] [, equicost]) | ||
RETURNS SET OF (seq, [start_vid,] node, edge, cost, agg_cost) | ||
|
||
`pgr_drivingDistance Documentation <https://docs.pgrouting.org/3.1/en/pgr_drivingDistance>`__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update documentation link version.
The link to pgRouting documentation points to version 3.1, but should be updated to match the workshop version 3.0.
-`pgr_drivingDistance Documentation <https://docs.pgrouting.org/3.1/en/pgr_drivingDistance>`__
+`pgr_drivingDistance Documentation <https://docs.pgrouting.org/3.0/en/pgr_drivingDistance>`__
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
`pgr_drivingDistance Documentation <https://docs.pgrouting.org/3.1/en/pgr_drivingDistance>`__ | |
`pgr_drivingDistance Documentation <https://docs.pgrouting.org/3.0/en/pgr_drivingDistance>`__ |
Preparing the release of the next workshop
@pgRouting/admins
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the high-level release notes for end-users:
New Features
Documentation Updates
Technical Improvements
Performance Enhancements
Compatibility