-
Notifications
You must be signed in to change notification settings - Fork 452
docs: avoid_borders build #2151
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
base: main
Are you sure you want to change the base?
Conversation
|
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.
Awesome job in digesting the information buried in the forum threads into a concise list of recommendations 💪
From a user's perspective I find a few points might be a bit confusing or unclear, please see my comments below 🙈
Also, please don't forget to update the CHANGELOG 😉
Cheers! 🚀
|
||
- **Performance considerations:** | ||
Large or highly detailed polygons can make graph creation slow or stall. | ||
For extensive regions, borders should be simplified or subdivided into smaller polygons. |
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.
I think it would be really helpful to provide a few more details on how to split the country polygons. @rabidllama did a great job describing the process in the forum, and there is a page on confluence too. How about compiling a short description based on these sources, along with some pictures? 🙏
|
||
- **Field names and encoding:** | ||
`borders.geojson` must include a `name` property. | ||
In `ids.csv`, the `name` column must use consistent Latin/ASCII names. |
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.
I don't think this is actually required. For example, this is how an example entry for one of the countries looks like across the files in production.
ids_iso.csv
"country_id","name","name:en","iso_cca2","iso_cca3"
...
"198","ประเทศไทย","Thailand","TH","THA"
...
boundaries.geojson
...
{ "type": "Feature", "properties": { "name": "ประเทศไทย", "hierarchy": 249 }, "geometry": { "type": "MultiPolygon", "coordinates": [ [ [ [ 98.99999, 16.875182 ], [ 99.99999, 16.875182 ], [ 99.99999, 14.875182 ], [ 98.99999, 14.875182 ], [ 98.99999, 15.875182 ], [ 98.99999, 16.875182 ] ] ] ] } },
...
- **Field names and encoding:** | ||
`borders.geojson` must include a `name` property. | ||
In `ids.csv`, the `name` column must use consistent Latin/ASCII names. | ||
Names must match across `borders.geojson`, `ids.csv`, and `openborders.csv`. |
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.
Apparently countries in openborders.csv are identified by their English names, as suggested by the following entries.
ids_iso.csv
...
"117","Lëtzebuerg","Luxembourg","LU","LUX"
...
openborders.csv
...
"Luxembourg","Austria"
"Luxembourg","Belgium"
"Luxembourg","Czech Republic
...
Pull Request Checklist
have been resolved.
[Unreleased] heading.
along with a short description of what it is for, and documented this in the Pull Request (below).
(at least Germany), and the graphs build without problems (i.e. no out-of-memory errors).
importer etc.), I have generated longer distance routes for the affected profiles with different options
(avoid features, max weight etc.) and compared these with the routes of the same parameters and start/end
points generated from the current live ORS.
If there are differences then the reasoning for these MUST be documented in the pull request.
and why the change was needed.
Fixes #2133 .
Information about the changes
avoid_borders
for bulding graphs.Examples and reasons for differences between live ORS routes, and those generated from this pull request
Required changes to ors config (if applicable)