Removal of unneeded dispute lua and additional country support #2068

jeffdefacto · 2022-04-20T19:13:39Z

This pulls out some of the lua changes for specific disputes that are no longer necessary as we are not displaying them at this time.

Additionally, 25 dependencies were missing from the map due to a lack of place tag and/or a value of place that is not country. These lua additions change these place nodes to place=country when matching the dependency's iso code.

Lastly, ne_id and ne:brk_id_a3 tags will get pushed to dispute ways to aid in creating a dispute_id.

…els.

…w for a dispute_id.

osm2pgsql.lua

tgrigsby-sc · 2022-04-20T19:22:03Z

this all makes sense to me, but I can't vouch for completeness, so I'm going to lean on @nvkelso for a proper review

… on the map.

osm2pgsql.lua

nvkelso · 2022-04-22T17:23:58Z

osm2pgsql.lua

+    end
+-- French Guiana
+    if object.tags.place == 'state' and object.tags['ISO3166-1'] == 'GF' then
+        output_hstore['place'] = 'country'


This is an odd one, it's an overseas region of France (like Alaska and Hawaii), but also gets it's own ISO code.

My preference is to be true to the data and mark it place = region... and if you want to add POV overrides for ISO and all the other countries we're tracking to be country then that's OK.

The full set are (src):

French Guiana

Guadeloupe

Martinique

Mayotte

Réunion

Changed them to region

(We keep bouncing back and forth on this one... I left new comments farther down.)

osm2pgsql.lua

nvkelso

Please see one last round of comment about French overseas regions.

nvkelso · 2022-04-28T16:48:02Z

osm2pgsql.lua

    if object.tags.place and object.tags.wikidata == 'Q23681' then
-        output_hstore['place'] = 'country'
+        output_hstore['place'] = 'region'
        output_hstore['disputed_by'] = 'CN;RU;IN;GR;CY'
    end
 -- Turn off Abkhazia label for most countries


Please update comment:

-- Show Abkhazia label as region for more countries

osm2pgsql.lua

for more information, see https://pre-commit.ci

… junderwood/additional_country_support

nvkelso · 2022-04-28T21:20:59Z

osm2pgsql.lua

    end
-- Turn off South Ossetia label for most countries
+-- Show South Ossetia label as region for most countries


Nit: Match comment to what code does:

-- Hide South Ossetia label for most countries

nvkelso · 2022-04-28T21:25:08Z

osm2pgsql.lua

-        output_hstore['place:US'] = 'region'
-        output_hstore['place:VN'] = 'region'
+        output_hstore['place'] = 'unrecognized'
+        output_hstore['place:RU'] = 'country'


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

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

nvkelso · 2022-04-28T21:40:21Z

osm2pgsql.lua

+        output_hstore['place'] = 'country'
+    end
+-- Heard Island and McDonald Islands
+    if object.tags.place == 'territory' and object.tags['ISO3166-1'] == 'HM' then


That's a new one to me

nvkelso · 2022-04-28T21:42:46Z

osm2pgsql.lua

+-- 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'


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).

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'

Let's still do this now...

It's also related to work in #2077.

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

nvkelso · 2022-04-28T21:42:54Z

osm2pgsql.lua

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


nvkelso · 2022-04-28T21:43:00Z

osm2pgsql.lua

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


nvkelso · 2022-04-28T21:43:06Z

osm2pgsql.lua

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


nvkelso · 2022-04-28T21:43:11Z

osm2pgsql.lua

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


nvkelso · 2022-04-28T21:47:49Z

yaml/places.yaml

@@ -176,6 +176,18 @@ filters:
      kind: region
      kind_detail: province
    table: osm
+  - filter: {name: true, place: unrecognized}


Considering consolidating this into the existing section above

- filter: {name: true, place: country}

As

- filter: {name: true, place: [country,unrecognized]}

With

kind: unrecognized

Changing to

kind: {col: place}

Around line 141

nice. even simpler

nvkelso · 2022-04-28T21:50:45Z

osm2pgsql.lua

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


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

nvkelso

-- Turn off Kosovo country label for CN;RU;IN;GR

Also add here NP, BR, PS, MA, AR, VN, ID, UA

-- Hide Kosovo region labels for several POVs including China and Russia

Also add here NP, BR, PS, MA, AR, VN, ID, UA

nvkelso · 2022-04-28T21:58:51Z

osm2pgsql.lua

-        output_hstore['place:UA'] = 'region'
-        output_hstore['place:VN'] = 'region'
+        output_hstore['place'] = 'unrecognized'
+        output_hstore['place:RU'] = 'country'
    end
 -- Turn off Nagorno-Karabakh label for most countries


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.

nvkelso

See comments

…al_country_support

jeffdefacto · 2022-05-02T22:19:00Z

@nvkelso I believe all the comments have been addressed now.

nvkelso

Thanks! LGTM

junderwood added 4 commits April 12, 2022 16:50

lua to upgrade dependencies and territories to country status for lab…

33979e5

…els.

Remove unneeded disputes and add ne_id and ne:brk to way tags to allo…

e63395e

…w for a dispute_id.

fixed Heard Island place type

c022e90

fix brk_a3 tag

f5c2b98

jeffdefacto marked this pull request as ready for review April 20, 2022 19:15

jeffdefacto requested review from nvkelso and tgrigsby-sc April 20, 2022 19:15

tgrigsby-sc reviewed Apr 20, 2022

View reviewed changes

osm2pgsql.lua Show resolved Hide resolved

tgrigsby-sc reviewed Apr 20, 2022

View reviewed changes

osm2pgsql.lua Show resolved Hide resolved

Change removed dispute place value to nil to ensure it doesn't end up…

964e37f

… on the map.

nvkelso reviewed Apr 20, 2022

View reviewed changes

osm2pgsql.lua Outdated Show resolved Hide resolved

nvkelso reviewed Apr 20, 2022

View reviewed changes

osm2pgsql.lua Show resolved Hide resolved

nvkelso reviewed Apr 21, 2022

View reviewed changes

osm2pgsql.lua Outdated Show resolved Hide resolved

nvkelso reviewed Apr 21, 2022

View reviewed changes

osm2pgsql.lua Show resolved Hide resolved

nvkelso reviewed Apr 21, 2022

View reviewed changes

osm2pgsql.lua Show resolved Hide resolved

nvkelso reviewed Apr 21, 2022

View reviewed changes

osm2pgsql.lua Show resolved Hide resolved

nvkelso reviewed Apr 21, 2022

View reviewed changes

osm2pgsql.lua Show resolved Hide resolved

nvkelso reviewed Apr 21, 2022

View reviewed changes

osm2pgsql.lua Show resolved Hide resolved

nvkelso reviewed Apr 21, 2022

View reviewed changes

osm2pgsql.lua Show resolved Hide resolved

nvkelso reviewed Apr 21, 2022

View reviewed changes

osm2pgsql.lua Outdated Show resolved Hide resolved

nvkelso reviewed Apr 21, 2022

View reviewed changes

osm2pgsql.lua Show resolved Hide resolved

Comment changes

d895639

nvkelso reviewed Apr 22, 2022

View reviewed changes

osm2pgsql.lua Outdated Show resolved Hide resolved

nvkelso reviewed Apr 22, 2022

View reviewed changes

osm2pgsql.lua Outdated Show resolved Hide resolved

nvkelso reviewed Apr 22, 2022

View reviewed changes

osm2pgsql.lua Show resolved Hide resolved

nvkelso reviewed Apr 22, 2022

View reviewed changes

osm2pgsql.lua Outdated Show resolved Hide resolved

nvkelso requested changes Apr 22, 2022

View reviewed changes

grouped the french overseas department into regions

85b7492

nvkelso reviewed Apr 28, 2022

View reviewed changes

osm2pgsql.lua Outdated Show resolved Hide resolved

nvkelso reviewed Apr 28, 2022

View reviewed changes

osm2pgsql.lua Outdated Show resolved Hide resolved

nvkelso reviewed Apr 28, 2022

View reviewed changes

osm2pgsql.lua Outdated Show resolved Hide resolved

junderwood and others added 4 commits April 28, 2022 12:35

Add yaml filter for place=unrecognized and upload lua

a98f7d9

[pre-commit.ci] auto fixes from pre-commit.com hooks

03046f4

for more information, see https://pre-commit.ci

typo

a784e74

Merge remote-tracking branch 'origin/additional_country_support' into…

b6a7cbc

… junderwood/additional_country_support

nvkelso reviewed Apr 28, 2022

View reviewed changes

nvkelso requested changes Apr 28, 2022

View reviewed changes

nvkelso reviewed Apr 28, 2022

View reviewed changes

nvkelso mentioned this pull request Apr 28, 2022

Add kind_detail for country places #2076

Open

nvkelso requested changes Apr 28, 2022

View reviewed changes

junderwood added 3 commits April 28, 2022 17:42

Addressed some of the comments

2c8da9b

Merge remote-tracking branch 'origin/master' into junderwood/addition…

86213fc

…al_country_support

updated french dependencies back to country

d6d1761

nvkelso approved these changes May 3, 2022

View reviewed changes

jeffdefacto merged commit 66740a3 into master May 3, 2022

jeffdefacto deleted the additional_country_support branch May 3, 2022 21:44

nvkelso mentioned this pull request Oct 5, 2022

Always x-check country labels against Natural Earth #2077

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removal of unneeded dispute lua and additional country support #2068

Removal of unneeded dispute lua and additional country support #2068

jeffdefacto commented Apr 20, 2022 •

edited

Loading

tgrigsby-sc commented Apr 20, 2022

nvkelso Apr 22, 2022

jeffdefacto Apr 22, 2022

nvkelso Apr 28, 2022

nvkelso left a comment

nvkelso Apr 28, 2022

nvkelso Apr 28, 2022

nvkelso Apr 28, 2022

nvkelso Apr 28, 2022

nvkelso Apr 28, 2022

nvkelso Apr 28, 2022 •

edited

Loading

nvkelso Apr 28, 2022

nvkelso Apr 28, 2022

nvkelso Apr 29, 2022

nvkelso Apr 28, 2022

nvkelso Apr 28, 2022

nvkelso Apr 28, 2022

nvkelso Apr 28, 2022

nvkelso Apr 28, 2022

nvkelso Apr 28, 2022

jeffdefacto Apr 28, 2022

nvkelso Apr 28, 2022

nvkelso left a comment

nvkelso Apr 28, 2022

nvkelso left a comment

jeffdefacto commented May 2, 2022

nvkelso left a comment

Removal of unneeded dispute lua and additional country support #2068

Removal of unneeded dispute lua and additional country support #2068

Conversation

jeffdefacto commented Apr 20, 2022 • edited Loading

tgrigsby-sc commented Apr 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvkelso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvkelso Apr 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvkelso left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvkelso left a comment

Choose a reason for hiding this comment

jeffdefacto commented May 2, 2022

nvkelso left a comment

Choose a reason for hiding this comment

jeffdefacto commented Apr 20, 2022 •

edited

Loading

nvkelso Apr 28, 2022 •

edited

Loading