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 TypedArray support #3211

Merged
merged 4 commits into from
Nov 5, 2018
Merged

Histogram TypedArray support #3211

merged 4 commits into from
Nov 5, 2018

Conversation

alexcjohnson
Copy link
Collaborator

Fixes #3210

I rearranged Lib array utilities into one file (lib/array) and added one new one: Lib.concat, a clone of Array.concat that works with TypedArrays and mixed types.

For now I only used it in one place, where it was needed to fix the histogram bug.

@alexcjohnson
Copy link
Collaborator Author

@etpinard how do I avoid making this mistake with package-lock again? Is there a way to ensure that these optional flags don't get wiped out when I npm i?

var concat = Lib.concat;
it('works with multiple Arrays', function() {
expect(concat([1], [[2], 3], [{a: 4}, 5, 6]))
.toEqual([1, [2], 3, {a: 4}, 5, 6]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Lib.concat is fantastic 🎉

Could we spyOn(Array.prototype, 'concat') in this test-case to make sure it is called in the "all regular arrays" case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good call - 1ca72c3
toEqual calls Array.concat profusely, so I had to rearrange a little to get this to work right...

@etpinard
Copy link
Contributor

etpinard commented Nov 5, 2018

how do I avoid making this mistake with package-lock again? Is there a way to ensure that these optional flags don't get wiped out when I npm i?

I'm voting for reverting all optional: true additions -> #3184 (comment)

@alexcjohnson alexcjohnson mentioned this pull request Nov 5, 2018
@etpinard
Copy link
Contributor

etpinard commented Nov 5, 2018

Nicely done 💃

@alexcjohnson alexcjohnson merged commit 03a12e0 into master Nov 5, 2018
@alexcjohnson alexcjohnson deleted the hist-typedarray branch November 5, 2018 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

histogram autobinning regression with TypedArray
2 participants