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

OSM timezone data contains LineStrings and GeometryGroups with LineStrings which breaks find. #90

Closed
despairblue opened this issue Apr 2, 2019 · 2 comments
Labels

Comments

@despairblue
Copy link

The data from OSM is not perfect so it contains sometimes nonsensical data like LineStrings. If it returns a LineString this find will through when we're trying to find out if a certain point is in a LineString:

> geoTz(-86, 0)
TypeError: Cannot read property '0' of undefined
    at inRing (/home/despairblue/git/node-geo-tz/node_modules/@turf/boolean-point-in-polygon/index.js:85:16)
    at booleanPointInPolygon (/home/despairblue/git/node-geo-tz/node_modules/@turf/boolean-point-in-polygon/index.js:56:13)
    at isPointinFeature (/home/despairblue/git/node-geo-tz/lib/find.js:187:13)
    at getTimezone (/home/despairblue/git/node-geo-tz/lib/find.js:197:13)

The data also contains GeometryCollections which the "@turf/boolean-point-in-polygon" also does not seem to handle well:

> geoTz(52.031192, 178.913872)
TypeError: Cannot read property 'length' of undefined
    at booleanPointInPolygon (/home/despairblue/git/node-geo-tz/node_modules/@turf/boolean-point-in-polygon/index.js:54:31)
    at isPointinFeature (/home/despairblue/git/node-geo-tz/lib/find.js:187:13)
    at getTimezone (/home/despairblue/git/node-geo-tz/lib/find.js:197:13)

I already have a fix for this. PR comes soon.

despairblue pushed a commit to despairblue/node-geo-tz that referenced this issue Apr 2, 2019
@evansiroky
Copy link
Owner

Thanks for this report. That's not good that there are non-polygon features in the lookup data. I appreciate the effort of #91, but I think that the source of the odd data is not from the OSM data (it's actually data derived from OSM via the timezone-boundary-builder project), rather it's from the data update script within this library.

It does not look like there are any LineString features in the output from timezone-boundary-builder. Therefore, I think a better fix for this would be to make sure that only Polygons and MultiPolygons are included within the updated data output of this library. Specifically, this function could be modified to normalize the resulting lookup data:

var unindexableAreaAnalyzingQueue = async.queue(
function (unindexableData, cb) {
var features = []
// calculate intersected area for each intersected zone
for (j = unindexableData.intersectedZones.length - 1; j >= 0; j--) {
var tzIdx = unindexableData.intersectedZones[j]
var intersectedGeoJson = getIntersectingGeojson(
tzIdx,
unindexableData.curBoundsGeometry
)
if (intersectedGeoJson) {
intersectedGeoJson.properties.tzid = data.timezones[tzIdx]
features.push(intersectedGeoJson)
}
}
var areaGeoJson = featureCollection(features)
var path = dataDir + '/' + unindexableData.curZone.id.replace(/\./g, '/')
fileWritingQueue.push(
{ folder: path, filename: 'geo.buf', data: areaGeoJson },
cb
)
},
10
)

This was referenced Apr 8, 2019
@evansiroky
Copy link
Owner

🎉 This issue has been resolved in version 5.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants