Adds lineSlice method - Githubissues

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

Adds lineSlice method #158

Closed leiflinse-trivector closed 5 months ago

leiflinse-trivector commented 5 months ago

Issue: https://github.com/dartclub/turf_dart/issues/157

leiflinse-trivector commented 5 months ago

Rebased upon main

leiflinse-trivector commented 5 months ago

Rebased upon main (along returns full Feature now)
Changed parameters to lineSlice to be Feature instead of just Point and updated testes for that
I fixed the port where I inserted a guard for too short lines that shouldn't be there and caused the interpolation to fail in some cases.

I didn't have the time to retrofit changes due to main rebase back into the history so squashed it all together.

jsiedentop commented 5 months ago

Are you finished so that I can do a review?

leiflinse-trivector commented 5 months ago

I think it is ready for review.

I have a local test that uses along + lineSlice + length full circle and the accuracy isn't that great. (correct to 1%-level but not at 0,1%-level) But I read in turf.js some complaints about the accuracy of lineSlice so it might be related to that.

leiflinse-trivector commented 5 months ago

I can add that I did some testing with lineSlice, along and length that I wrote both in dart and in javascript to verify the port. So I am quite certain that the error-rate is from turf.js itself rather than the port.

lukas-h commented 5 months ago

@leiflinse-trivector I think this is good enough for a merge.

@jsiedentop any objections? looking forward to the review

leiflinse-trivector commented 5 months ago

@leiflinse-trivector I think this is good enough for a merge.

I agree.

jsiedentop commented 5 months ago

Since this project is a port of TurfJs, it is also legetim if we provide the same accuracy. Perhaps it would be good to mention the inaccuracy in the method description? We could also link to the TurfJs issue in the code.

@leiflinse-trivector was it TurfJs - lineSlice is very imprecise #2023 you were talking about?

lukas-h commented 5 months ago

merging this for now.

leiflinse-trivector commented 5 months ago

Since this project is a port of TurfJs, it is also legetim if we provide the same accuracy. Perhaps it would be good to mention the inaccuracy in the method description? We could also link to the TurfJs issue in the code.

@leiflinse-trivector was it TurfJs - lineSlice is very imprecise #2023 you were talking about?

Yes that is the complaints on turf.js that I was referring to. The tests I did myself have not been published there though.