dartclub / turf_dart

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

[blocking] Port bbox-polygon package #97

Closed armantorkzaban closed 2 years ago

armantorkzaban commented 2 years ago

blocked center.dart's test

lukas-h commented 2 years ago

With what is this associated? #96? or with #91?

lukas-h commented 2 years ago

and what does center.dart have to do with it?

armantorkzaban commented 2 years ago

96 ports 'center', center's test uses bbox-polyline (implemented here: #98)

91 uses bbox in many of the booleans. Hence porting #96

armantorkzaban commented 2 years ago

I have left this out of the ported Dart: https://github.com/Turfjs/turf/blob/f2023aee26f50fa1fe804fba86be212a99b1a181/packages/turf-bbox-polygon/index.ts#L28

This I think is the corresponding test (which I will exclude as well): https://github.com/Turfjs/turf/blob/f2023aee26f50fa1fe804fba86be212a99b1a181/packages/turf-bbox-polygon/test.js#L27 What do you think?

armantorkzaban commented 2 years ago

It is kind of chicken and egg here. We need Bbox to write the test for bbox_polygon and center(a part of #96), which blocks our work on #96 is related to bbox_polygon. I would have first waited for the implementation here to be merged, then finished #96 and eventually written the test for bbox_polygon, but now, to push forward I implement the test as well, but test it at the end one more time.

lukas-h commented 2 years ago

We could go into the branch of #96 and just implement it there, if they all are interdependent!

It is kind of chicken and egg here. We need Bbox to write the test for bbox_polygon and center(a part of #96), which blocks our work on #96 is related to bbox_polygon.

armantorkzaban commented 2 years ago

That's also an approach. I thought it might get bigger of a task and get out of hand if we included this.

lukas-h commented 2 years ago

I thought it might get bigger of a task and get out of hand if we included this.

I think it‘s okay, because we are already shortly before merging, it‘s only about two interdependent unit tests