This PR fixes a bug in the `waterway` layer in which empty `name_en` and `name_de` tags were being added to the tiles.
`name_en` and `name_de` are legacy tags, intended to be removed in v4.0. Thus, any legacy consumers that have not yet transitioned to `name:en` and `name:de` etc are currently NOT rendering labels for these features when `name:en` and `name:de` are not tagged on the feature. Therefore there are two possible approaches:
1. Remove these tags which are not rendering anyways from the tile, which keeps the bug in place, but shouldn't matter because no data consumer has taken notice of the issue (it would probably be easier to migrate to the new tag than to post a bug report!), _and they're going to be removed eventually anyways_. That's the approach in this PR.
This PR removes cases of `oneway=0` as this is an assumed default. It is not necessary to have this tag on every road or path and by removing it we can save space in the tiles by only including the meaningful values. It leaves intact `oneway=1` and `oneway=-1`, the latter of which indicates a oneway in the opposite direction of the way.
Unblocks #1265
This PR removes unnecessary `ramp` tagging from the `transportation` layer. The `ramp` tag is used for two situations. First, it is set for all `highway=***_link` tags to indicate a highway ramp. The second situation is when the `ramp` tag is set on a highway object, which indicates the presence of a ramp, such as one that would be used for a wheelchair. `ramp=0` is a reasonable default in these situations; by removing cases of `ramp=0` and presenting only `ramp=1` to the tiles, we can save considerable size in the tiles. Additionally, there appears to be a bug in which all objects tagged `highway=steps` are inexplicably tagged `ramp=1`.
The changes as follows:
1. Remove `ramp=0` where it appears in the tile.
2. `highway=steps` no longer assumes `ramp=1` in cases where `ramp=*` is not explicitly set.
In the current behavior, `ramp=0` is tagged only in at z13+ but suppressed in lower zooms:
This PR unifies `ramp=0` suppression between z13+ and z12-.
This PR adds the ability to create SQL tests that ensure that OSM data is properly imported and updated in the OpenMapTiles data schema. The tests work by injecting test OSM data and updates into the database and checking to ensure that the data is properly loaded into the database using standard SQL statements. With this framework in place, developers can now write small tests to inject known data into the database and ensure that imports and updates are working correctly.
In addition to the framework, basic tests are provided for four layers. These initial tests are in no way comprehensive, but they provide a structure and framework for developers to add their own tests or expand the existing ones to cover more cases.
Usage:
`make clean && make sql-test`
## How it works
The SQL tests consist of the following parts:
1. **Test import data**, located in `tests/import`. This test data is in the [OSM XML](https://wiki.openstreetmap.org/wiki/OSM_XML) format and contains the data that should be initially injected into the database. The files are numbered in order to ensure that each test data file contains OSM id numbers that are distinct from the other files. For example, the file starting with `100` will use node ids from 100000-199999, way ids from 1000-1999, and relation ids from 100-199.
1. **Test update data**, located in `tests/update`. This test data is in the [osmChange XML](https://wiki.openstreetmap.org/wiki/OsmChange) format, and contains the data that will be used to update the test import data (in order to verify that the update process is working correctly. These files are also numbered using the same scheme as the test import data.
1. **Import SQL test script**, located at `tests/test-post-import.sql`. This script is executed after the test import data has been injected, and runs SQL-based checks to ensure that the import data was properly imported. If there are failures in the tests, an entry will be added to the table `omt_test_failures`, with one record per error that occurs during the import process. A test failure will also fail the build. To inspect the test failure messages, run `make psql` and issue the comment `SELECT * FROM omt_test_failures`.
1. **Update SQL test script**, located at `tests/test-post-update.sql`. This script performs the same function as the import test script, except that it occurs after the test update data has been applied to the database. Note that script will only run if the import script passes all tests.
This PR does the following:
1. Suppresses underground buildings from rendering, when such buildings are tagged `location=underground`. Based on the wiki documentation, the `layer` tag does not determine whether a feature is above of below ground.
2. Unifies the building and building relation imposm exclusion mappings.
The following underground building feature in Luxembourg is used as the test object:
https://www.openstreetmap.org/way/582873794
* Add boundaries info to Readme
* make NE usage less specific - it may change later.
Co-authored-by: Yuri Astrakhan <yuriastrakhan@gmail.com>
Co-authored-by: Tomas Pohanka <TomPohys@gmail.com>
This PR fixes a bug in the `WHERE` clauses in the transportation layer generalization tables for zoom 10. The intended behavior of the layer is to suppress `highway=tertiary` and `highway=tertiary_link` at zoom 10 and lower. However, due to this bug, these objects were not suppressed as intended, because an `OR` was used in the SQL where an `AND` was needed instead.
This bug was inadvertently introduced in #1172😞
After commit b4b897999d the `make import-borders` rule no longer exists, but it's still mentioned in the README a couple of times. This PR removes those references.
Some [cleanup files are also mentioned in `.env`](4d6945b935/.env (L42-L44)). I'm not sure if those lines can be removed too, because the variables are used in [`bin/import-borders`](3796c4eab0/bin/import-borders (L8-L10)) in the `openmaptiles/openmaptiles-tools` repo.
Cache dir should always exist, so after deleting it,
it must be recreated. This is done in `make init-dirs`,
but if gets destroyed here, it won't auto-recreate.
This PR removes unneeded `CASE` clauses which converts `toll` tags to booleans. However, the `toll` tag is already mapped as a boolean by imposm, therefore this processing can be simplified.
I was reading through the SQL used to generalise the landcover layer ([`layers/landcover/generalized.sql`](596f44aa26/layers/landcover/generalized.sql)) and I noticed that when creating the generalised landcover tables for zooms 9-13, the SQL was doing something like this:
```sql
SELECT * FROM simplify_vw_z13 WHERE ST_NPoints(geometry) < 50
UNION ALL
SELECT * FROM simplify_vw_z13 WHERE ST_NPoints(geometry) >= 50 AND ST_NPoints(geometry) < 300
```
As in, the simplification/clustering of landcover generalisations was being done in two steps: once for polygons with fewer than fifty points, and once for polygons with 50+ points.
As far as I can see, there's no benefit to doing this — it's probably just an artefact of the dev work done for the [original pull request](https://github.com/openmaptiles/openmaptiles/pull/1035). Of course, I might be completely missing the reason for clustering polygons into two groups (<50 points and 50+ points). But assuming I haven't, I've created this pull request to simplify the SQL used in the generalisation, merging the two steps into something like this:
```sql
SELECT * FROM simplify_vw_z13 WHERE ST_NPoints(geometry) < 300
```
The effect is to slightly reduce the number of landcover features (polygons with <50 points can now be clustered/unioned with those 50+), and to reduce the time taken to generalise the landcover tables (in a small test using OSM's Iceland data I saw a 7% reduction in the time spent). It doesn't alter the features that are shown on the map.
Builds upon work from commits 4a1b0afa26 and da689f9e42.
While reviewing #1255, it became apparent that the ETL documentation for the landcover layer was incomplete. This PR updates the ETL documentation in `generalized.sql` so that each table has a node in the ETL diagram.
Avoid redirection of error messages to output files
- build/openmaptiles.tm2source/data.yml
- build/mapping.yaml
Currently, if an error occurs when `make all` creates any of these files, the error message is not printed to the console.
```
docker-compose run --rm --user=1000:1000 openmaptiles-tools generate-tm2source openmaptiles.yaml --host="postgres" --port=5432 --database="openmaptiles" --user="openmaptiles" --password="openmaptiles" > build/openmaptiles.tm2source/data.yml
Creating openmaptiles_openmaptiles-tools_run ... done
ERROR: 1
make: *** [Makefile:257: build/openmaptiles.tm2source/data.yml] Error 1
```
An inexperienced user would not know where to look for the details.
With this PR, the error is sent to the console:
```
docker-compose run --rm --user=1000:1000 openmaptiles-tools bash -c \
'generate-tm2source openmaptiles.yaml --host="postgres" --port=5432 --database="openmaptiles" --user="openmaptiles" --password="openmaptiles" > build/openmaptiles.tm2source/data.yml'
Creating openmaptiles_openmaptiles-tools_run ... done
Could not parse /tileset/layers/aeroway/mapping.yaml
found undefined alias 'refkjbkjkhh'
in "/tileset/layers/aeroway/mapping.yaml", line 94, column 7
ERROR: 1
make: *** [Makefile:257: build/openmaptiles.tm2source/data.yml] Error 1
```
Redirection of stdout is now done in the container, leaving stderr to go to the host.
This test applies 2-3 months of weekly updates on the `europe/monaco` extract from [Geofabrik](http://download.geofabrik.de/europe/).
It is worth noting that the contents of the updates may vary, and some SQL update flows may not be tested if the relevant changes did not occur during that period.
Resolves#1226
This PR fixes a bug in the park polygon update code introduced in #1160. Because park polygons at zoom 4 are dissolved, the centroid is not present in the table at that layer. Thus, a separate update trigger is needed to avoid errors. This PR adds the separate trigger for the zoom 4 park layer.
This PR likely resolves an unidentified blocker for #1245
Fixes#1156Fixes#810Fixes#1228
This PR replaces `osmborder`, which is no longer maintained, with `imposm` mappings and SQL code to generate borders. Key features that were moved into the imposm/SQL layer:
1. Grouping by `osm_id` and aggregating by lowest `admin_level` value so that there's only one copy of ways that are members of multiple relations.
2. Filtering out of point features in boundary relations (typically `admin_centre` and `label` roles).
3. Move disputed boundary detection logic into SQL.
This will increase the database size slightly because of the limits of what imposm can do, as some of the filtering is done in the SQL layer after importing, rather than being done in `osmborder`.
This PR is a bugfix for the `transportation_name` layer. Currently, the updates process does not work, as noted by @zstadler in #1230. The issue is that the computed `concurrency_index` column in `osm_route_member` is never updated after the initial updates. Therefore, whenever route member ways are updated, they will lose their route concurrency index, and hence route concurrencies will be lost in the `transportation_name` tiles. This PR adds the missing concurrency index update code to the transportation/network update triggers.
This PR as written also incorporates the fixes in #1230 and #1233.
The following SQL is a unit test to demonstrate that these triggers work correctly:
```
select count(*) from osm_route_member where concurrency_index > 5;
select count(*) from osm_transportation_name_network where route_6 is not null;
update osm_route_member set concurrency_index=NULL where concurrency_index > 5;
select count(*) from osm_route_member where concurrency_index > 5;
select count(*) from osm_transportation_name_network where route_6 is not null;
```
If working correctly, both pairs of `count(*)` values should return the same pair of numbers.
Because of the parallel processing of `import-sql` the error message is printed in the middle of the output. With this PR the error is displayed again at the end:
Issue: https://github.com/openmaptiles/openmaptiles-tools/pull/370
```
awk '1{print; fflush()} $$0~".*ERROR" {txt=$$0} END{ if(txt){print "\n*** ERROR detected, aborting:"; print txt; exit(1)} }'
```
Explanation:
- `1{print; fflush()}` means if true then print output -> `1{}` same as `if(true){}`
- `$$0~".*ERROR" {txt=$$0}` get first argument `$$0` (line of output) and check if it contains (regex string) `ERROR`. If true save line to var `txt`: `{txt=$$0}`
- `END{ ... }` if last line of output is reached
- `if(txt){print "\n*** ERROR detected, aborting:"; print txt; exit(1)}` if error was found in a line / var `txt` is existing print it out
While troubleshooting #1230, I discovered that there were cases where a way was a member of the same route relation more than once, such as: https://www.openstreetmap.org/way/17439235
<img src="https://user-images.githubusercontent.com/3254090/133726616-acd01332-343e-4930-ad3d-1fb3c3190a4c.png" width=280/>
This segment would end up showing duplicate highway shields in a rendered style, which is not desirable. While I personally think this style of tagging is wrong, it's a relatively easy fix to remove those duplicates. This PR replaces `ROW_NUMBER()` with `DENSE_RANK()` in the concurrency index generation code and adds a `DISTINCT` constraint on the concurrency join to handle multiple member rows with the same concurrency index.
Since I was in this file, I also fixed the copy/paste error in generating the route names.
Screenshot from database showing correct mapping:
The tag `landuse=park` is a rare tagging mistake of `leisure=park`. The OSM wiki [lists this tag as a mistake](https://wiki.openstreetmap.org/wiki/Tag:leisure%3Dpark). It is included in the landcover layer with a note that there were (at the time) 600 parks tagged in this way. There are currently a bit over 100 objects tagged with this mistake. Based on the wiki documentation and obvious decline of this tag, we should remove it so that we're not introducing mapping mistakes into the database and not inadvertently providing mapper feedback that encourages the use of mistake tagging.
This PR is a refactor. The purpose of this PR is to refactor the logic in the z12-z14 transportation layer to increase readability and reduce code complexity.
In addition, this fixes bugs in the original filter code:
* Create filter parity between `service` and `service_construction` highway classes.
* Removal of unintended `class` values `minor_construction`, `path_construction`, `service_construction`, and `track_construction` from zoom 12 filter by explicitly listing the highway classes that are included.
On some systems the `bc` command is not installed, so I added a check if the command can be used.
It is a simply calculation from KB to GB. If `bc` is missing, then it is displayed in KB.
fix: #1151
Adding -T fixes the issue when users are unable to stop tile genartion with ctrl+c
For some reason it doesn't work without it, even though the process of building
and running both docker images is the same.
This PR adds support for several features in the `natural` key, including `saddle`, `ridge`, `arete`, and `cliff`. These features all currently render in openstreetmap-carto, so adding these features would advance OpenMapTiles towards its goal of parity with the standard renderer, as indicated in https://github.com/openstreetmap/operations/issues/565#issuecomment-907303115.
This PR also adds the features requested in #274, with the exception of valleys, which I've left out of this PR because there are different complexities to mapping valleys that should be addressed as a separate (future) PR.
### Examples of mountain features in paper maps
These features are regularly found in American-style maps and thus is of interest to the openstreetmap-americana map style. Below are examples of these mountain-related features found in general purpose maps:
<img src="https://user-images.githubusercontent.com/3254090/131270340-af80c7bf-9416-40a3-9b64-56418abb2aef.png" width=400/>
<img src="https://user-images.githubusercontent.com/3254090/131270375-b9eb2095-7708-443d-b7bc-22bf8adab721.png" width=400/>
<img src="https://user-images.githubusercontent.com/3254090/131270423-64447ad3-d90a-4615-82e1-91175a8f8a6b.png" width=400/>
<img src="https://user-images.githubusercontent.com/3254090/131270506-2248db9f-ded5-443f-82ed-b0e8d4d12a70.png" width=400/>
### Approach
This PR extends the existing `mountain_peak` layer by adding other mountain features. We may want to consider renaming this layer in version 4.0 to be more inclusive of other mountain features including the potential future addition of valleys. However, the features added in this PR are associated with mountains and so their inclusion in this layer is the most appropriate location.
A new `osm_mountain_linestring` mapping maps the new linear mountain features, with similar ranking logic as is used for the existing `peak`/`volcano` features. Additionally, `natural=saddle` is added to the `osm_peak_point` mapping and ranked using the formula for peaks. Since saddles will have lower elevations than peaks, important saddles will be preempted by important peaks.
The new linestring features are rendered only at zoom 13-14, in order to match the zoom at which they appear in openstreetmap-carto. However, it may be appropriate in a future PR to extend the rendering of these features as generalized linestrings at lower zooms.
### Test Renderings
Below is a test rendering showing aretes and peaks in Austria, just north of the Swiss border, followed by a screen shot of the [same location](https://www.openstreetmap.org/#map=14/46.8682/10.0863) in openstreetmap-carto:
**OpenMapTiles, zoom 14**
<img src="https://user-images.githubusercontent.com/3254090/131271258-5cd90bdb-cac2-41d8-887f-b4bf6be83673.png" width=400/>
**openstreetmap-carto, zoom r14/v13:**
<img src="https://user-images.githubusercontent.com/3254090/131271332-32d5bcfc-41c6-4625-829c-df063b7af523.png" width=400/>
While importing multiple .pbf files into postgres I was confronted with some unnecessary problems:
1. I wanted to use the borders of the planet which I had already generated. But `make import-borders` needs a .pbf file which is not necessary if the line.csv file is already existing -> So I added a check if the line.csv is **not** existing it checks for the `area parameter` / .pbf file
2. `make generate-tiles-pg` is only possible with a specified `area parameter` BUT the area is not used / needed for generation -> So I removed the area check
Fixes#1191
This PR suppresses `highway=service` at zoom 12, where it is not a useful level of detail. This makes OpenMapTiles consistent with openstreetmap-carto, which does not begin showing `highway=service` until raster zoom 14 / vector zoom 13.
Additionally, this PR suppresses `highway=service` + `service=parking_aisle` / `service=driveway` from zooms 12-13, as this detail is excessive below zoom 14. As a point of comparison, openstreetmap-carto does not begin showing `service=parking_aisle` / `service=driveway` until raster zoom 16 (vector zoom 15).
Fixes#948
This PR does the following:
1. Changes the trigger for the PR comment updater from the cron method to workflow_run, triggered on completion of the test cases. This should remove the delay between the completion of the performance tests and the updating of the corresponding comment in the PR.
2. Separates the integrity check and performance check into separate workflows and allows them to run in parallel. This will allow the project to take advantage of multiple CI runners if they're available (which appears to be the case).
In addition, this fixes an issue with post-merge undeleted/updated branches on PRs. The current "cron" method causes the CI to run the pr-update job over and over forever, unnecessarily.
As described in github/docs#799, and the [github docs](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#workflow_run), a `workflow_run` trigger will only fire when the workflow file is on the main branch. Thus, this change will not fire the PR updater on this PR. Thus there's no way to test this working properly without merging onto master and then testing on one of the other PRs.
zstadler
