Skip to content

Fix scattergl selection#2267

Closed
dy wants to merge 8 commits into
masterfrom
reglscatter-fix
Closed

Fix scattergl selection#2267
dy wants to merge 8 commits into
masterfrom
reglscatter-fix

Conversation

@dy

@dy dy commented Jan 19, 2018

Copy link
Copy Markdown
Contributor

Fixes #2266

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Thanks @dfcreative - is there a test we can add that shows that this fixes the bug?

@dy

dy commented Jan 22, 2018

Copy link
Copy Markdown
Contributor Author

@alexcjohnson added selection test at 348d4d0

var x = [], y = [];
for(var i = 0; i < 2e4; i++) {
x.push(Math.random());
y.push(Math.random());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's probably an infinitesimal chance that this will cause any problems, but I still don't like having random calls in a test suite. If you really want something random-looking, as opposed to some simple formula, consider appropriating something reproducible like we used for box plot jitter

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @alexcjohnson! Fixed in 39be256

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Beautiful, thanks! 💃

@jackparmer

Copy link
Copy Markdown
Contributor

cc @cpsievert

@dy

dy commented Jan 23, 2018

Copy link
Copy Markdown
Contributor Author

Closed by cb2729e

@dy dy closed this Jan 23, 2018
@dy
dy deleted the reglscatter-fix branch January 23, 2018 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants