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

Clarify DEVELOPMENT.md from a new dev perspective #622

Merged
merged 3 commits into from Oct 6, 2017

Conversation

nfelt
Copy link
Collaborator

@nfelt nfelt commented Oct 5, 2017

This clarifies some things that I encountered - in particular, it specifies that you'll probably want TensorFlow installed in a virtualenv, and it updates the advice about getting the nightly build now that tf-nightly is on pypi (https://pypi.python.org/pypi/tf-nightly). It also links to Bazel installation instructions just for the sake of being explicit.

This clarifies some things that I encountered - in particular, it specifies that you'll probably want TensorFlow installed in a virtualenv, and it updates the advice about getting the nightly build now that tf-nightly is on pypi (https://pypi.python.org/pypi/tf-nightly).  It also links to Bazel installation instructions just for the sake of being explicit.
@nfelt nfelt requested review from jart and chihuahua October 5, 2017 02:02
DEVELOPMENT.md Outdated
@@ -1,10 +1,10 @@
# How to Develop TensorBoard

TensorBoard at HEAD relies on the nightly installation of TensorFlow, so please install TensorFlow nightly for development. To install TensorFlow nightly, `pip install` the link to the [appropriate whl file listed at the TensorFlow repository](https://github.com/tensorflow/tensorflow).
TensorBoard at HEAD relies on the nightly installation of TensorFlow: this allows plugin authors to use the latest features of TensorFlow, but it means release versions of TensorFlow may not suffice for development. We recommend installing TensorFlow nightly in a [Python virtualenv](https://virtualenv.pypa.io), and then running your modified development copy of TensorBoard within that virtualenv. To install TensorFlow nightly within the virtualenv, you can simply run `pip install tf-nightly`.
Copy link
Contributor

Choose a reason for hiding this comment

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

How about: "To install TensorFlow nightly within a virtualenv, simply run:

virtualenv foo
source foo/bin/activate
pip install --upgrade pip
pip install tf-nightly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Also added prompt indicators for which commands should be in the virtualenv.

DEVELOPMENT.md Outdated

Running TensorBoard automatically asks Bazel to create a vulcanized HTML binary:
Running TensorBoard via Bazel will automatically create a vulcanized HTML python binary:
Copy link
Contributor

Choose a reason for hiding this comment

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

How about something like:

Running TensorBoard via Bazel will automatically "vulcanize" all the HTML files and generate a "binary" launcher script. When HTML is vulcanized, it means all the script tags and HTML imports are inlined into one big HTML file. Then the Bazel build puts that index.html file inside a static assets zip. The HTTP server then reads static assets from that zip while serving.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

PTAL, adopted most of your wording but combined it with the previous paragraph instead.

@nfelt
Copy link
Collaborator Author

nfelt commented Oct 5, 2017

PTAL

Copy link
Member

@chihuahua chihuahua left a comment

Choose a reason for hiding this comment

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

LGTM. Just 1 question.

```sh
$ virtualenv tf
$ source tf/bin/activate
(tf)$ pip install --upgrade pip
Copy link
Member

Choose a reason for hiding this comment

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

Why is upgrading pip needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was wondering that myself. @jart ?

Copy link
Member

Choose a reason for hiding this comment

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

Well, https://www.tensorflow.org/versions/r0.12/get_started/os_setup#cant_find_setuppy
I suppose this line can't hurt. Maybe @jart or someone found it necessary based on experience. A comment such as # Upgrade pip to generate necessary temporary files. might help.

Copy link
Contributor

@jart jart Oct 6, 2017

Choose a reason for hiding this comment

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

It's part of the TensorFlow Linux install documentation (search for easy_install -U pip and see cl/156379483).

Basically, you need pip ≥8.1 in order to install TensorFlow, due to some relatively recent whl packaging magic. But Ubuntu 14.04 uses pip 1.5.4 according to apt-cache show python-pip.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, that makes sense. Indeed, with pip 1.5.4 I get the following error:

$ pip install tf-nightly
Downloading/unpacking tf-nightly
  Could not find any downloads that satisfy the requirement tf-nightly
Cleaning up...
No distributions at all found for tf-nightly

@nfelt nfelt merged commit c2a8373 into master Oct 6, 2017
@nfelt nfelt deleted the nfelt-developmentmd-edits branch October 6, 2017 00:39
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.

None yet

3 participants