Move to NE 5.1.2 - Add Config for new POVs #2078
Conversation
tgrigsby-sc
changed the title
tgrigsby-sc
changed the title
|
Updated for #2074 |
…with matching NE features (#2082)
for more information, see https://pre-commit.ci
tgrigsby-sc
force-pushed
the
travisg/20220428-add-NE-support-for-new-POVs
branch
from
eddd5d4 to
e0fa3de
Compare
data/assets.yaml
Outdated
| @@ -1,5 +1,5 @@ | |||
| bucket: nextzen-tile-assets | |||
| datestamp: 20220426 | |||
| datestamp: 20220429 | |||
There was a problem hiding this comment.
NIt: but 5.1.0 was only officially released on May 4th, so bring this forward?
There was a problem hiding this comment.
updated to most recent (which I think @jeffdefacto found an issue in for the iso countries.zip)
There was a problem hiding this comment.
That zero length char definition is really odd. Opens fine in QGIS but PostGIS barfs on it (saying it doesn't support 0 length varchar fields).
Filed mbloch/mapshaper#541 upstream.
There was a problem hiding this comment.
With mbloch/mapshaper#541 fixed upstream, I'll update Natural Earth to v5.1.1 with the fix.
There was a problem hiding this comment.
Natural Earth 5.1.1 is now out, please update here and see if the problem persists.
data/assets.yaml
Outdated
| @@ -55,98 +55,104 @@ shapefiles: | |||
| url: http://s3.amazonaws.com/tilezen-assets/curated/admin_areas_20180409.zip | |||
|
|
|||
| - name: ne_110m_lakes | |||
| url: http://www.naturalearthdata.com/http//www.naturalearthdata.com/download/110m/physical/ne_110m_lakes.zip | |||
| url: https://naciscdn.org/naturalearth/5.1.0/110m/physical/ne_110m_lakes.zip | |||
There was a problem hiding this comment.
Thanks for switching these to versioned CDN links... much more obvious which version we're on and makes updating NE a deliberate process.
There was a problem hiding this comment.
I agree. Much clearer
There was a problem hiding this comment.
These will all need to switch to 5.1.1 to pick up the other change.
data/functions.sql
Outdated
|
|
||
| -- There is no label_x and label_y for the non-countries | ||
| SELECT | ||
| fclass_iso, fclass_tlc, NULL, NULL INTO fclass_iso, fclass_tlc, label_x, label_y |
There was a problem hiding this comment.
This is confusingly called: "latitude" (label_y) and "longitude" (label_x) in the NE admin-1 table. Please update to pull them out.
There was a problem hiding this comment.
(so keep the output as label_{x,y} but the input would be longitude, latitude)
There was a problem hiding this comment.
fixed in 31a5b4a
data/functions.sql
Outdated
| -- finally, try localities | ||
| IF NOT FOUND THEN | ||
| SELECT | ||
| fclass_iso, fclass_tlc, NULL, NULL INTO fclass_iso, fclass_tlc, label_x, label_y |
There was a problem hiding this comment.
Same deal: This is confusingly called: "latitude" (label_y) and "longitude" (label_x) in the NE populated places table. Please update to pull them out.
There was a problem hiding this comment.
fixed in 31a5b4a
data/assets.yaml
Outdated
| @@ -141,12 +141,15 @@ shapefiles: | |||
|
|
|||
| - name: ne_10m_admin_0_countries | |||
| url: https://naciscdn.org/naturalearth/5.1.0/10m/cultural/ne_10m_admin_0_countries.zip | |||
| prj: 3857 | |||
There was a problem hiding this comment.
These files aren't web mercator (3857). Instead they need to be re-projected and clipped into web mercator from WGS84 lat/lng.
That's usually a later step in the (mostly?) automated build assets scripting. Doesn't look like that part ran in the ZIP you posted.
(Like the other NE files here, the prj: 3857 line is both not needed and in this case wrong and should be removed.)
There was a problem hiding this comment.
updated here: 2635448
nvkelso
changed the title
tgrigsby-sc
changed the title
fclass_isoandfclass_tlcandlabel_xandlabel_yfor OSM countries that match NE countriesFor #2082, I added the functionality to drop it into the output, but we aren't doing anything with it yet, so I haven't written any tests for it. Once we decide to adopt a label_x and label_y if present, for example, there will be compelling function to test.