Skip to content
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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions app/assets/javascripts/index/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,11 @@ OSM.Query = function (map) {
var tags = feature.tags;
var prefix = "";

if (tags.boundary === "administrative" && tags.admin_level) {
prefix = I18n.t("geocoder.search_osm_nominatim.admin_levels.level" + tags.admin_level, {
defaultValue: I18n.t("geocoder.search_osm_nominatim.prefix.boundary.administrative")
if (tags.boundary === "administrative" && (tags.border_type || tags.admin_level)) {
prefix = I18n.t("geocoder.search_osm_nominatim.border_types." + tags.border_type, {
defaultValue: I18n.t("geocoder.search_osm_nominatim.admin_levels.level" + tags.admin_level, {
defaultValue: I18n.t("geocoder.search_osm_nominatim.prefix.boundary.administrative")
})
});
} else {
var prefixes = I18n.t("geocoder.search_osm_nominatim.prefix");
Expand Down
6 changes: 5 additions & 1 deletion app/controllers/geocoder_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

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.

place.elements["extratags"].elements.each("tag") do |extratag|
prefix_name = t "geocoder.search_osm_nominatim.prefix.place.#{extratag.attributes['value']}", :default => prefix_name if extratag.attributes["key"] == "linked_place" || extratag.attributes["key"] == "place"
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"
Comment on lines +107 to +108
Copy link
Collaborator

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?

Copy link
Contributor Author

@1ec5 1ec5 Jan 13, 2025

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.

end
prefix_name = place_type || border_type || prefix_name
end
prefix = t ".prefix_format", :name => prefix_name
object_type = place.attributes["osm_type"]
Expand Down
47 changes: 37 additions & 10 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1489,16 +1489,43 @@ en:
weir: "Weir"
"yes": "Waterway"
admin_levels:
level2: "Country Boundary"
level3: "Region Boundary"
level4: "State Boundary"
level5: "Region Boundary"
level6: "County Boundary"
level7: "Municipality Boundary"
level8: "City Boundary"
level9: "Village Boundary"
level10: "Suburb Boundary"
level11: "Neighbourhood Boundary"
level2: "International Boundary"
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)"
Comment on lines +1493 to +1501
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those are longer prefixes, results often don't fit on one line and have line breaks in inconvenient places:

image

Copy link
Contributor Author

@1ec5 1ec5 Jan 12, 2025

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.

Copy link
Collaborator

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:

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;
}
}

Copy link
Collaborator

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.

Copy link
Contributor Author

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.

Copy link
Collaborator

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would the number be clearer than the locally common word for the type of boundary? Mappers who specialize in boundaries would know the local admin_level=* scale by heart, but others would find words more intuitive and would already know which are larger than others. Also, the Overpass results are ranked by size.

border_types:
arrondissement: "Arrondissement Boundary"
borough: "Borough Boundary"
cercle: "Cercle Boundary"
city: "City Boundary"
comarca: "Comarca Boundary"
county: "County Boundary"
departement: "Departmental Boundary"
department: "Departmental 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"
results:
no_results: "No results found"
more_results: "More results"
Expand Down
Loading