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

linestring centroids #31

Merged
merged 8 commits into from
Aug 4, 2016
Merged

linestring centroids #31

merged 8 commits into from
Aug 4, 2016

Conversation

missinglink
Copy link
Member

@missinglink missinglink commented Aug 4, 2016

this PR introduces a second centroid function for line strings, this is useful for things like roads and rail routes. see pelias/openstreetmap#72

it's easier to review the commits one-by-one

note: the method OSM uses to define polygons is a massive hack and so any way which does not have the same co-ordinate value for its first and last node will be considered a line, eg:

centroids

this is technically correct as ways must be closed to be considered polygons, the above image shows the two different centroid algorithms used for http://www.openstreetmap.org/way/28596356

this makes a big difference for rail networks because the centroid now actually lies on the line string:

rail

for all polygons (closed ways) the original sphere centroid calc is still used, I deleted it from this code base as it's now been merged in to the go.geo library.

// determine if the way is a closed centroid or a linestring
// by comparing first and last coordinates.
isClosed := false
if points.Length() > 2 {
  isClosed = points.First().Equals(points.Last())
}

// compute the centroid using one of two different algorithms
var compute *geo.Point
if isClosed {
  compute = GetPolygonCentroid(points)
} else {
  compute = GetLineCentroid(points)
}

resolves: pelias/openstreetmap#72

@orangejulius
Copy link
Member

Nice, LGTM

@dianashk
Copy link
Contributor

dianashk commented Aug 4, 2016

LGTM (small caveat: I don't really know Go)

@missinglink missinglink merged commit 8fe1117 into master Aug 4, 2016
@missinglink
Copy link
Member Author

$ npm publish
+ [email protected]

@orangejulius orangejulius deleted the line_centroid branch August 9, 2016 18:01
@riordan
Copy link

riordan commented Aug 10, 2016

looks go to me

god this is exciting!

@dianashk
Copy link
Contributor

👋 hi @riordan!

@riordan
Copy link

riordan commented Aug 10, 2016

I miss y'all!

On Aug 10 2016, at 5:54 pm, Diana Shkolnikov [email protected] wrote:

👋 hi @riordan!

You are receiving this because you were mentioned.
Reply to this email directly, view it on
GitHub
, or
[mute the thread](https://github.com/notifications/unsubscribe-auth/AAN4kPqh-P
WCGwjEtbPk9e7V5o7Wsvrcks5qekitgaJpZM4Jc5mZ).![](https://github.com/notificatio
ns/beacon/AAN4kH5NVMCkNK8ifhFWnm7KK21QWDLzks5qekitgaJpZM4Jc5mZ.gif)

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.

railroad centroids
4 participants