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

Recommend running converter pip in virtualenv #364

Merged
merged 6 commits into from May 22, 2019

Conversation

Projects
None yet
2 participants
@sdll
Copy link
Contributor

commented May 21, 2019

Resolves tensorflow/tfjs#1546.


This change is Reviewable

@pyu10055
Copy link
Collaborator

left a comment

Thank you for the contribution. Couple small comments.

Reviewed 2 of 2 files at r1.
Reviewable status: 0 of 1 approvals obtained (waiting on @sdll)


README.md, line 16 at r1 (raw file):

## Step 1: Converting a [SavedModel](https://github.com/tensorflow/tensorflow/blob/master/tensorflow/python/saved_model/README.md), [Keras h5](https://keras.io/getting-started/faq/#how-can-i-save-a-keras-model), [tf.keras SavedModel](https://www.tensorflow.org/api_docs/python/tf/contrib/saved_model/save_keras_model) or [TensorFlow Hub module](https://www.tensorflow.org/hub/) to a web-friendly format

0. Please make sure that you run in a Docker container or a virtual environment. The script pulls its own subset of TensorFlow,which might conflict with the existing TensorFlow/Keras installation.

add a space after 'TensorFlow,'


README.md, line 18 at r1 (raw file):

0. Please make sure that you run in a Docker container or a virtual environment. The script pulls its own subset of TensorFlow,which might conflict with the existing TensorFlow/Keras installation.

For example, create and activate a `venv` virtual environment in your current folder:

add a link to the installation guide of virtualenv.


README.md, line 79 at r1 (raw file):

| Positional Arguments | Description                                                                                                                  |
| -------------------- | ---------------------------------------------------------------------------------------------------------------------------- |

are these table header changes needed?

@pyu10055
Copy link
Collaborator

left a comment

Reviewable status: 0 of 1 approvals obtained (waiting on @sdll)


python/setup.py, line 22 at r1 (raw file):

from pip._vendor import pkg_resources

def _is_correct_tensorflow_installed():

Looks like there is another PR that checks the pkg version #365

sdll added some commits May 22, 2019

@sdll sdll force-pushed the sdll:devx/use-venv branch from 38f9012 to 7795106 May 22, 2019

@sdll

This comment has been minimized.

Copy link
Contributor Author

commented May 22, 2019

Reverted the commits in favour of #365. Might be prudent to close this PR and add the doc changes there instead.

@sdll sdll force-pushed the sdll:devx/use-venv branch from a1678f0 to d5d4227 May 22, 2019

@pyu10055 pyu10055 merged commit 38922a4 into tensorflow:master May 22, 2019

2 checks passed

Trigger: ab61216a-f608-42f5-8bae-c5628a645517 Summary
Details
cla/google All necessary CLAs are signed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.