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

use admin_centre node for relation centroid where available #98

Merged
merged 1 commit into from
Jul 21, 2021

Conversation

missinglink
Copy link
Member

as per discussion in #96 some relations have a node member of role 'admin_centre' which we should favour over the computed geographic centroid.

@timonmasberg
Copy link

Wouldn't it be better if we check that the relation is of type "boundary":"administrative" (that's where admin_centre is of relevance) before we go through every member for every relation?

@missinglink missinglink force-pushed the relation_admin_centre branch from 6a632d1 to 565ed8d Compare July 21, 2021 10:18
@missinglink
Copy link
Member Author

Wouldn't it be better if we check that the relation is of type "boundary":"administrative" (that's where admin_centre is of relevance) before we go through every member for every relation?

It's really not very expensive since the members are already in memory and there's only one per immediate member (so one per ring), it probably only takes a fraction of a millisecond to compute.

That being said, if it's only relevant for boundary=administrative then we can only apply it on those, are we 100% sure that's correct? I had a look at the wiki and it seems logical.

@missinglink missinglink force-pushed the relation_admin_centre branch from 565ed8d to 2864121 Compare July 21, 2021 10:27
@missinglink
Copy link
Member Author

Added a boundary=administrative check via rebase.

@missinglink
Copy link
Member Author

missinglink commented Jul 21, 2021

go run ./... -tags 'official_status:de~Land' /data/osm/hamburg-latest.osm.pbf | jq .centroid

{
  "lat": "53.5437641",
  "lon": "10.0099133",
  "type": "admin_centre"
}

@missinglink missinglink force-pushed the relation_admin_centre branch from 2864121 to 3c00dd8 Compare July 21, 2021 15:01
@missinglink missinglink merged commit 92a450e into master Jul 21, 2021
@missinglink missinglink deleted the relation_admin_centre branch July 21, 2021 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants