Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
tgrigsby-sc
left a comment
There was a problem hiding this comment.
This all makes sense to me! I have some minor cosmetic suggestions but, assuming there are no issues when one of the rows is bad (e.g. a null ne lang value should be ok), ship it!
| -- if it's a country, only look it up in the iso and tlc tables | ||
| IF place_tag='country' OR place_tag='unrecognized' THEN | ||
| SELECT hstore(ARRAY[ | ||
| 'ne_name_ar', i.name_ar, |
There was a problem hiding this comment.
I'm curious about the choice to do array - wouldn't just a straight hstore be better?
|
|
||
| ELSE | ||
| SELECT hstore(ARRAY[ | ||
| 'ne_name_ar', sp.name_ar, |
There was a problem hiding this comment.
I'm not sure if this is possible, but it seems preferable to have this arg list in a var somewhere and reuse it across all these queries - you'd just have to change the alias for each of the tables to the same thing. It's just visually difficult to verify that these are all the same
There was a problem hiding this comment.
It would also mean fewer places to update when we decide to add a country
| END IF; | ||
|
|
||
| ELSE | ||
| SELECT hstore(ARRAY[ |
There was a problem hiding this comment.
there's enough visual distance from the last comment that it's probably worth saying 'not a country' etc in a comment here
| IF NOT FOUND THEN | ||
| RETURN '{}'::jsonb; | ||
| END IF; | ||
| RETURN to_jsonb(coalesce(ne_name_tags, ''::hstore)); |
There was a problem hiding this comment.
oh does this coalesce turn an array into a map? if so, then ignore my previous comment about arrays
vectordatasource/transform.py
Outdated
| for k in props.keys(): | ||
| if k.startswith(name_prefix): | ||
| language = k[len(name_prefix):] | ||
| name = props.pop(k) |
There was a problem hiding this comment.
nit: maybe ne_name ? Makes it a little clearer what's going on.
Names from Natural Earth will overwrite OSM places features names where available