d3 / d3-delaunay

Compute the Voronoi diagram of a set of two-dimensional points.
https://d3js.org/d3-delaunay
ISC License
611 stars 57 forks source link

voronoi.cellPolygons skips null cells, and doesn’t report the cell’s index. #106

Closed mbostock closed 4 years ago

mbostock commented 4 years ago

As a result, it’s not very useful: you can’t tell which cell polygon corresponds to which site.

Perhaps it could just assign cell.index on the yielded cell?

Fil commented 4 years ago

Yes it's a problem. The README says "Returns an iterable over the polygons for each cell, in order" and was written after https://github.com/d3/d3-delaunay/commit/e641963d112263f6117b51e742773c459cab62b9, so the situation is a bit fuzzy. I'm not sure why you would want to skip null polygons, but the cell.index solution is fine.

mbostock commented 4 years ago

Hmm. Yielding null might be fine, too?

(I’m also not really sure it’s useful that these methods are generators rather than returning an array or sparse array…)

Fil commented 4 years ago

Yielding null is fine IMO, but reverting to it might break someone's assumption (though not the documentation). I have no preference. Generators are nice to look at in observable, but I don't know if there's a practical use, since we can in any case use voronoi.cellPolygon(i) to enumerate.