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

Valid geojson geometry fails to parse #2319

Closed
springmeyer opened this issue Jul 28, 2014 · 7 comments
Closed

Valid geojson geometry fails to parse #2319

springmeyer opened this issue Jul 28, 2014 · 7 comments
Assignees
Milestone

Comments

@springmeyer
Copy link
Member

This parses (as it should):

>>> mapnik.Path.from_geojson('{"type":"Point","coordinates":[-92.22568,38.59553]}').to_geojson()
'{"type":"Point","coordinates":[-92.22568,38.59553]}'

But this does not:

>>> mapnik.Path.from_geojson('{"coordinates":[-92.22568,38.59553],"type":"Point"}')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: Failed to parse geojson geometry
@artemp
Copy link
Member

artemp commented Jul 29, 2014

@springmeyer - yes, our geojson parser expects "type" before "coordinates" as it's greedy parser.
To fix this we need to a generic data structure to parse various "coordinates" versions first and then extract correct geometry representation based on "type". This will result in an extra memory allocation I was trying to avoid but looks like we need to handle it. On my TODO list.

springmeyer pushed a commit that referenced this issue Jul 29, 2014
@sgillies
Copy link

I'm late to the party here, but it's possible that my recent commit mapbox/mapnik-omnivore#17 might introduce Mapnik to more key-sorted GeoJSON (coordinates/geometry/features before type).

@springmeyer
Copy link
Member Author

Thanks for the heads up @sgillies - for now we should be safe because mapnik-omnivore currently uses OGR still to read GeoJSON files. Once this is fixed in Mapnik however we'll be safe to switch to this native approach (the goal being faster i/o than through OGR).

@artemp
Copy link
Member

artemp commented Aug 20, 2014

fixed in 3c99514

@artemp artemp closed this as completed Aug 20, 2014
@springmeyer
Copy link
Member Author

re-opening since this needs backported to 2.3.x.

@springmeyer
Copy link
Member Author

Assigning to 3.x until we determine its worth backporting.

@springmeyer springmeyer modified the milestones: Mapnik 2.3.0, Mapnik 3.x Aug 20, 2014
@springmeyer
Copy link
Member Author

closing, we now have tests in master, let's just run this out in 3.x for now.

springmeyer pushed a commit that referenced this issue Sep 23, 2014
Conflicts:
	plugins/input/geojson/geojson_datasource.cpp
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

No branches or pull requests

3 participants