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
Replace d3-voronoi with d3-delaunay. #1594
Conversation
Thanks for the PR! The failing tests are due to the test scene for the airports example, where the voronoi polygon path strings have changed. I don't want to merge until we have a solution in place for d3/d3-delaunay#19. Vega can certainly encounter voronoi instances with less than 3 input points. |
Yeah, I agree that d3/d3-delaunay#19 is a blocker for us. I wonder whether @mourner or @mbostock have this issue on their queue. |
There is also d3/d3-delaunay#20 but it's less of an issue. |
Hmm, I think d3/d3-delaunay#20 is absolutely a blocker as well. I've seen Vega users generate collinear points for a Voronoi diagram to enable a form of "nearest" selection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need solutions for outstanding issues in d3-delaunay before this can be considered.
fe8655f
to
2438f34
Compare
d3/d3-delaunay#20 has been fixed |
@jheer I updated the PR and fixed any remaining issues. Can you rebuild the airport example? I tried it locally and tests are passing but the results differ from what's happening on Travis. |
Ahh, it's because of #1893 |
I updated to the latest version of d3-delaunay and fixed the build. This is ready to go (squashed). |
Why did you merge? I am working on this... Ugh. |
Ups, I took the approval as a permission to merge. I squashed all of the changes into one commit so hopefully it's not too confusing. |
Fixes #1492