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
Get site ready to publish with minimal doc set #1909
Conversation
@@ -237,7 +301,7 @@ | |||
}, | |||
"cell_type": "markdown", | |||
"source": [ | |||
"<!-- <img class=\"tfo-display-only-on-site\" src=\"images/quickstart_model_fit.png\"/> -->" | |||
"<img class=\"tfo-display-only-on-site\" src=\"https://github.com/tensorflow/tensorboard/blob/master/docs/r2/images/quickstart_model_fit.png?raw=1\"/>" |
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.
This would make the image show up in the Colab notebook when it's running, right? So when the user runs the %tensorboard
cell, they will see both the live TensorBoard and the image. How do we avoid that?
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.
Unfortunately, that's correct, at least at our current state. I filed b/124892469 and b/124898762 to request support for this in our import script to publish to tensorflow.org. @MarkDaoust
Since %tensorboard
is not supported in the TF docker image, we can't use it t render out the cells on import, so that's the reason for saving outputs here—and removing the output cell from %tensorboard while uncommenting the image.
The notebook is rendered in three places: tensorflow.org, the github previewer, and colab (static and when re-run). This temp solution works for each of these except when Colab is re-run, which, as you note, creates the double view.
Basically what we need is some conditional logic that says, if in this notebook environment, show this, otherwise, show this. But that's just not available across the display environments we're targeting.
{ | ||
"output_type": "stream", | ||
"text": [ | ||
"\u001b[K 100% |████████████████████████████████| 79.1MB 325kB/s \n", |
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 standard practice for our docs to include these kinds of progress
bars in the notebook output? It seems like it could appear misleading:
if you load the notebook into a fresh Colab VM, you’ll see “100%
downloaded”, “trained 5 epochs”, etc. even though nothing has actually
happened.
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.
See note above.
No, we usually do not like to save notebooks with outputs. Usually, we execute the notebook during import with docs_to_devsite for display on tensorflow.org.
But we need to do it here because %tensorboard
is not supported in the TF docker images and we need to add additional functionality to docs_to_devsite to handle this. See: b/124892469 and b/124898762
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.
Not sure if this is a good data point but I 👍 based on observations from other notebooks like this:
https://www.tensorflow.org/tutorials/keras/basic_classification
Is it weird to show such executed outputs in verbatim?
Edit: Seems like it is a bad idea to include the outputs
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.
TF Probability prefers to save their notebooks with outputs: https://github.com/tensorflow/probability/blob/master/tensorflow_probability/examples/jupyter_notebooks/Probabilistic_PCA.ipynb
But, generally, we don't like saving notebooks with outputs because it makes the diffs more difficult and it bloats the repo with generated images.
But, in your case, we must to do it here because Colab seems to be the only place that renders your notebooks and we can't use that as part of our import script.
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.
I should note: If you want to selectively delete cells and then save with outputs, that's perfectly fine. That's what I'm doing with the %tensorboard output cell.
You are welcome to remove the pip install
output
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.
Great—thanks for the explanation.
Thank you! |
This PR gets the site in a publish-ready state, staged: go/tfo-stage/tensorboard
Creates r1/ and r2/ directories to make it clear where guides and tutorials go.
Notebooks ared save with outputs saved (except the tensorboard cells). The screenshot images are enabled. This way, the notebook will render fine, with outputs, on the tensorflow.org page. Notebooks are still runnable in Colab, but will look a little weird with the tensorboard UI followed by the screenshot. IT'll have to be this way until we add some features to the import script.
For now, the API docs are not generated and are removed from the lower tabs nav.
Companion to: cl/235870203