[go] fix geometry parts build #355
Conversation
|
Thanks for your PR. I will try to review it by EOD today. |
|
LGTM. @bruno-ga you ok with merge? |
src/go/flatgeobuf_test.go
Outdated
| @@ -77,6 +77,9 @@ func TestCreateFGBFileAndBasicSearch(t *testing.T) { | |||
| createSquareFeature(-1, 0, 0, 1, 2), | |||
| createSquareFeature(-1, -1, 0, 0, 3), | |||
| createSquareFeature(0, -1, 1, 0, 4), | |||
| createMultiPolygonFeature([][]float64{ | |||
There was a problem hiding this comment.
Would you mind moving this to its own test? It seems important enough to deserve this (also it might not match the basic test here).
There was a problem hiding this comment.
Good point, I created another test
src/go/flatgeobuf_test.go
Outdated
| @@ -90,7 +93,7 @@ func TestCreateFGBFileAndBasicSearch(t *testing.T) { | |||
| header := writer.NewHeader(headerBuilder). | |||
| SetName("Households ShapeFile Data"). | |||
| SetTitle("Households ShapeFile Data"). | |||
| SetGeometryType(flattypes.GeometryTypePolygon) | |||
| SetGeometryType(flattypes.GeometryTypeUnknown) | |||
There was a problem hiding this comment.
If you move the changes to their own test, you do not need to use Unknown here (which again, goes for the "basic" part of this test). :)
src/go/flatgeobuf_test.go
Outdated
| @@ -137,6 +140,8 @@ func TestCreateFGBFileAndBasicSearch(t *testing.T) { | |||
| expectedResultProperties: []int{1, 2, 3, 4}}, | |||
| {searchMinX: 2, searchMinY: 2, searchMaxX: 3, searchMaxY: 3, | |||
| expectedResultProperties: []int{}}, | |||
| {searchMinX: 4.5, searchMinY: 4.5, searchMaxX: 4.6, searchMaxY: 4.6, | |||
| firstPartEnd := len(g.xy) | ||
| if firstPartEnd == 0 { | ||
| // Nothing to do. | ||
| return | ||
| } | ||
|
|
||
| if len(g.ends) != 0 { | ||
| firstPartEnd = int(g.ends[0]) | ||
| firstPartEnd = int(g.ends[0]) * 2 |
There was a problem hiding this comment.
Can you add a comment here to explain why you are multiplying by 2? It is not obvious form the code around it and it is also not clear to me why you need to do it (although I might be missing something).
There was a problem hiding this comment.
I added a comment. don't hesitate to tell me if it is not clear enough
Sorry, I punted reviewing it and then forgot about it. Just did it now. Will wait for the author's reply. @bjornharrtell BTW, heads up that I will start using my personal account to work on this, @brunoga (no dash). |
|
@brunoga thanks for your input and contribution. :) |
Thanks. :) |
|
@bjornharrtell Apparently I can not approve with either account so up to you. :) |
|
Thanks for your contribution @durandAD! |
First thank you for this great go package. When using this package I had a few issues while trying to write multipolygons and using parts of geometry.
I had 2 issues that are fixed by this PR.
I added a test to show what kind of feature was not working before