search

perliedman / geojson-path-finder

Find shortest path through a network of GeoJSON
https://www.liedman.net/geojson-path-finder/
ISC License
301 stars 87 forks source link

capture missing edge data #30

Closed royhobbstn closed 6 years ago

royhobbstn commented 6 years ago

First of all, fantastic library!

I found a small issue that the library was not capturing all edge data.

As an example, for each route I was collecting the unique ID's from each road segment. I would then filter my overall network by the captured ID's in QGIS, and I found there were gaps.

before

Adding the line in this PR ensured that all edges were captured, and I no longer had any gaps.

after
perliedman commented 6 years ago

Hi, nice to hear you're finding use for the library!

Also, great that you found a nice way to spot this issue with QGIS. Obviously I haven't tested this thoroughly enough.

One thing though, this change currently break the tests: https://travis-ci.org/perliedman/geojson-path-finder/jobs/341102559 - from just a quick look, it might be that you're not actually testing if the edgeDataReduceFn options is actually set before calling it, but it might be something else.

If you could fix this, I would be happy to merge. Extra bonus points if you could add a unit test that breaks without your fix, otherwise I might go ahead and accidentally break this again in the future 😜

royhobbstn commented 6 years ago

In the course of fixing this, I have identified another bug where the edge previous to the initial vertices is being included in the edge data. I'll probably try to fix that too eventually (will file an issue sometime in the next few days illustrating this.)

perliedman commented 6 years ago

@royhobbstn nice to see this being put to the test. Looking forward to your report on the second issue.

Thanks!