-
-
Notifications
You must be signed in to change notification settings - Fork 4
Add a places layer #19
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.
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
Conversation
src/places.py
Outdated
| Most place features are mapped as nodes, but some are mapped as areas | ||
| (typically neighborhoods, islands, etc). In these cases we include the | ||
| area's centroid in the output dataset. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There’s a potential for duplicate features if the same place is mapped as both a place point and a boundary relation and both are tagged with place=*. One approach is to limit place=hamlet/village/town/city to points only and avoid mapping a place point for territories that have no logical center, such as place=state. This approach is favored in some regions like the U.S. but not yet fully implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that thread and the links therein are useful references.
I'm currently including place areas only if they are not tagged boundary=*, to avoid creating duplicate rows in the output. Ideally I'd do something more sophisticated (checking if the boundary relation has a label member, for example) but this is beyond what I can easily do with pyosmium.
It seems like a reasonable suggestion to just omit certain place=* elements. The main use case I have in mind for this layer is as a dataset of populated human settlements, so omitting place=country/state/province which aren't settlements in the anthropological sense would be fine. I wonder if anyone would miss place=archipelago/island/islet if they were also omitted from this layer. Personally I would not, and having at most one row in the output for each human settlement seems like it's important enough to warrant some trade-offs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
place=archipelago/island/islet sound like good candidates for a different layer about landforms. Similarly, place=ocean would be a good candidate for a water layer, and place=square for a layer about pedestrian infrastructure or perhaps public spaces.
|
I renamed the layer to |
ianthetechie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor nits. I like the way you're factoring out the map-style tags, the approach to alternate names, and the new name. Place is such an overloaded term; this captures it better I think!
src/settlements.py
Outdated
| def tags_with_prefix(prefix, tags): | ||
| """ | ||
| Returns a dict of all tags with the given prefix string; keys in the | ||
| dict will have the prefix dropped. | ||
| """ | ||
| prefix_len = len(prefix) | ||
| return {k[prefix_len:]: v for (k, v) in tags if k.startswith(prefix)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this helper function! Since we'll probably need some variation of it elsewhere, maybe we should create put it in another module (helpers? sounds cliche but I'm not very creative at the moment :P) so others can call it as a top level function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call, thanks! I wrote this thinking it'd be used in other layers too but forgot to move it to a helpers module.
Co-authored-by: Ian Wagner <ian.wagner@stadiamaps.com>
|
Thank you both for the valuable feedback! I think this is ready to merge now. Definitely open to iterating on the schema further (and that goes for all of the other layers too), but by merging this we can get it built and available for people to try out, and also the helpers.py module will be available for other layers like Ian's boundaries PR. |
This PR adds a "Places" layer containing OSM features tagged
place=*, which represent countries, states, provinces, cities, towns, villages, neighborhoods, etc. Features in this layer are all Point geometries: they represent the approximate center of the feature.I added this layer partly because it's useful to me, but also because it's simple and therefore a good testbed for two new schema ideas:
populationcolumn is an integer. This is the first foray into parsing OSM's string-valued tags into other data types.namecolumn contains the value of that tag in OSM, which (usually) represents the name in the primary local language. But this layer also contains anamescolumn which is aMap<str, str>which contains all of the OSM tags whose keys start withname:, e.g.name:left,name:fr, and evenname:etymology:wikidata. Keys in the map are the tag key with thename:prefix dropped, and values are the OSM tag value. So for exampletags.names.escontains the value ofname:es(the Spanish language name), if available. I also addedalt_namesandofficial_namesmaps to complement thealt_nameandofficial_namecolumns.