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.

Sign up for GitHub

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

Removal of unneeded dispute lua and additional country support #2068

Merged
merged 18 commits into from
May 3, 2022
Merged

Removal of unneeded dispute lua and additional country support #2068

Changes from 11 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
33979e5
lua to upgrade dependencies and territories to country status for lab…
Apr 12, 2022
e63395e
Remove unneeded disputes and add ne_id and ne:brk to way tags to allo…
Apr 14, 2022
c022e90
fixed Heard Island place type
Apr 20, 2022
f5c2b98
fix brk_a3 tag
Apr 20, 2022
964e37f
Change removed dispute place value to nil to ensure it doesn't end up…
Apr 20, 2022
d895639
Comment changes
Apr 21, 2022
85b7492
grouped the french overseas department into regions
Apr 22, 2022
0bc79b9
Merge remote-tracking branch 'origin/master' into junderwood/addition…
Apr 22, 2022
987d52c
grouped the french overseas department into regions
Apr 22, 2022
50a6199
removed accidental inclusion
Apr 22, 2022
e345017
fixed place types
Apr 26, 2022
a98f7d9
Add yaml filter for place=unrecognized and upload lua
Apr 28, 2022
03046f4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 28, 2022
a784e74
typo
Apr 28, 2022
b6a7cbc
Merge remote-tracking branch 'origin/additional_country_support' into…
Apr 28, 2022
2c8da9b
Addressed some of the comments
Apr 29, 2022
86213fc
Merge remote-tracking branch 'origin/master' into junderwood/addition…
Apr 29, 2022
d6d1761
updated french dependencies back to country
May 2, 2022
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
228 changes: 120 additions & 108 deletions osm2pgsql.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,130 +499,129 @@ function osm2pgsql.process_node(object)
object.tags.wikidata == 'Q994730') then
output_hstore['disputed_by'] = 'CN;RU;IN;GR'
end
-- Recast Northern Cyprus as country label and turn off for several POVs including China and Russia

-- Turn off Northern Cyprus label for most countries
if object.tags.place and object.tags.wikidata == 'Q23681' then
output_hstore['place'] = 'country'
output_hstore['place'] = 'region'
Copy link
Member

@nvkelso nvkelso Apr 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This needs to be region or country to get included by filters in places.yaml, but then most countries shouldn't see this label, but Turkey should.

Something like?

output_hstore['place'] = 'country'

But if the country is not in the other NE join table in post-processing, then it'd default to kind: unrecognized.

output_hstore['place:TR'] = 'country'

To make sure Turkish audience does see it?

Copy link
Member

Choose a reason for hiding this comment

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

Another option is we extend places.yaml to filter also on place = unrecognized for countries, that would make all this easier to follow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

place=unrecognized sounds like a good idea. It will remove some points of failure in the process and simplify this. Going to give that try

Copy link
Member

Choose a reason for hiding this comment

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

This seems to be working nicely, let's see it in a build to 100% check :)

output_hstore['disputed_by'] = 'CN;RU;IN;GR;CY'
nvkelso marked this conversation as resolved.
Show resolved Hide resolved
end
-- Turn off Abkhazia label for most countries
nvkelso marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Please update comment:

-- Show Abkhazia label as region for more countries

if object.tags.place and object.tags.wikidata == 'Q23334' then
output_hstore['place:AR'] = 'region'
nvkelso marked this conversation as resolved.
Show resolved Hide resolved
output_hstore['place:BD'] = 'region'
output_hstore['place:BR'] = 'region'
output_hstore['place:CN'] = 'region'
output_hstore['place:DE'] = 'region'
output_hstore['place:EG'] = 'region'
output_hstore['place:GB'] = 'region'
output_hstore['place:GR'] = 'region'
output_hstore['place:ID'] = 'region'
output_hstore['place:IL'] = 'region'
output_hstore['place:IN'] = 'region'
output_hstore['place:IT'] = 'region'
output_hstore['place:JP'] = 'region'
output_hstore['place:KO'] = 'region'
output_hstore['place:MA'] = 'region'
output_hstore['place:NL'] = 'region'
output_hstore['place:NP'] = 'region'
output_hstore['place:PK'] = 'region'
output_hstore['place:PL'] = 'region'
output_hstore['place:PS'] = 'region'
output_hstore['place:PT'] = 'region'
output_hstore['place:SA'] = 'region'
nvkelso marked this conversation as resolved.
Show resolved Hide resolved
output_hstore['place:SE'] = 'region'
output_hstore['place:TR'] = 'region'
output_hstore['place:TW'] = 'region'
output_hstore['place:UA'] = 'region'
output_hstore['place:VN'] = 'region'
output_hstore['place'] = 'region'
output_hstore['place:RU'] = 'country'
end
-- Turn off South Ossetia label for most countries
nvkelso marked this conversation as resolved.
Show resolved Hide resolved
if object.tags.place and object.tags.wikidata == 'Q23427' then
output_hstore['place:AR'] = 'region'
nvkelso marked this conversation as resolved.
Show resolved Hide resolved
output_hstore['place:BD'] = 'region'
output_hstore['place:BR'] = 'region'
output_hstore['place:CN'] = 'region'
output_hstore['place:DE'] = 'region'
output_hstore['place:EG'] = 'region'
output_hstore['place:GB'] = 'region'
output_hstore['place:GR'] = 'region'
output_hstore['place:ID'] = 'region'
output_hstore['place:IL'] = 'region'
output_hstore['place:IN'] = 'region'
output_hstore['place:IT'] = 'region'
output_hstore['place:JP'] = 'region'
output_hstore['place:KO'] = 'region'
output_hstore['place:MA'] = 'region'
output_hstore['place:NL'] = 'region'
output_hstore['place:NP'] = 'region'
output_hstore['place:PK'] = 'region'
output_hstore['place:PL'] = 'region'
output_hstore['place:PS'] = 'region'
output_hstore['place:PT'] = 'region'
output_hstore['place:SA'] = 'region'
output_hstore['place:SE'] = 'region'
output_hstore['place:TR'] = 'region'
output_hstore['place:TW'] = 'region'
output_hstore['place:UA'] = 'region'
output_hstore['place:VN'] = 'region'
output_hstore['place'] = 'region'
nvkelso marked this conversation as resolved.
Show resolved Hide resolved
output_hstore['place:RU'] = 'country'
end
-- Turn off Nagorno-Karabakh label for most countries
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Change the name here to Artsakh (Nagorno-Karabakh) as that's the new name but not in common circulation so still put the old name in the parenthetical.

if object.tags.place and object.tags.wikidata == 'Q2397204' then
output_hstore['place:AR'] = 'region'
nvkelso marked this conversation as resolved.
Show resolved Hide resolved
output_hstore['place:BD'] = 'region'
output_hstore['place:BR'] = 'region'
output_hstore['place:CN'] = 'region'
output_hstore['place:DE'] = 'region'
output_hstore['place:EG'] = 'region'
output_hstore['place:ES'] = 'region'
output_hstore['place:FR'] = 'region'
output_hstore['place:GB'] = 'region'
output_hstore['place:GR'] = 'region'
output_hstore['place:ID'] = 'region'
output_hstore['place:IL'] = 'region'
output_hstore['place:IN'] = 'region'
output_hstore['place:IT'] = 'region'
output_hstore['place:JP'] = 'region'
output_hstore['place:KO'] = 'region'
output_hstore['place:MA'] = 'region'
output_hstore['place:NL'] = 'region'
output_hstore['place:NP'] = 'region'
output_hstore['place:PK'] = 'region'
output_hstore['place:PL'] = 'region'
output_hstore['place:PS'] = 'region'
output_hstore['place:PT'] = 'region'
output_hstore['place:RU'] = 'region'
output_hstore['place:SA'] = 'region'
output_hstore['place:SE'] = 'region'
output_hstore['place:TR'] = 'region'
output_hstore['place:TW'] = 'region'
output_hstore['place:US'] = 'region'
output_hstore['place:VN'] = 'region'
output_hstore['place'] = 'region'
nvkelso marked this conversation as resolved.
Show resolved Hide resolved
output_hstore['place:RU'] = 'country'
Copy link
Member

Choose a reason for hiding this comment

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

Rounding problems, is a map unit more like region or country?

Copy link
Member

Choose a reason for hiding this comment

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

Fine to do this way for now, but we should revisit once we add kind_detail for these.

end
-- Turn off Somaliland label for most countries
if object.tags.place and object.tags.wikidata == 'Q34754' then
output_hstore['place:AR'] = 'region'
output_hstore['place:BD'] = 'region'
output_hstore['place:BR'] = 'region'
output_hstore['place:CN'] = 'region'
output_hstore['place:EG'] = 'region'
output_hstore['place:GR'] = 'region'
output_hstore['place:ID'] = 'region'
output_hstore['place:IL'] = 'region'
output_hstore['place:IN'] = 'region'
output_hstore['place:MA'] = 'region'
output_hstore['place:NP'] = 'region'
output_hstore['place:PK'] = 'region'
output_hstore['place:PL'] = 'region'
output_hstore['place:PS'] = 'region'
output_hstore['place:PT'] = 'region'
output_hstore['place:RU'] = 'region'
output_hstore['place:SA'] = 'region'
output_hstore['place:SO'] = 'region'
output_hstore['place:TR'] = 'region'
output_hstore['place:TW'] = 'region'
output_hstore['place:UA'] = 'region'
output_hstore['place:VN'] = 'region'
output_hstore['place'] = 'region'
nvkelso marked this conversation as resolved.
Show resolved Hide resolved
end

-- Recast various dependencies as countries
-- American Samoa
jeffdefacto marked this conversation as resolved.
Show resolved Hide resolved
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'AS' then
output_hstore['place'] = 'country'
nvkelso marked this conversation as resolved.
Show resolved Hide resolved
end
-- Christmas Island
if object.tags.place == 'territory' and object.tags['ISO3166-1'] == 'CX' then
output_hstore['place'] = 'country'
end
-- Cocos (Keeling) Islands
if object.tags.place == 'territory' and object.tags['ISO3166-1'] == 'CC' then
output_hstore['place'] = 'country'
end
-- French Polynesia
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'PF' then
output_hstore['place'] = 'country'
end
-- French Southern and Antarctic Lands
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'TF' then
output_hstore['place'] = 'country'
end
-- Guam
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'GU' then
output_hstore['place'] = 'country'
end
-- Heard Island and McDonald Islands
if object.tags.place == 'territory' and object.tags['ISO3166-1'] == 'HM' then
Copy link
Member

Choose a reason for hiding this comment

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

That's a new one to me

output_hstore['place'] = 'country'
end
-- New Caledonia
if object.tags.place == 'archipelago' and object.tags['ISO3166-1'] == 'NF' then
output_hstore['place'] = 'region'
Copy link
Member

Choose a reason for hiding this comment

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

New Caledonia should be country as it's a dependency of France

end
-- Norfolk Island
if object.tags.place == 'territory' and object.tags['ISO3166-1'] == 'NF' then
output_hstore['place'] = 'country'
end
-- Northern Mariana Islands
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'MP' then
output_hstore['place'] = 'country'
end
-- Puerto Rico
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'PR' then
output_hstore['place'] = 'country'
end
-- Saint Barthelemy
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'BL' then
output_hstore['place'] = 'country'
end
-- Saint Helena, Ascension and Tristan da Cunha
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'SH' then
output_hstore['place'] = 'country'
end
-- Saint Martin
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'MF' then
output_hstore['place'] = 'country'
end
-- Saint Pierre and Miquelon
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'PM' then
output_hstore['place'] = 'country'
end
-- Svalbard
if object.tags.place == 'region' and object.tags['ISO3166-1'] == 'SJ' then
output_hstore['place'] = 'country'
end
-- United States Virgin Islands
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'VI' then
output_hstore['place'] = 'country'
end
-- Wallis and Futuna
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'WF' then
output_hstore['place'] = 'country'
end

-- Recast various French overseas departments as region
-- Réunion
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'RE' then
output_hstore['place'] = 'region'
Copy link
Member

@nvkelso nvkelso Apr 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This will result in no label drawn a particular style, unless you also add the some/full set of place:XX codes as country (or you set the min_zoom value to be better than 3.7). This is also because they aren't listed in the NE country table (only map unit tables).

Copy link
Member

Choose a reason for hiding this comment

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

EG:

output_hstore['place:AR'] = 'country'
output_hstore['place:BD'] = 'country'
output_hstore['place:BR'] = 'country'
output_hstore['place:CN'] = 'country'
output_hstore['place:DE'] = 'country'
output_hstore['place:EG'] = 'country'
output_hstore['place:GB'] = 'country'
output_hstore['place:GR'] = 'country'
output_hstore['place:ID'] = 'country'
output_hstore['place:IL'] = 'country'
output_hstore['place:IN'] = 'country'
output_hstore['place:IT'] = 'country'
output_hstore['place:JP'] = 'country'
output_hstore['place:KO'] = 'country'
output_hstore['place:MA'] = 'country'
output_hstore['place:NL'] = 'country'
output_hstore['place:NP'] = 'country'
output_hstore['place:PK'] = 'country'
output_hstore['place:PL'] = 'country'
output_hstore['place:PS'] = 'country'
output_hstore['place:PT'] = 'country'
output_hstore['place:RU'] = 'country'
output_hstore['place:SA'] = 'country'
output_hstore['place:SE'] = 'country'
output_hstore['place:TR'] = 'country'
output_hstore['place:TW'] = 'country'
output_hstore['place:UA'] = 'country'
output_hstore['place:VN'] = 'country'

Copy link
Member

Choose a reason for hiding this comment

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

Let's still do this now...

It's also related to work in #2077.

Copy link
Member

Choose a reason for hiding this comment

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

Let's continue this in #2077... and reset to "place" = "country" here and for all 5.

end
-- Martinique
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'MQ' then
output_hstore['place'] = 'region'
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

end
-- Mayotte
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'YT' then
output_hstore['place'] = 'region'
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

end
-- Guadeloupe
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'GP' then
output_hstore['place'] = 'region'
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

end
-- French Guiana
if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'GF' then
output_hstore['place'] = 'region'
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

end


output.tags = output_hstore

if hstore_column then
Expand Down Expand Up @@ -720,6 +719,12 @@ function osm2pgsql.process_way(object)
if v.recognized_by then
output_hstore.recognized_by = v.recognized_by
end
if v.ne_id then
jeffdefacto marked this conversation as resolved.
Show resolved Hide resolved
output_hstore.ne_id = v.ne_id
end
if v['ne:brk_a3'] then
output_hstore['ne:brk_a3'] = v['ne:brk_a3']
end
end
end

Expand Down Expand Up @@ -833,6 +838,13 @@ function osm2pgsql.process_relation(object)
output_hstore['admin_level'] = '4'
end

-- Convert admin_level 5 boundaries in Cyprus to 4
if type == 'boundary' and object.tags.admin_level == '5' and object.tags['ISO3166-2'] then
if osm2pgsql.has_prefix(object.tags['ISO3166-2'], 'CY-') then
output_hstore['admin_level'] = '4'
end
end

-- Turn off West Bank and Judea and Samaria relations
if type == 'boundary' and object.tags.wikidata == 'Q36678' then
output_hstore = {}
Expand Down