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

delete duplicates from vertices before passing to voronoi #448

Closed
wants to merge 2 commits into from

Conversation

luke3141
Copy link

This fixes the problem where a scatter plot (as well as other types of plots that use scatter, including lines that use scatter to determine the mouse-over areas) generates an error for recent versions of d3 (after about 3.3.7) and where there are overlapping points.

The problem is that voronoi() requires the vertices that it's passed to not contain duplicates. (see: https://github.com/mbostock/d3/wiki/Voronoi-Geom#wiki-_voronoi).

This solves issues #330 and #368/#427. It may be a better fix than the latter as it fixes the underlying cause. I also found cases where 427 did not fix the problem.

@Filoni
Copy link

Filoni commented Feb 18, 2014

Works brilliantly, thanks for this!

@TheRiver
Copy link

I can confirm that this fixed the problem with duplicate points in line graphs for me. Thanks!

@mmacfadden
Copy link

Is there a plan to merge this into the head. It appears that several people are facing this issue.

@luke3141
Copy link
Author

@mmacfadden, the readme has been recently updated (see: https://github.com/novus/nvd3/blob/master/README.md) which talks about a refactoring of the code, and hence no pull requests are being accepted except on the new branch (refactor/2.0.0-dev).

I've checked, and the same problems still exist with the refactored code. Hence I'll endeavour to rebase this pull request sometime in the next week.

@DavidSouther
Copy link
Contributor

Closing in favor of #584

@ghost
Copy link

ghost commented Jan 22, 2015

Thank you. This was killing me and this patch fixed my issue. Hopefully it gets pulled into the main trunk.

@liquidpele liquidpele reopened this Mar 13, 2015
@liquidpele
Copy link
Contributor

I'm re-opening this just to track this concept... a better solution than #880 should be done, and this is probably part of that.. but we also need a way to get at the multiple point data on hover for the tooltips.

@liquidpele
Copy link
Contributor

This was actually done at some point by someone I think.

@liquidpele liquidpele closed this Apr 17, 2017
@AgDude
Copy link

AgDude commented Jan 14, 2020

This was implemented in #1934

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants