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

Polygon Definition lacks 1 more level of list #2

Closed
amcvega opened this issue May 7, 2014 · 3 comments · Fixed by #3
Closed

Polygon Definition lacks 1 more level of list #2

amcvega opened this issue May 7, 2014 · 3 comments · Fixed by #3
Assignees

Comments

@amcvega
Copy link

amcvega commented May 7, 2014

based on http://geojson.org/geojson-spec.html#polygon GeoPolygon lacks 1 more level of coordinates to make it an "array of LinearRing coordinate arrays" and compatible with the spec.

I rectified it with a quick fix (amcvega@8f0c4c0) in the GeoPolygon.hs. Tried making the tests pass again but got stumped by the bigAssFeatureCollectionJSON test.

Great work! This package is a life-saver.

@domdere
Copy link
Collaborator

domdere commented May 7, 2014

Ah ok, thanks, I'll look at it tonight :)

@domdere
Copy link
Collaborator

domdere commented May 7, 2014

I've started my fix in the polygon-fix branch I'll put the LinearRing type in the GeoPolygon and fix all the tests tomorrow night.

@domdere domdere mentioned this issue May 9, 2014
@domdere
Copy link
Collaborator

domdere commented May 9, 2014

I've got a fix in the referenced pull request, which I will merge in tomorrow if you dont see any problems with it :)

The LinearRing enforces the length of the "LinearRings" the spec describes. Later I will probably modify it to also compare the first and last element of the vector in the JSON Value and double check they are "equal", though I'm a bit hesitant to do that since the most common use cases is going to involve Doubles.

In any case, it wont affect the interface and when/if i do that it can be a minor point release.

I'll probably also do something similar with the Line objects to ensure that they are at least 2 elements long, further down the track.

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 a pull request may close this issue.

2 participants