Skip to content

Update flatbuffers to b99332e #47

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

Merged
merged 2 commits into from
Jan 8, 2019
Merged

Conversation

yongtang
Copy link
Member

@yongtang yongtang commented Jan 8, 2019

Since google/flatbuffers#5104 has been merged, it is possible to use flatbuffers directly without patch. This fix update the flatbuffers to the latest one.

This fix is related to #36.

Signed-off-by: Yong Tang [email protected]

Since flatbuffers/5104 has been merged, it is possible to use
flatbuffers directly without patch. This fix update the flatbuffers
to the latest one.

This fix is related to 36.

Signed-off-by: Yong Tang <[email protected]>
@yongtang
Copy link
Member Author

yongtang commented Jan 8, 2019

@BryanCutler I created this PR, as google/flatbuffers#5104 has been merged and there is no need to apply a fix any more.

With this PR, you can just add the following to flatbuffer_cc_library in arrow.BUILD. I think that will be all needed.

-    out_prefix = "cpp/src/arrow/ipc/"
+    out_prefix = "cpp/src/arrow/ipc/",
+    flatc_args = [
+        "--no-union-value-namespacing",
+        "--gen-object-api",
+    ],

@BryanCutler
Copy link
Member

Ok, will do. Thanks @yongtang !

Copy link
Member

@BryanCutler BryanCutler left a comment

Choose a reason for hiding this comment

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

It might be good to pip install the specific version of arrow we are using in case of a breaking change

@@ -9,3 +9,4 @@ chmod +x install.sh
rm -f install.sh
# Configure TensorFlow
./configure.sh
pip install pyarrow
Copy link
Member

Choose a reason for hiding this comment

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

should this be fixed to pyarrow==0.11.1?

@yongtang
Copy link
Member Author

yongtang commented Jan 8, 2019

Thanks @BryanCutler. I updated the pyarrow version. It looks like pyarrow does not support python 3.4, so skipped the install for 3.4.

Copy link
Member

@BryanCutler BryanCutler left a comment

Choose a reason for hiding this comment

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

LGTM

@BryanCutler
Copy link
Member

It looks like pyarrow does not support python 3.4, so skipped the install for 3.4.

Yeah, pyarrow stopped building for Python 3.4 since it is scheduled to be end-of-life in March this year. I should make the tests that require pyarrow optional, since we are testing against this version currently.

@yongtang yongtang merged commit 5c9e973 into tensorflow:master Jan 8, 2019
@yongtang yongtang deleted the flatbuffers branch January 8, 2019 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants