Skip to content
This repository has been archived by the owner on Dec 21, 2023. It is now read-only.

Support notebook target #2410

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tapaswenipathak
Copy link
Contributor

Fix #2368.

Should I write a test?

Copy link
Contributor

@znation znation left a comment

Choose a reason for hiding this comment

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

Hi @tapaswenipathak, this isn't the right fix for #2368. Sorry about that, I should have given more details in that issue about what it meant.

In this PR, you are adding a new target, notebook, for all visualizations -- this target has the same semantics as auto when the user is in a notebook. I don't think this is something we want to do in general, since unlike other targets (like gui or browser), the ability to render inline in notebook is dependent on the user's environment, so I figured only auto makes sense for that case. It's by design that only auto is capable of rendering inline in a notebook.

In #2368, we are wanting the explore method on the Evaluation object (returned by the evaluate method in image classifier and drawing classifier) to render its interactive UI inline in the notebook. This will require quite a bit more work for the backend to talk to the notebook UI over the web server, and for the web based UI to render inline without causing page-wide issues.

I wouldn't consider this an approachable issue unless you're familiar with how the two frontend communication protocols work in Turi Create (Unix pipe vs. http to localhost), how to build and test the web front end pieces, etc. Let me know if you would like for me to suggest a more approachable issue to start with.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Evaluation.explore should support notebook target
2 participants