Skip to content

Conversation

@jameswex
Copy link
Contributor

@jameswex jameswex commented Jan 4, 2019

  • Motivation for features / changes

Adds colab implementation of WitWidget to enable use of What-If Tool directly in colab notebooks. This implementation differs by necessity from the Jupyter notebook widget.

  • Technical description of changes

Added Colab implemenation of WitWidget, which uses the colab-recommended way of passing information from python to JS and passing information from JS to python.

Added example colab notebook showing its use.

  • Screenshots of UI changes

wit-colab

  • Detailed steps to verify changes work correctly (as executed by you)

Run the colab notebook that is part of the PR and verify all What-If Tool features work correctly.

  • Alternate designs / implementations considered

N/A

@jameswex
Copy link
Contributor Author

jameswex commented Jan 4, 2019

@tolga-b Please take a look, thanks!

@tolga-b
Copy link
Contributor

tolga-b commented Jan 4, 2019

Thanks James, it looks great. You may want to add a direct link to colab in the readme as we discussed: https://colab.sandbox.google.com/github/jameswex/tensorboard/blob/colab/tensorboard/plugins/interactive_inference/WIT_in_colab.ipynb. Format is github/[repo-link]/blob/[branch-name]/[path-within-repo-to-file].

def infer_examples():
WitWidget.widget.infer()
output.register_callback('notebook.InferExamples', infer_examples)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think its suggested to have two spaces between module functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@jameswex
Copy link
Contributor Author

jameswex commented Jan 4, 2019

thanks tolga, added that direct link in the spot at the bottom and also another spot near the top of the readme where i mention colab. also updated widget to be able to handle multiple witwidgets in a single colab and tested it with two witwidgets.

Copy link
Contributor

@stephanwlee stephanwlee left a comment

Choose a reason for hiding this comment

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

Everything else look good



# HTML/javascript for the WIT frontend.
WIT_HTML = """
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be wonderful if this is created/went through compilation/Vulcanization and read as a string. Would it be possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it might be possible. I have a what-if tool tutorial I'm giving at a conference later this month and was hoping to have this mode up and running for it, and our team is just starting a two-week sprint on other ideas so I don't have much time to investigate right now.

Is it possible to commit this with the code in-line and create an issue to track moving it to a string read from a compiled html/js file in the future?

Copy link
Contributor

@tolga-b tolga-b left a comment

Choose a reason for hiding this comment

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

Thanks James, everything looks good to me.

@jameswex jameswex merged commit 280c5cc into tensorflow:master Jan 10, 2019
@jameswex jameswex deleted the colab branch January 10, 2019 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants