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

Fix missing isTypedArray when mixing versions of @tensorflow packages #7489

Merged

Conversation

mattsoulanille
Copy link
Member

@mattsoulanille mattsoulanille commented Mar 16, 2023

A new function, isTypedArray was added to the platform interface by #7181 and first published in tfjs-core 4.2.0. This made 4.2.0 incompatible with earlier versions of backends that implemented platform but did not include this new function, such as node and react-native. This change adds a fallback to the use of isTypedArray so earlier versions of platforms that don't implement isTypedArray will not throw an error.

Note that the fallback behavior may not be perfect, such as when running Jest tests in node. See #7175 for more details and upgrade all @tensorflow scoped packages to ^4.2.0 to avoid this.

Fixes #7273

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.


This change is Reviewable

A new function, `isTypedArray` was added to the `platform` interface by tensorflow#7181
and first published in tfjs-core 4.2.0. This made 4.2.0 incompatible with
earlier versions of backends that implemented `platform`, such as node and
react-native. This change adds a fallback to the use of `isTypedArray` so
earlier versions of platforms that don't implement `isTypedArray` will not throw
an error.

Note that the fallback behavior may not be perfect, such as when running Jest
tests in node. See tensorflow#7175 for more details and upgrade all @tensorflow scoped
packages to ^4.2.0 to avoid this.

Fixes tensorflow#7273
Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@mattsoulanille
Copy link
Member Author

@pyu10055 I had to update tfjs-node's jasmine to fix the new tests. This required updating a lot of existing tests, so I'm re-requesting your review.

Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 10 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

Copy link
Collaborator

@Linchenn Linchenn left a comment

Choose a reason for hiding this comment

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

Thanks for fixing! LGTM

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