-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix flatten implementation on objects #7059
Conversation
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.
In the issue, the skipTypedArray flag seems to have been set to true?
flatten(x, [], true)
, will your fix cover that use case?
Reviewable status: 0 of 1 approvals obtained (waiting on @ahmedsabie and @jinjingforever)
tfjs-core/src/util_test.ts
line 183 at r1 (raw file):
}); it('index signature', () => {
what is the use case for supporting number indexed map?
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.
It doesn't matter Int8Array is not part of our TypedArray definition
Reviewable status: 0 of 1 approvals obtained (waiting on @jinjingforever and @pyu10055)
tfjs-core/src/util_test.ts
line 183 at r1 (raw file):
Previously, pyu10055 (Ping Yu) wrote…
what is the use case for supporting number indexed map?
Int8Array and any other fixed size buffers/arrays all fit the index signature interface but do not pass Array.isArray.
Also it doesn't make sense to have RecursiveArray be defined as an index signature and then write a function flatten that's broken on non-array index signatures. That breaks the function type unless an error was meant to be thrown, but then RecursiveArray should not be defined as index signature.
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.
#6989
thanks for the explanation
Reviewable status: complete! 1 of 1 approvals obtained (waiting on @jinjingforever)
BUG Co-authored-by: Ping Yu <[email protected]>
Fixes #6989.
To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.
This change is