Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mbostock opened this issue Mar 5, 2020 · 3 comments · Fixed by #108
Closed

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

mbostock opened this issue Mar 5, 2020 · 3 comments · Fixed by #108

Comments

@mbostock
Copy link
Member

mbostock commented Mar 5, 2020

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
Copy link
Member

Fil commented Mar 5, 2020

Yes it's a problem. The README says "Returns an iterable over the polygons for each cell, in order" and was written after e641963, 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
Copy link
Member Author

mbostock commented Mar 6, 2020

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
Copy link
Member

Fil commented Mar 6, 2020

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.

Fil added a commit that referenced this issue May 28, 2020
…erty

fixes #106

(Also fixes the test case, in which the clipped cell had 0 point but was not null, resulting in [undefined, undefined] values.)
@Fil Fil closed this as completed in #108 May 28, 2020
Fil added a commit that referenced this issue May 28, 2020
…erty

fixes #106

(Also fixes the test case, in which the clipped cell had 0 point but was not null, resulting in [undefined, undefined] values.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants