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

WKB is missing collections support #64

Closed
nicerobot opened this issue Apr 23, 2017 · 6 comments
Closed

WKB is missing collections support #64

nicerobot opened this issue Apr 23, 2017 · 6 comments
Assignees

Comments

@nicerobot
Copy link

This WKB

0107000000030000000101000000550B36BFABD753C07CE58B07A5D24540010200000005000000C4190D2ABBD753C028AA6D799BD24540B51F84DBB5D753C07954A1269FD24540F9DB9E20B1D753C0E3D011AFA1D24540FB8E86F8ACD753C0ED444948A4D24540550B36BFABD753C07CE58B07A5D24540010200000002000000550B36BFABD753C07CE58B07A5D24540F6D786E5AAD753C0C619C39CA0D24540

is

GEOMETRYCOLLECTION(POINT(-79.3698576 43.6456613),LINESTRING(-79.3707986 43.6453697,-79.3704747 43.6454819,-79.370186 43.6455592,-79.3699323 43.6456385,-79.3698576 43.6456613),LINESTRING(-79.3698576 43.6456613,-79.3698057 43.6455265))

But fails with

wkb: unsupported type: 7
@nicerobot nicerobot changed the title Missing collections support WKB is missing collections support Apr 23, 2017
@twpayne
Copy link
Owner

twpayne commented Apr 23, 2017

Thanks for the report!

go-geom currently does not support geometry collections. The reason is that they behave very differently to every other type of geometry, and I haven't found a good way to implement them within the constraints of Go's type system.

All ideas for how geometry collections could be implemented are welcome :)

@nicerobot
Copy link
Author

nicerobot commented Apr 23, 2017

It seems to me []geom.T would handle it where collections are themselves a geom.T?

@twpayne
Copy link
Owner

twpayne commented Apr 23, 2017

Agree. A (slightly) tricky aspect is: should geometry collections themselves implement the geom.T interface? If so, how?

The last time I read the OGC spec, I didn't spot anything that excluded geometry collections from containing nested geometry collections. So, this means that geometry collections have to implement the geom.T interface. However, geom.T includes methods to access the underlying flat coordinates slice, layout, and stride, but there is no requirement that the geometries in a geometry collection even have the same layout or stride, and the code to handle this variation while packing all the coordinates in a geometry collection into the same flat coordinates array will be quite tricky to write.

An alternative would be to implement geometry collections as a special case of geom.T, i.e. stub out the normal geom.T methods to return nil or panic with an error, but this will mean littering any code that handles geometry collections with type assertions.

I suspect that the second approach is the right one. I'll try typing up some code to see how it looks. Feel free to propose something too :)

@nicerobot
Copy link
Author

I was thinking of the special-case geom.T for collections. I think it'll be a little unfortunate to clutter the code with special cases but I do think that respects the OGC spec since collections are different than geometries.

Also, right now, i'm just unmarshaling for marshaling to geojson so i don't care about any other operations on the collection.

@twpayne
Copy link
Owner

twpayne commented Apr 23, 2017

I put some initial work-in-progress code here:
https://github.com/twpayne/go-geom/compare/geometry-collection?expand=1
It doesn't pass the tests yet, but is a rough idea of what I had in mind.

@nicerobot
Copy link
Author

Very nice. I commented on SetSRID.
Let me know if/when it's testable and i'll give it a spin (probably not right away but I do have a test case waiting for testing when I can get back to it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants