From 42085337573d62974b34f99752c76d3bb92ae2e8 Mon Sep 17 00:00:00 2001 From: Paul Norman Date: Wed, 13 Mar 2024 19:27:21 -0700 Subject: [PATCH 1/2] shortbread: boundaries: Require admin_level to be one of the valid values Shortbread says 2 and 4 are the valid values of admin_level, so this doesn't look at a relation that is only tagged with type=boundary boundary=administrative and has no admin_level tagging. This doesn't strictly comply with shortbread because a member of a admin_level 3 and admin_level 4 relation should get admin_level set to 3 but I don't believe that's the intent of the spec. --- themes/shortbread_v1/topics/boundaries.lua | 25 +++++++++++++--------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/themes/shortbread_v1/topics/boundaries.lua b/themes/shortbread_v1/topics/boundaries.lua index 95d3e44..39cfacd 100644 --- a/themes/shortbread_v1/topics/boundaries.lua +++ b/themes/shortbread_v1/topics/boundaries.lua @@ -11,8 +11,8 @@ themepark:add_table{ name = 'boundaries', ids_type = 'way', geom = 'linestring', - columns = themepark:columns('core/name', { - { column = 'admin_level', type = 'int' }, + columns = themepark:columns({ + { column = 'admin_level', type = 'int', not_null = true }, { column = 'maritime', type = 'bool' }, { column = 'disputed', type = 'bool' }, }), @@ -33,24 +33,27 @@ local rinfos = {} -- --------------------------------------------------------------------------- +-- Check the (string) admin level. Change this depending on which admin +-- levels you want to process. Shortbread only shows 2 and 4. +-- valid values must work with tonumber! +local function valid_admin_level(level) + return level == '2' or level == '4' +end + -- Check if this looks like a boundary and return admin_level as number --- Return nil if this is not a valid boundary. +-- Return nil if this is not a valid administrative boundary. local function get_admin_level(tags) local type = tags.type if type == 'boundary' or type == 'multipolygon' then local boundary = tags.boundary - if boundary == 'administrative' or boundary == 'disputed' then + if boundary == 'administrative' and valid_admin_level(tags.admin_level) + or boundary == 'disputed' then return tonumber(tags.admin_level) end end end --- Check the (numeric) admin level. Change this depending on which admin --- levels you want to process. Shortbread only shows 2 and 4. -local function valid_admin_level(level) - return level == 2 or level == 4 -end -- --------------------------------------------------------------------------- @@ -76,7 +79,9 @@ themepark:add_proc('way', function(object, data) end) themepark:add_proc('select_relation_members', function(relation) - if valid_admin_level(get_admin_level(relation.tags)) then + -- It isn't necessary to process boundary=disputed relations separately because + -- if they have an admin_level from another relation they will get added anyways. + if valid_admin_level(relation.tags.admin_level) then return { ways = osm2pgsql.way_member_ids(relation) } end end) From deb0eae1dce10dea303a37544c047dcb33247c50 Mon Sep 17 00:00:00 2001 From: Paul Norman Date: Wed, 13 Mar 2024 19:50:36 -0700 Subject: [PATCH 2/2] shortbread: boundaries: Fix disputed boundary processing Previously a member of multiple relations would take its disputed status from the last relation processed. For example, processing a way that is a member of two relations, with type=boundary boundary=administrative admin_level=2 type=boundary boundary=disputed admin_level=2 would lead to order-dependent results, where disputed only comes from the last relation processed. Another issue was that a relation with type=boundary boundary=disputed and no other tags would not be considered as tonumber(tags.admin_level) is not truth-like. Instead, what is done is to get the disputed status for the relation, and then set it to true for the rinfos in that relation, not setting any existing rinfos to false. They are only set false when creating the entry in rinfos for the first time. --- themes/shortbread_v1/topics/boundaries.lua | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/themes/shortbread_v1/topics/boundaries.lua b/themes/shortbread_v1/topics/boundaries.lua index 39cfacd..d7d9d30 100644 --- a/themes/shortbread_v1/topics/boundaries.lua +++ b/themes/shortbread_v1/topics/boundaries.lua @@ -47,14 +47,18 @@ local function get_admin_level(tags) if type == 'boundary' or type == 'multipolygon' then local boundary = tags.boundary - if boundary == 'administrative' and valid_admin_level(tags.admin_level) - or boundary == 'disputed' then + if boundary == 'administrative' and valid_admin_level(tags.admin_level) then return tonumber(tags.admin_level) end end end +local function valid_disputed(tags) + local type = tags.type + return (type == 'boundary' or type == 'multipolygon') and tags.boundary == 'disputed' +end + -- --------------------------------------------------------------------------- themepark:add_proc('way', function(object, data) @@ -68,6 +72,9 @@ themepark:add_proc('way', function(object, data) end local t = object.tags + if not info.admin_level then + return + end local a = { admin_level = info.admin_level, maritime = (t.maritime and (t.maritime == 'yes' or t.natural == 'coastline')), @@ -90,19 +97,21 @@ themepark:add_proc('relation', function(object, data) local t = object.tags local admin_level = get_admin_level(t) - - if not valid_admin_level(admin_level) then + local disputed = valid_disputed(t) + -- If a relation does not an admin boundary or disputed boundary it has + -- nothing to tell us and we don't need the ways. + if not admin_level and not disputed then return end for _, member in ipairs(object.members) do if member.type == 'w' then if not rinfos[member.ref] then - rinfos[member.ref] = { admin_level = admin_level } + rinfos[member.ref] = { admin_level = admin_level, disputed = false } elseif rinfos[member.ref].admin_level > admin_level then rinfos[member.ref].admin_level = admin_level end - rinfos[member.ref].disputed = (t.boundary == 'disputed') + rinfos[member.ref].disputed = disputed or rinfos[member.ref].disputed end end end)