search

dartclub / turf_dart

A turf.js-like geospatial analysis library working with GeoJSON, written in pure Dart.
https://pub.dev/packages/turf
MIT License
62 stars 28 forks source link

Implement length and along #153

Closed leiflinse-trivector closed 5 months ago

leiflinse-trivector commented 6 months ago

Implements length() and along() by porting from turf.js.

Tests are included and the along() test uses length() so therefore I created one PR instead of two.

One difference to turf.js is that for along() the dart implementation has a guard to return first point in case the provided distance is negative, while the turf.js implementation will return a different point than the start.

Related issue: https://github.com/dartclub/turf_dart/issues/152

lukas-h commented 5 months ago

@leiflinse-trivector thanks for the contributions. We will shortly publish this with a new minor release on pub.dev

leiflinse-trivector commented 5 months ago

Thank you.

There was an error related to the format of a test, so I auto-formatted it and pushed to the PR.

leiflinse-trivector commented 5 months ago

One question, I made along return null if the line has no coordinates, but maybe that should have been an Exception?

lukas-h commented 5 months ago

One question, I made along return null if the line has no coordinates, but maybe that should have been an Exception?

I think both behaviours are fine, I don't have a preference here. What does the JS-Version do?

leiflinse-trivector commented 5 months ago

I will look into what turf.js does.

I also want to revision that I used a pure LineString as input while turf.js uses a Feature, which after using the library more in the recent time, I realize it works better to take a full Feature and do the unpacking in turf_dart functions as it makes it easier to consistently use the library with full features to be able to carry over properties where it makes sense.

leiflinse-trivector commented 5 months ago

Also realized I could make use of internal helper getCoords in places where I didn't.

leiflinse-trivector commented 5 months ago

I have now got the PR in line with the behaviour that upstream turf.js wants when distance is negative. (counting from back) A PR for upstream is open: https://github.com/Turfjs/turf/pull/2573

And changed so line parameter is a Feature and got both length() and along() converted to no longer return nullable return values and instead throw in case of error.

leiflinse-trivector commented 5 months ago

Thank you very much for your contribution. I have made a few small comments. Also, a unit test fails for me. Is it successful for you?

Thank you for the review. I believe I have addressed all your points now.

leiflinse-trivector commented 5 months ago

Thanks, and I realize I messed up the formatting of length_test.dart that fails the format test in main. I will enable auto format on save for future work.