Removal of unneeded dispute lua and additional country support by jeffdefacto · Pull Request #2068 · tilezen/vector-datasource

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.

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.

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

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

nvkelso

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

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

+        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

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

… junderwood/additional_country_support

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

+        output_hstore['place:RU'] = 'country'
    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

-        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

+        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

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

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


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

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


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

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


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

+    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

      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

+    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

+        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

Comment thread osm2pgsql.lua

tgrigsby-sc reviewed Apr 20, 2022

View reviewed changes

Comment thread osm2pgsql.lua

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

Comment thread osm2pgsql.lua

nvkelso reviewed Apr 20, 2022

View reviewed changes

Comment thread osm2pgsql.lua