-
Notifications
You must be signed in to change notification settings - Fork 927
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
Label administrative boundaries by real-world designation #5493
base: master
Are you sure you want to change the base?
Conversation
In search and query results, administrative boundaries are now labeled by the literal admin_level=* value instead of a word that could be confused with a legal designation.
config/locales/en.yml
Outdated
arrondissement: "Arrondissement Boundary" | ||
borough: "Borough Boundary" | ||
cercle: "Cercle Boundary" | ||
city: "City Boundary" | ||
comarca: "Comarca Boundary" | ||
county: "County Boundary" | ||
departement: "Department Boundary" | ||
department: "Department Boundary" | ||
district: "District Boundary" | ||
distrito: "District Boundary" | ||
freguesia: "Freguesia Boundary" | ||
local_authority: "Local Authority Boundary" | ||
municipality: "Municipal Boundary" | ||
municipi: "Municipal Boundary" | ||
município: "Municipal Boundary" | ||
nation: "International Boundary" | ||
national: "International Boundary" | ||
neighbourhood: "Neighborhood Boundary" | ||
parish: "Parish Boundary" | ||
province: "Provincial Boundary" | ||
região: "Regional Boundary" | ||
region: "Regional Boundary" | ||
state: "State Boundary" | ||
town: "Town Boundary" | ||
township: "Township Boundary" | ||
village: "Village Boundary" |
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.
For starters, I added a translatable string for each of the 30 or so border_type=*
values that occur most frequently with boundary=administrative
. Only one translatable string is provided per border_type=*
value globally. This will likely present a problem for some languages that would translate English words like “district”, “county”, or “town” differently depending on the country. I think we’ll need to add some more specific strings qualified by country code, as explained in #5450 (comment), but it’ll require a bit more research to find out which values are important in which countries.
1b9dcca
to
e7f1141
Compare
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'm not totally convinced showing the numeric level is useful but generally this looks good apart from the specific point I've mentioned in a comment.
@@ -101,9 +101,13 @@ def search_osm_nominatim | |||
if klass == "boundary" && type == "administrative" | |||
rank = (place.attributes["address_rank"].to_i + 1) / 2 | |||
prefix_name = t "geocoder.search_osm_nominatim.admin_levels.level#{rank}", :default => prefix_name | |||
border_type = false | |||
place_type = false |
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 think nil
would be a better initial value here given they are going to be strings if any match is found.
level3: "Administrative Boundary (Level 3)" | ||
level4: "Administrative Boundary (Level 4)" | ||
level5: "Administrative Boundary (Level 5)" | ||
level6: "Administrative Boundary (Level 6)" | ||
level7: "Administrative Boundary (Level 7)" | ||
level8: "Administrative Boundary (Level 8)" | ||
level9: "Administrative Boundary (Level 9)" | ||
level10: "Administrative Boundary (Level 10)" | ||
level11: "Administrative Boundary (Level 11)" |
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 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 noticed that other boundary types don’t say “Boundary”. For example, boundary=electoral
is just “Electoral”. We could shorten “Administrative Boundary (Level 4)” to just “Administrative (Level 4)”, but I’m ambivalent about similarly truncating the border_type=*
-based labels, such as “City Boundary”. That would easily get confused with the place=*
-based labels; “City” means something different according to that classification system.
Another option would be to drop the level suffix, since @tomhughes doesn’t seem enamored with including this information. To the extent that this introduces ambiguity in the search results, the solution would be to tag boundaries with border_type=*
, which is a good idea anyways.
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 noticed that other boundary types don’t say “Boundary”. For example, boundary=electoral is just “Electoral”.
Those are constructed here:
openstreetmap-website/app/assets/javascripts/index/query.js
Lines 91 to 100 in 5d76ec0
for (key in tags) { | |
value = tags[key]; | |
if (prefixes[key]) { | |
var first = value.slice(0, 1).toUpperCase(), | |
rest = value.slice(1).replace(/_/g, " "); | |
return first + rest; | |
} | |
} |
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 thought about using icons in search/query result lists, like on changeset/element pages. For administrative boundaries it could be something like a square with a border similar to the boundary line rendered on the map, maybe with a number inside for the level.
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.
Not a bad idea overall, though the numbers probably aren’t as intuitive for many mappers as designations coming from either border_type=*
or place=*
. I had forgotten about the code to transform an unrecognized tag into a label. As long as we have labels, we could fall back to a format string like “Border Type Boundary” if border_type=*
is present but set to an unrecognized value.
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.
Numbers tell how relatively big/important the area is, something that may not be clear otherwise.
border_type = t "geocoder.search_osm_nominatim.border_types.#{extratag.attributes['value']}", :default => prefix_name if extratag.attributes["key"] == "border_type" | ||
place_type = t "geocoder.search_osm_nominatim.prefix.place.#{extratag.attributes['value']}", :default => prefix_name if extratag.attributes["key"] == "linked_place" || extratag.attributes["key"] == "place" |
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.
Shouldn't :default
be border_type
and place_type
correspondingly, given that later the values are combined with place_type || border_type || prefix_name
?
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.
Yes, good catch. I originally had two loops that set prefix_name
directly, so your suggestion makes sense.
Search results for Nominatim and Overpass queries now label administrative boundaries according to the real-world designation according to the
border_type=*
tag. If the value of this tag is unrecognized, the label falls back to a more generic “Administrative Boundary (Level n)”, where n is theadmin_level=*
value. The English localization no longer attempts to translate these administrative levels to a fictional country’s administrative hierarchy.There are many tagging schemes for the real-world designation, each with their advantages and disadvantages, but I focused on
border_type=*
for now because it’s the most common scheme globally.Fixes #5450.