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

Update to mapnik 3.0.13 mason package for travis builds #143

Closed
springmeyer opened this issue Mar 17, 2017 · 8 comments
Closed

Update to mapnik 3.0.13 mason package for travis builds #143

springmeyer opened this issue Mar 17, 2017 · 8 comments

Comments

@springmeyer
Copy link
Member

mapbox/mason#365 is available now in mason. Next step is to point the python-mapnik builds at it. The goal here is to move to testing python-mapnik master against a stable mapnik version (rather than a moving target).

/cc @artemp @flippmoke

@springmeyer
Copy link
Member Author

Started this in https://github.com/mapnik/python-mapnik/tree/stable-upstream-3.0.13, however I'm blocked on:

src/mapnik_datasource.cpp:37:10: fatal error: 'mapnik/geometry/box2d.hpp' file not found
#include <mapnik/geometry/box2d.hpp>
         ^
1 error generated.

@artemp can you please either:

  1. branch to create a python-mapnik that works against Mapnik 3.0.x, or
  2. Ideally, fix master to work with both 3.1.x and 3.0.x like @flippmoke did at mapbox/mapnik-vector-tile@4ef9913

@artemp
Copy link
Member

artemp commented Mar 17, 2017

@springmeyer
Copy link
Member Author

Thanks @artemp for making a v3.0.13 tag and helping me notice the v3.0.x branch. I've re-applied the pinned mason package against the v3.0.x branch. All tests pass except one - #139 - do you know what changed?

@artemp
Copy link
Member

artemp commented Mar 20, 2017

All tests pass except one - #139 - do you know what changed?

According to https://tools.ietf.org/html/rfc7946#section-3.1.1 :

A position is an array of numbers.  There MUST be two or more
elements.  The first two elements are longitude and latitude, or
easting and northing, precisely in that order and using decimal
numbers.  Altitude or elevation MAY be included as an optional third
element.
"coordinates": [ [ [ [] ] ] ] 

The above is a valid JSON but not valid GeoJSON (as per spec ^)

/cc @springmeyer

@springmeyer
Copy link
Member Author

@artemp okay, so do the test expectations need updated? If so, can you push the correct update to master and the v3.0.x branch?

@artemp
Copy link
Member

artemp commented Mar 20, 2017

@springmeyer - ^ yes! This issue surfaced in node-mapnik mapnik/node-mapnik#729 but test coverage there is somewhat limited.

I'm going to make some changes to make handling empty geometries more consistent, because following GeoJSON should also throw invalid dataset exception:

{ "type": "Feature", "properties": { }, "geometry": { "type": "Point", "coordinates": [] }}

@artemp
Copy link
Member

artemp commented Mar 21, 2017

I'm working on this PR mapnik/mapnik#3643
/cc @springmeyer

@springmeyer
Copy link
Member Author

Original issue solved in #144

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

2 participants