Fix centroid calculation. Bump turf to v6.5 #7115
Conversation
|
Thanks for the PR. |
|
Found a bug in the centroid tests, that I've fixed (both single and multipolygon tests affected). In geojson, the first and last vertex of a polygon has the same coordinates, so the first or last element should be ignored in the centroid math. |
|
Please update the PR title and description to mention the bug fix. |
birkskyum
changed the title
src/lib/geo_location_utils.js
Outdated
| var turfArea = require('@turf/area'); | ||
| var turfCentroid = require('@turf/centroid'); | ||
| var turfBbox = require('@turf/bbox'); | ||
| var { area: turfArea } = require('@turf/area'); |
There was a problem hiding this comment.
Wondering how these imports are transpiled in to the dist?
May I ask you to test the dist from CI on the plotly.py side?
There was a problem hiding this comment.
Unless minified will they be kept as a destructuring assignment (plotly-geo.js bundle):
var {
area: turfArea
} = __webpack_require__(9532);
Not sure how to run it in python
|
How about just fixing the bug in this PR and bump to latest v6 packages and then make the move from v6 to v7 in a separate PR (which may require additional testing)? |
birkskyum
changed the title
|
It's rolled back to 6.5 now with the bugfix |
birkskyum
changed the title
birkskyum
changed the title
|
Only the usual suspects break, so I consider it CI Green |
|
Please open your new PR for v7. |
Update turf to v6.5.
This fixes a bug where the centroid calculation erroneously would include the first point of a polygon twice, thus giving it double weight. Single- and Multi-polygon tests are updates
Changelog