Skip to content
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

Tile size: water only changes #2010

Merged
merged 10 commits into from
Dec 16, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions integration-test/1135-water-lines-merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def test_successful_merge(self):
'name': 'foo',
'kind': 'river',
'label_placement': type(None),
}, 1)
}, 0)

with self.features_in_tile_layer(z, x, y, 'water') as features:
for f in features:
Expand Down Expand Up @@ -66,7 +66,7 @@ def test_unsuccessful_merge_same_props(self):
'name': 'foo',
'kind': 'river',
'label_placement': type(None),
}, 1)
}, 0)

with self.features_in_tile_layer(z, x, y, 'water') as features:
for f in features:
Expand Down Expand Up @@ -106,11 +106,11 @@ def test_unsuccessful_merge_diff_props(self):
z, x, y, 'water', {
'kind': 'river',
'label_placement': type(None),
}, 2)
}, 1)

with self.features_in_tile_layer(z, x, y, 'water') as features:
for f in features:
if 'label_placement' in f['properties']:
continue
assert f['geometry']['type'] == 'LineString'
assert f['geometry']['type'] == 'MultiLineString'
assert len(f['geometry']['coordinates']) == 2
6 changes: 2 additions & 4 deletions integration-test/1730-further-water-layer-name-dropping.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,11 @@ def test_label_lake_athabasca_z5(self):
}),
)

# we should get a label placement point, and its zoom should have been
# adjusted. we should also have all the names at this point.
# we should also have all the names at this point.
self.assert_has_feature(
z, x, y, 'water', {
'kind': 'lake',
'label_placement': True,
'min_zoom': 5,
'min_zoom': 2, # min_zoom changed at https://github.com/tilezen/vector-datasource/pull/2010/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the 'label_placement': True point should have names at this early zoom, and the min_zoom on that should probably be a different value besides 2 (like 7)?

'name': str,
'name:de': str,
})
6 changes: 3 additions & 3 deletions integration-test/1838-too-many-bays.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ def _bay(osm_id, area, name):
)

with self.features_in_tile_layer(z, x, y, 'water') as features:
# should only have top 10 features
self.assertEqual(10, len(features))
# should only have top 2 features
self.assertEqual(2, len(features))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At zoom 8 our area threshold for labels is 500,000,000 (was 200,000,000) and for polygons it's still 20000000.

Under the earlier system 5 meet the area threshold, but after only 2 do. So the changes here make sense to me.


# check that they're in order
ranks = [f['properties']['kind_tile_rank'] for f in features]
self.assertEqual(range(1, 11), ranks)
self.assertEqual(range(1, 3), ranks)
99 changes: 85 additions & 14 deletions queries.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,41 @@ post_process:
where: >-
kind == 'lake'

# remove names per kind for a given zoom
# before generating any label placements
- fn: vectordatasource.transform.drop_properties
params:
source_layer: water
start_zoom: 0
end_zoom: 16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exclusive

# short-hand for "name" property in the list below means all name-like
# properties.
all_name_variants: true
properties:
- name
- old_name
where: >-
(kind == 'sea' and zoom < 4) or
(kind == 'bay' and zoom < 5) or
(kind == 'fjord' and zoom < 5) or
(kind == 'strait' and zoom < 5) or
(kind == 'lake' and zoom < 5) or
(kind == 'playa' and zoom < 6) or
(kind == 'reef' and zoom < 6) or
(kind == 'river' and zoom < 12) or
(kind == 'riverbank' and zoom < 12) or
(kind == 'water' and zoom < 12) or
(kind == 'canal' and zoom < 13) or
(kind == 'basin' and zoom < 13) or
(kind == 'dock' and zoom < 13) or
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expanded the list of kinds in this section based on QA where I saw low zoom basin, dock and straight points around London and the English Channel. Now it has every water kind.

(kind == 'dam' and zoom < 14) or
(kind == 'stream' and zoom < 14) or
(kind == 'ditch' and zoom < 15) or
(kind == 'drain' and zoom < 15) or
(kind == 'swimming_pool' and zoom < 15) or
(kind == 'fountain' and zoom < 15) or
(properties is not None and properties.get('is_tunnel') is True)
nvkelso marked this conversation as resolved.
Show resolved Hide resolved

- fn: vectordatasource.transform.handle_label_placement
params:
layers:
Expand Down Expand Up @@ -822,22 +857,58 @@ post_process:
# table.
min_zoom: >-
[min_zoom for min_zoom, area_threshold in [
(4, 40000000000),
(5, 10000000000),
(6, 5000000000),
(7, 400000000),
(8, 200000000),
(9, 100000000),
(10, 10000000),
(11, 4000000),
(12, 750000),
(13, 100000),
(14, 50000),
(15, 10000),
(16, None),
(1, 1000000000000),
(2, 500000000000),
(3, 250000000000),
(4, 120000000000),
(5, 80000000000),
(6, 40000000000),
(7, 10000000000),
(8, 500000000),
(9, 200000000),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The larger range of min_zoom remappings is working for features are in the allowlist, woot!

(10, 40000000),
(11, 8000000),
(12, 1000000),
(13, 500000),
(14, 50000),
(15, 10000),
(16, None),
] if area >= area_threshold][0]
where: >-
kind == 'lake' and label_placement
(properties is not None and properties.get('label_placement') is True)

# now that we have the label points
# drop most water properties at lower zooms
# this is for lines and polygons
- fn: vectordatasource.transform.drop_properties
params:
source_layer: water
start_zoom: 0
end_zoom: 15
geom_types: [LineString, MultiLineString, Polygon, MultiPolygon]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to geom_types param instead of where clause.

properties:
- id
- area
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could add boundary and osm_relation, too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed via 77e663c.

- boundary
- layer
- wikidata_id
- osm_relation
- kind_tile_rank

# now that we have the label points
# drop water names at lower zooms for polys only
- fn: vectordatasource.transform.drop_properties
params:
source_layer: water
start_zoom: 0
end_zoom: 15
geom_types: [Polygon, MultiPolygon]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to geom_types param instead of where clause.

# short-hand for "name" property in the list below means all name-like
# properties.
all_name_variants: true
properties:
- name
- old_name

- fn: vectordatasource.transform.handle_label_placement
params:
Expand Down
10 changes: 8 additions & 2 deletions yaml/water.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,15 @@ globals:
key: { col: way_area }
op: '>='
table:
# OSM data starts at z8, but we should include some stuff at z7 so that
# OSM data starts at z8, but we should include some stuff at earlier so
# the transition doesn't have a gap in it.
- [ 7, 160000000 ]
- [ 1, 20000000000 ]
- [ 2, 10000000000 ]
- [ 3, 5000000000 ]
- [ 4, 2000000000 ]
- [ 5, 500000000 ]
- [ 6, 200000000 ]
- [ 7, 50000000 ]
- [ 8, 20000000 ]
- [ 9, 5000000 ]
- [ 10, 1000000 ]
Expand Down