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

Network.to_json should handle numpy.ndarray correctly #21073

Merged
merged 3 commits into from
Sep 10, 2018

Conversation

yanboliang
Copy link
Contributor

Pick the fix at keras-team/keras#10754 to tf.keras.

@caisq
Copy link
Contributor

caisq commented Aug 10, 2018

Can you review this please, @fchollet?

@caisq caisq added the awaiting review Pull request awaiting review label Aug 10, 2018
fchollet
fchollet previously approved these changes Aug 13, 2018
Copy link
Member

@fchollet fchollet 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 the PR.

@fchollet fchollet added awaiting testing (then merge) kokoro:force-run Tests on submitted change and removed awaiting review Pull request awaiting review labels Aug 13, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 13, 2018
@caisq caisq added awaiting testing (then merge) kokoro:force-run Tests on submitted change ready to pull PR ready for merge process and removed awaiting testing (then merge) labels Aug 14, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 14, 2018
@yanboliang
Copy link
Contributor Author

I fixed the bad indentation, can anyone trigger the build again? Thanks.

@caisq caisq added the kokoro:force-run Tests on submitted change label Aug 16, 2018
@caisq
Copy link
Contributor

caisq commented Aug 16, 2018

@yanboliang Done

@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 16, 2018
@yanboliang
Copy link
Contributor Author

@caisq I fixed the bad indentation again, can you re-trigger it? Thanks. BTW, do you know how I can trigger python lint checker locally before I submit the PR?

@caisq caisq added the kokoro:force-run Tests on submitted change label Aug 16, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Aug 16, 2018
@caisq
Copy link
Contributor

caisq commented Aug 16, 2018

@yanboliang With the necessary dependencies installed, you should be able to run it with this script (from the root directory of the repo): https://github.com/tensorflow/tensorflow/blob/master/tensorflow/tools/ci_build/ci_sanity.sh

@yanboliang
Copy link
Contributor Author

@caisq Great. I just run that script and verified this PR can pass sanity check locally. Can you trigger this test again? I'm sorry to bother you again. Thanks.

@tensorflowbutler
Copy link
Member

Nagging Assignee @caisq: It has been 14 days with no activity and this issue has an assignee. Please update the label and/or status accordingly.

@yanboliang
Copy link
Contributor Author

Gently ping @fchollet @caisq . Would mind to have a look at this? Thanks.

@caisq
Copy link
Contributor

caisq commented Sep 10, 2018

I think @fchollet has implicitly approved this PR when he applied the "awaiting test then merge" tag. I've applied the "ready to pull" tag, which should have triggered the merging process. let me try it again.

@caisq caisq added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Sep 10, 2018
@tensorflow-copybara tensorflow-copybara merged commit 4a1fdff into tensorflow:master Sep 10, 2018
tensorflow-copybara pushed a commit that referenced this pull request Sep 10, 2018
PiperOrigin-RevId: 212332139
@yanboliang
Copy link
Contributor Author

@caisq Thanks!

@yanboliang yanboliang deleted the to-json branch September 10, 2018 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes ready to pull PR ready for merge process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants