{"action":"created","issue":{"url":"https://api.github.com/repos/paulmach/go.geo/issues/68","repository_url":"https://api.github.com/repos/paulmach/go.geo","labels_url":"https://api.github.com/repos/paulmach/go.geo/issues/68/labels{/name}","comments_url":"https://api.github.com/repos/paulmach/go.geo/issues/68/comments","events_url":"https://api.github.com/repos/paulmach/go.geo/issues/68/events","html_url":"https://github.com/paulmach/go.geo/issues/68","id":337480645,"node_id":"MDU6SXNzdWUzMzc0ODA2NDU=","number":68,"title":"note about computing centroid for polygons","user":{"login":"missinglink","id":738069,"node_id":"MDQ6VXNlcjczODA2OQ==","avatar_url":"https://avatars2.githubusercontent.com/u/738069?v=4","gravatar_id":"","url":"https://api.github.com/users/missinglink","html_url":"https://github.com/missinglink","followers_url":"https://api.github.com/users/missinglink/followers","following_url":"https://api.github.com/users/missinglink/following{/other_user}","gists_url":"https://api.github.com/users/missinglink/gists{/gist_id}","starred_url":"https://api.github.com/users/missinglink/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/missinglink/subscriptions","organizations_url":"https://api.github.com/users/missinglink/orgs","repos_url":"https://api.github.com/users/missinglink/repos","events_url":"https://api.github.com/users/missinglink/events{/privacy}","received_events_url":"https://api.github.com/users/missinglink/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-07-02T11:31:45Z","updated_at":"2018-08-29T19:47:22Z","closed_at":"2018-08-29T19:47:22Z","author_association":"CONTRIBUTOR","body":"hi @paulmach \r\n\r\nI got bit by a gotcha today, which although is technically correct, could cause confusion for some users.\r\n\r\nWhen using the `PointSet.Centroid()` and `PointSet.GeoCentroid()` methods on a polygon, the calculated result was incorrect, for instance `[1 1, 1 -1, -1 -1, -1 1, 1, 1]` was not returning `[0 0]`.\r\n\r\nMy error was that I failed to remove the last coordinate from the polygon (a duplicate of the first), which is a common convention to indicate that the polygon is closed (a convention in openstreetmap).\r\n\r\nthe fix was fairly simple:\r\n\r\n```\r\nisClosed = points.First().Equals(points.Last())\r\nif isClosed {\r\n  points.RemoveAt(len(*points) - 1)\r\n}\r\n```\r\n\r\nThe current implementation of `numPoints := float64(len(ps))` is technically correct, but could possibly also bite others like me who aren't careful about it's usage.\r\n\r\nHow would you feel about me changing these calculations in `PointSet` to deduplicate the points before computing `numPoints`? This would likely slow down the code and require some memory to do the deduplication.\r\n\r\nOther options include:\r\n- Just dedupe the first and last points\r\n- Add a `Polygon` type which has polygon specific code\r\n- Write a few words about it in the code comments\r\n- Not worry about it and hope that other people aren't big dummies like me :)\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/paulmach/go.geo/issues/comments/417081689","html_url":"https://github.com/paulmach/go.geo/issues/68#issuecomment-417081689","issue_url":"https://api.github.com/repos/paulmach/go.geo/issues/68","id":417081689,"node_id":"MDEyOklzc3VlQ29tbWVudDQxNzA4MTY4OQ==","user":{"login":"paulmach","id":366320,"node_id":"MDQ6VXNlcjM2NjMyMA==","avatar_url":"https://avatars3.githubusercontent.com/u/366320?v=4","gravatar_id":"","url":"https://api.github.com/users/paulmach","html_url":"https://github.com/paulmach","followers_url":"https://api.github.com/users/paulmach/followers","following_url":"https://api.github.com/users/paulmach/following{/other_user}","gists_url":"https://api.github.com/users/paulmach/gists{/gist_id}","starred_url":"https://api.github.com/users/paulmach/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/paulmach/subscriptions","organizations_url":"https://api.github.com/users/paulmach/orgs","repos_url":"https://api.github.com/users/paulmach/repos","events_url":"https://api.github.com/users/paulmach/events{/privacy}","received_events_url":"https://api.github.com/users/paulmach/received_events","type":"User","site_admin":false},"created_at":"2018-08-29T19:47:22Z","updated_at":"2018-08-29T19:47:22Z","author_association":"OWNER","body":"hi @missinglink sorry for the late reply. \r\n\r\nMy feeling is the correct answer is \"Add a Polygon type which has polygon specific code\". If you're converting a polygon to a pointset it's up to the program to handle the edge cases.\r\n\r\nThe conventions in this library make it hard to add another type. I created a new library https://github.com/paulmach/orb a while back that does thing much better (in both a geo and go way) and I think handles this case much more cleanly. "}}
{"actor":{"display_login":"paulmach"}}