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

Added 2 failing feature tests around empty GeoJSON data structures #912

Closed
wants to merge 1 commit into from

Conversation

four43
Copy link

@four43 four43 commented Feb 26, 2019

These two added tests fail with the latest tag 4.2.1 of node-mapnik. From what I understand these are valid GeoJSON (as checked against https://geojson.io)

These two added tests fail with the latest tag 4.2.1 of node-mapnik. From what I understand these are valid GeoJSON (as checked against https://geojson.io)
@flippmoke
Copy link
Member

@artemp is this an issue in our parser in mapnik core?

@artemp artemp self-assigned this Mar 7, 2019
@artemp
Copy link
Member

artemp commented Mar 7, 2019

@four43 @flippmoke - {} is a valid JSON but is it a valid GeoJSON object?

https://tools.ietf.org/html/rfc7946

2.  GeoJSON Text

   A GeoJSON text is a JSON text and consists of a single GeoJSON
   object.

3.  GeoJSON Object

   A GeoJSON object represents a Geometry, Feature, or collection of
   Features.

   *  A GeoJSON object is a JSON object.

   *  A GeoJSON object has a member with the name "type".  The value of
      the member MUST be one of the GeoJSON types.

   *  A GeoJSON object MAY have a "bbox" member, the value of which MUST
      be a bounding box array (see Section 5).

   *  A GeoJSON object MAY have other members (see Section 6).
   

My reading (!) of the spec implies that while bbox and other members are optional, type is required. I think language is bit unfortunate here and I believe it has some historical background doh.

@four43
Copy link
Author

four43 commented Mar 9, 2019

@artemp thanks for weighing in on this. That's a good point and good referring to the spec. What do you think about the second test case in my pull request about the empty feature collection?

Thanks

@artemp
Copy link
Member

artemp commented Mar 12, 2019

@four43 - I think an empty collection is a valid GeoJSON and parser shouldn't fail but rather return an empty featureset

@artemp artemp added the bug label Mar 12, 2019
@artemp
Copy link
Member

artemp commented Mar 12, 2019

@four43 I created an issue mapnik/mapnik#4035 thanks for reporting!

@springmeyer
Copy link
Member

tracking at mapnik/mapnik#4035

@springmeyer springmeyer closed this Aug 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants