-
Notifications
You must be signed in to change notification settings - Fork 307
Enable and Fix Arrow tests for TF 1.15.0 #519
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
Conversation
|
Fixes Arrow test failures from #431 |
e833998 to
78357a2
Compare
|
|
||
|
|
||
| class ArrowBaseDataset(data.Dataset): | ||
| class ArrowBaseDataset(dataset_ops.DatasetV2): |
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.
Is this ok to use as a base for 1.15.0?
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.
That should be OK I think. As long as dataset could be passed to tf.keras (with 1.15 and 2.0) we should be fine. Both DatasetV1 and DatasetV2 works with tf.keras. 👍
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.
LGTM. Thanks for the fix!
|
Actually it might be easier to PR against master, then cherry-pick or rebase in R0.8 instead. As the Google CLA will post some issues in R0.8 branch. Let me fix that. |
|
Cherry-picked to master and fixed the CLA issue. |
|
Thanks @yongtang ! |
This corrects API issues with TF 1.15.0 and fixes a bug in reading ArrowStreamDataset messages causing test failures. Arrow tests are now all enabled.