Scope extent in zoom calculations to avoid error

[davidtheclark]

I think this may be connected to #142, and may illustrate an incomplete resolution to #88.

As far as I can tell, this new fixture is valid GeoJSON, but when we try to get its min/max zoom, we end up hitting the error here:

mapnik-omnivore/lib/utils.js

Line 27 in 805d115

if (tiles <= 0) {

[davidtheclark]

I added a function that normalizes longitudes by ensuring that they fall within the +/-180 range. This fixes the failing test case and doesn't cause any other tests to fail.

[coveralls]

Coverage increased (+0.04%) to 94.14% when pulling 6aec23d on bounds-invalid into 6d54895 on master.

[coveralls]

Coverage increased (+0.04%) to 94.14% when pulling 6aec23d on bounds-invalid into 6d54895 on master.

[coveralls]

Coverage increased (+0.04%) to 94.14% when pulling 6aec23d on bounds-invalid into 6d54895 on master.

[coveralls]

Coverage increased (+0.04%) to 94.14% when pulling 6aec23d on bounds-invalid into 6d54895 on master.

[davidtheclark]

@rclark Just pinging you so this is on your radar. We talked about it anyway. BTW: Do you think Coveralls has anything to say about it?

[rclark]

💔 coveralls

[davidtheclark]

I suspect that the minzoom and maxzoom numbers I'm getting for the new tests are wrong. I'm not sure, though, and am not sure how to know. So I could use some guidance here from anybody more experienced.

[rclark]

You zoom ranges do not look wrong:

• the files are very small, so all the data safely fits in a z0 tile -- minzoom should be 0
• the max zoom of 6 is the minimum allowable max zoom, which also fits since your data is not very detailed -- overzooming (looking at z6-simplified data at z >6) will have very little effect

What zoom ranges do you get for these tests without your code change?

[davidtheclark]

Ok, thanks! Without the code changes we get errors thrown instead of zoom ranges.

[coveralls]

Coverage increased (+0.04%) to 94.14% when pulling 3dacc2e on bounds-invalid into 6d54895 on master.

[davidtheclark]

I changed the way the extent gets "normalized" so that it doesn't bother trying to re-scope the longitudes, just stops them at +/-180.

@rclark What do you think about merging this? (Or is there someone else I should ping?) The alternative would be change something upstream so that the end user of an upload gets an error that's more meaningful to them than Error calculating min/max zoom: Bounds invalid.

[coveralls]

Coverage increased (+0.04%) to 94.14% when pulling 54b39cc on bounds-invalid into 6d54895 on master.

[davidtheclark]

I'm going to close this and open another, more focused and directed issue/ticket.