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

WKT for collection doesn't do much #227

Closed
remster opened this issue Dec 15, 2017 · 4 comments
Closed

WKT for collection doesn't do much #227

remster opened this issue Dec 15, 2017 · 4 comments
Assignees
Labels

Comments

@remster
Copy link

remster commented Dec 15, 2017

Here is:

		for sg := range g.Geometries() {
			s := WKT(sg)
			geometries = append(geometries, s)
		}

should be:

		for _, sg := range g.Geometries() {
			s := WKT(sg)
			geometries = append(geometries, s)
		}

Essentially, I think you have a bit of a design problem. By the way you have defined your tegola.Geometry interface, everything fulfills it, even number seven. And your code (e.g.: here) is quite lenient producing empty wkt. Perhaps you want to add a token aspect of the geometry to the interface, like: PointCount() or Area() or Intersection(thought that would be more involved)

@gdey gdey self-assigned this Dec 15, 2017
@gdey gdey added the bug label Dec 15, 2017
@gdey
Copy link
Member

gdey commented Dec 15, 2017

Thanks for this. This is a bug. The WKT was one of the first things I build and it was mainly to help debug things. I'm in the process of rewriting the WKB and WKT encoding libs. Yeah, the WKT(sg) function should have done a type check and returned an error. The issue is it returns an empty string when it does not recognize a type; instead of returning an error. https://github.com/terranodo/tegola/blob/master/wkb/wkt.go#L64
I'll create a quick fix for this, now.

With the go-spatial org we have been trying to figure out a better design. I'm in the middle move Tegola over to those, but yes there is a bit of an issue with geometries being just an interface{}. Though I don't have a better idea. One is to add an SRID() function to it - but I'm not sure.

gdey added a commit that referenced this issue Dec 15, 2017
When writing out WKT's of a geometry collection we were using the index,
instead of the geometry.
@gdey
Copy link
Member

gdey commented Dec 23, 2017

I'll am working through all wkb and wkt issues in the geom branch. Right now that has a better support for WKB in the tegola/geom/encoding/wkb directory. wkt support is still incomplete. Encode into WKT should work; I'm still working on decode as time allows.

ARolek added a commit that referenced this issue Dec 29, 2017
[issue-227] Quick fix for bug reported in #227
@ARolek
Copy link
Member

ARolek commented Dec 29, 2017

This fix is now merged into the v0.6.0 branch and will be part of the upcoming release. As @gdey mentioned, we're overhauling the wkb package which might be released with v0.6.0. We're also redefining the core geom types which the new wkb package will leverage. Thanks for the report!

@ARolek ARolek closed this as completed Dec 29, 2017
@remster
Copy link
Author

remster commented Jan 8, 2018

👍

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

No branches or pull requests

3 participants