-
-
Couldn't load subscription status.
- Fork 2.7k
V1.10.0 - Inject plotly.js into the output cell on every init_notebook_mode call
#469
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
Conversation
Fixes #458 Running init_notebook_mode injects plotly.js into the output cell. Before, if you ran it twice, it would clear the output cell on the second call and remove plotly.js from DOM. This was deceiving, because any existing plots on the page wouldn’t disappear even though their underlying plotly.js was no longer present. But, if you refresh the page, the plots would not render (as plotly.js was no longer in the output cell)
Full description of changes (from changelog):
Version 1.9.13 fixed an issue in offline mode where if you ran
init_notebook_mode
more than once the function would skip importing (because it saw that it had
already imported the library) but then accidentally clear plotly.js from the DOM.
This meant that if you raninit_notebook_modemore than once, your graphs would
not appear when you refreshed the page.
Version 1.9.13 solved this issue by injecting plotly.js with every singleiplotcall.
While this works, it also injects the library excessively, causing notebooks
to have multiple versions of plotly.js inline in the DOM, potentially making
notebooks with manyiplotcalls very large.
Version 1.10.0 brings back the requirement to callinit_notebook_modebefore
making aniplotcall. It makesinit_notebook_modeidempotent: you can call
it multiple times without worrying about losing your plots on refresh.
init_notebook_mode call (削除ここまで)init_notebook_mode call (追記ここまで)
cc @yankev @theengineear @mdtusz @BRONSOLO @Kully @aggFTW
This fixes #458 in a slightly different way than version 1.9.13 proposed. This change fixes an issue in 1.9.13 introduced where plotly.js is injected into the DOM excessively.
This is the notebook that I am using for QA. Unfortunately, this stuff is still hard to catch in our automated tests. https://gist.github.com/chriddyp/170f46b36a9a3be7caee96eb3d4118bc - @yankev - could you make sure that this works as intended? @mdtusz - can we add a test that ensures that plotly.js isn't injected multiple times?
@aggFTW
aggFTW
May 19, 2016
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.
You are missing
if not __PLOTLY_OFFLINE_INITIALIZED:
no?
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'm not actually, that's the heart of the fix. Before, we had this check:
if not __PLOTLY_OFFLINE_INITIALIZED:
return
which meant that if you ran this function twice, we return None the second time which cleared the output cell (which previously had plotly.js in it). Now, we just inject plotly.js into the output cell every time.
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.
*__* oh, sweet! sorry, def confused by that on the first read.
💃 for the change after we hear back from @yankev on #469 (comment)
aggFTW
commented
May 19, 2016
What we do is we use plotly inside of an Output ipywidget. Can you check in your notebook that this still works after this change? Widgets are not persisted to the output fields.
Basically, add this to your test notebook code as a use case:
from ipywidgets import Output
from IPython.core.display import display
from plotly.offline import init_notebook_mode
from plotly.graph_objs import Pie, Figure, Data
from plotly.offline import iplot
o = Output()
display(o)
with o:
print("I'm not persisted to output area!")
init_notebook_mode()
data = [Pie(values=[1,2,3])]
fig = Figure(data=Data(data))
iplot(fig, show_link=False)
@aggFTW so it seems that the Output widget persists, but the text and the pie chart don't after a refresh. Were the text and charts persisting in a previous version of plotly.py?
aggFTW
commented
May 19, 2016
No, it has never persisted. When plotly was broken, a refresh would show an exception though. We should avoid that behavior.
@yankev sounds like we're good. can you fix or update the tests and deploy?
@chriddyp yep got it
Uh oh!
There was an error while loading. Please reload this page.
No description provided.