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

Implement empty_point for GeoJsonWriter #161

Merged
merged 3 commits into from
Aug 22, 2023

Conversation

Oreilles
Copy link
Contributor

According to the RFC, empty points may be represented with empty coordinates:

RFC 7946


3.1.  Geometry Object

   o  A GeoJSON Geometry object of any type other than "GeometryCollection" has a member with the name "coordinates". The value of the "coordinates" member is an array.  The structure of the elements in this array is determined by the type of geometry.  GeoJSON processors MAY interpret Geometry objects with empty "coordinates" arrays as null objects.

This PR therefore implements the GeometryProcessor method point_empty for GeojsonWriter by writing a point with empty coordinates.
For reference, PostGIS also has this behavior:

SELECT ST_AsGeoJSON('POINT EMPTY'::geometry);
--> {"type":"Point","coordinates":[]}

@michaelkirk
Copy link
Member

Could you add a test for this?

@Oreilles
Copy link
Contributor Author

Wkt -> GeoJSON test added. I wanted to add the reverse, but turns out the geojson create doesn't accept it:

https://github.com/georust/geojson/blob/4cfbf3a951cff12818e30c269606d46fb4c80486/src/util.rs#L211-L215

@michaelkirk
Copy link
Member

turns out the geojson create doesn't accept [empty points].

TBH I didn't realize geojson could represent empty points, but you've convinced me that it seems like a bug in the geojson crate. I've just opened georust/geojson#231

Copy link
Member

@michaelkirk michaelkirk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

@Oreilles
Copy link
Contributor Author

Ok, so after further inspection it seems like PostGIS is the one deviating from the spec here...

https://datatracker.ietf.org/doc/html/rfc7946#section-3.1.1

A position is an array of numbers.  There MUST be two or more
   elements.

@michaelkirk
Copy link
Member

michaelkirk commented Aug 13, 2023

heh i noticed the same thing, but I thought maybe it was just sloppy spec writing, and that the "null geometry" thing seemed to trump the "position" thing.

Probably we should be careful with this one, and get some others to weigh in.

@pka pka merged commit ab0cdae into georust:main Aug 22, 2023
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 this pull request may close these issues.

3 participants