Turfjs / turf

A modular geospatial engine written in JavaScript and TypeScript
https://turfjs.org/
MIT License
9.05k stars 930 forks source link

turf.convex doesn't support single or colinear points #2449

Open martinhath opened 1 year ago

martinhath commented 1 year ago

This might be more of a feature request instead of a bug report, but I found the behaviour of turf.convex non-intuitive. I would expect something like this behaviour:

var singlePoint = turf.convex(turf.point([10, 45])); // == turf.point([10, 45])

var twoPoints = turf.featureCollection([
  turf.point([10, 45]),
  turf.point([11, 45]),
]);
var twoHull = turf.convex(twoPoints) // == turf.lineString([[10, 45], [11, 45]])  (or the other direction)

var colinearPoints = turf.featureCollection([
  turf.point([12, 45]),
  turf.point([10, 45]),
  turf.point([11, 45]),
]);
var colinearHull = turf.convex(colinearPoints) // == turf.lineString([[10, 45], [12, 45]])  (or the other direction)

My exact use-case is to use turf.convex followed by turf.buffer on a set of points, so I don't mind that the intermediate result isn't a polygon, since the final result will be. The two first cases are easy to handle (just check the number of input points), but the third one isn't as easy.

This would also potentially break code that assumes turf.convex will always return a Polygon or throw.

I'm currently at v6.5.0.

barbalex commented 3 months ago

Not only doesn't it support them: It errors:

TypeError: Cannot read properties of null (reading 'type')    at getGeom 

The fact that it errors is not apparent from the docs (https://turfjs.org/docs/#convex) where it says:

Takes a Feature or a FeatureCollection and returns a convex hull Polygon.

As a point is a feature, the fact that an error is returned, seems like a bug. After all, I would expect to get a polygon instead, going by the docs.

In any case it is not intuitive and not always easy to detect. In my case the error was not surfaced and the results of the computation were incomplete as points went missing.

EDIT:

It is not convex itself that errors. The error occurs in my case because convex returns null which does not have a geometry and so other turf functions following error.

So the bug is the fact that convex in some cases returns null while the docs declare that it returns a convex hull Polygon.

Anyway it is not intuitive and can be hard to debug. Though I do understand that returning a polygon for a point or straight line is not straightforward...

My way to deal with these issues is to buffer any feature collection by a millimetre before passing it into convex. That seems to work well and might be worth adding in the docs for cases where diverse feature collections are processed.

twelch commented 3 months ago

@barbalex can you share what is your input? Is the geometry type property defined for all features or is one null? Okay I see your edit.

twelch commented 3 months ago

It sounds like at the very least more clarity is needed in the typings and docs of the current function behavior.

Contributions are welcome if someone wants to look closely at the behavior of the function and propose changes.