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

histogram autobinning regression with TypedArray #3210

Closed
jonmmease opened this issue Nov 3, 2018 · 2 comments · Fixed by #3211
Closed

histogram autobinning regression with TypedArray #3210

jonmmease opened this issue Nov 3, 2018 · 2 comments · Fixed by #3211
Labels
regression this used to work

Comments

@jonmmease
Copy link
Contributor

It looks like the new autobinning logic has a regression with typed arrays. See plotly/plotly.py#1257.

CodePen: https://codepen.io/jonmmease/pen/mQdXJK?editors=1010

Standard array (1.42.2):
newplot-1

Typed array (1.42.2):
newplot-2

Typed array (1.41.3):
newplot-3

@alexcjohnson
Copy link
Collaborator

Wow that's weird - thanks @jonmmease ! No errors or anything, just renders incorrectly...

I'll fix (and test) this specific bug, but @etpinard I'm not sure what would make a good generic test of typed array support - in principle we could rerun the entire image test suite, combing through data (and layout for numeric arrays and converting them to typed arrays first... that seems pretty heavy but how else could we catch silent bugs like this?

@etpinard etpinard added the regression this used to work label Nov 5, 2018
@etpinard
Copy link
Contributor

etpinard commented Nov 5, 2018

that seems pretty heavy but how else could we catch silent bugs like this?

That does seems pretty heavy, unless we can add two more CircleCI test containers 💰

But maybe we could try to run all our mocks with typed arrays once locally and then lock failing cases (if any 😏 ) with jasmine tests.

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

Successfully merging a pull request may close this issue.

3 participants