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

Plotly plots show locally but not on RTD #4367

Closed
astrojuanlu opened this issue Jul 13, 2018 · 28 comments
Closed

Plotly plots show locally but not on RTD #4367

astrojuanlu opened this issue Jul 13, 2018 · 28 comments
Assignees
Labels

Comments

@astrojuanlu
Copy link

@astrojuanlu astrojuanlu commented Jul 13, 2018

Details

Expected Result

I expected the Plotly plots to show in RTD the same way as locally.

Actual Result

Because of readthedocs/sphinx_rtd_theme#328, I am using a hack, which is adding require.js at the top of the page from a notebook cell:

from IPython.display import HTML
HTML('<script type="text/javascript" src="https://cdnjs.cloudflare.com/ajax/libs/require.js/2.1.10/require.min.js"></script>')

https://juanlu-poliastro.readthedocs.io/en/latest/examples/Visualizing%20the%20SpaceX%20Tesla%20Roadster%20trip%20to%20Mars.html

I tried to build locally and the result is correctly seen in Firefox and Chrome.

When deploying to RTD, on Firefox this gives several JavaScript ReferenceErrors but the plot loads. These errors come from RTD scripts:

ReferenceError: _ is not defined  # doctools.js:15
ReferenceError: $ is not defined  # readthedocs-doc-embed.js:1
ReferenceError: jQuery is not defined  # Visualizing the SpaceX Tesla Roadster trip to Mars.html:681

In Chrome, on the other hand, I get similar errors but the plot never loads.

When I try to add jQuery as suggested in plotly/plotly.py#1033 (comment):

HTML('''
<script type="text/javascript" src="https://cdnjs.cloudflare.com/ajax/libs/require.js/2.1.10/require.min.js"></script>
<script type="text/javascript" src="https://cdnjs.cloudflare.com/ajax/libs/jquery/2.0.3/jquery.min.js"></script>
''')

https://juanlu-poliastro.readthedocs.io/en/latest/examples/Plotting%20in%203D.html

Then I get errors coming from require.js and the plot doesn't load in Firefox nor Chrome.

@humitos
Copy link
Member

@humitos humitos commented Jul 13, 2018

juanlu-poliastro.readthedocs.io/en/latest/examples/Plotting%20in%203D.html

Then I get errors coming from require.js and the plot doesn't load in Firefox nor Chrome.

I do see the plots in Firefox 61.0.1 (64-bits) on Linux Arch even with the errors that are shown in the Firefox console. On the other hand in Chrome it works, does not work, crashes my Chrome and behaves in a crazy way 😁

We will need to research more about this, but I can tell that after refreshing a couple of times the page from Chrome Versión 61.0.3163.79 (Build oficial) (64 bits) (with and without cache) I was able to see the graph:

captura de pantalla_2018-07-13_20-31-24

I'd say that there is something that depends on the order of loading those js libraries...

I'm not sure how to continue debugging this, though.

@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented Jul 13, 2018

Thanks for your prompt response @humitos! If there's anything I can do to help from my side, please let me know.

@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented Sep 23, 2018

I created a repo with a much simpler test case to display the buggy behavior:

https://github.com/Juanlu001/rtd-js-test

Result:

https://rtd-js-test.readthedocs.io/

There are two branches (one with the RTD theme and one with Alabaster) and five notebooks that display four different combinations + an interactive widget.

  • With the Alabaster theme, all the five notebooks are shown and no relevant JavaScript errors appear in the developer console, both locally and at https://rtd-js-test.readthedocs.io/en/alabaster/.
  • With the RTD theme, there are several different behaviors:
    • The notebooks that do not add require.js at the top (as suggested in plotly/plotly.py#1033 (comment)) raise Uncaught ReferenceError: requirejs is not defined and Uncaught ReferenceError: require is not defined errors and do not show anything, neither locally nor online.
    • The notebooks that add require.js at the top show very long errors and intermittently show the plot online (in Firefox or Chrome) or almost always locally.
    • The notebook that uses a widget works everywhere.

I still did not have time to test what @Blendify started in readthedocs/sphinx_rtd_theme#545, but the fact that Alabaster does not show display this behavior would indicate that there it's a RTD + RTD theme specific behavior.

@jonmmease suggested in plotly/plotly.py#1033 that the problem might be require.js rather than jQuery.

Also, although this was first reported in spatialaudio/nbsphinx#128, I doubt that this can be fixed on the nbsphinx side (cc @mgeier, @bluprince13).

My current workarounds are:

  • Use widgets, because they seem to work better.
  • Use an unreleased version of rtd_sphinx_theme that should work.
  • Use a totally different theme.

I acknowledge that we have to conceal expectations and settle on who should handle this, so can we try to reach an agreement?

@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented Oct 8, 2018

By the way, one of the JS errors I'm getting contains the string "/Users/anthony/dev/readthedocs.org/bower_components/jquery/jquery.js". Is this normal?

@davidfischer davidfischer self-assigned this Oct 8, 2018
@davidfischer
Copy link
Contributor

@davidfischer davidfischer commented Oct 8, 2018

By the way, one of the JS errors I'm getting contains the string "/Users/anthony/dev/readthedocs.org/bower_components/jquery/jquery.js". Is this normal?

Can you tell me where you see this error? I don't see it on
https://rtd-js-test.readthedocs.io/
or
https://rtd-js-test.readthedocs.io/en/alabaster/

@davidfischer
Copy link
Contributor

@davidfischer davidfischer commented Oct 8, 2018

As to fixing the underlying issue, that will be done here:
readthedocs/sphinx_rtd_theme#545

Until then, try the workaround documented here:
readthedocs/sphinx_rtd_theme#328 (comment)

In general, you get better performance by loading JS at the end of the page rather than in the head as loading JS blocks rendering until it is fetched. However, no other Sphinx theme seems to care about that performance so I guess the RTD theme won't either.

@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented Oct 8, 2018

I see that error here: https://rtd-js-test.readthedocs.io/en/latest/Connected%20-%20require.js%20Hack.html

Glad to know readthedocs/sphinx_rtd_theme#545 is still under consideration, I was worried because the branch is lagging a bit behind :) The problem with the workaround is that it's not that easy to modify the output of nbsphinx, as it's not a .rst file where you insert the contents of the notebook. Right @mgeier?

@davidfischer
Copy link
Contributor

@davidfischer davidfischer commented Oct 8, 2018

Thanks. I see that in the error and that's very weird. However, I don't think that's the cause of the error.

@davidfischer
Copy link
Contributor

@davidfischer davidfischer commented Oct 8, 2018

It's definitely an oddity but that is not the source of the problem. The source of the problem is that _ was used before underscore.js was loaded.

screen shot 2018-10-08 at 1 58 19 pm

@basnijholt
Copy link

@basnijholt basnijholt commented Oct 22, 2018

In #4787 (comment) I encountered the same problem. I fixed it by (copy from that issue):

def remove_jquery_and_underscore(app):
    # We need to remove the jquery and underscore file that are
    # added by default because we already add it in the <head> tag.
    remove = lambda x: not any(js in x for js in ['jquery', 'underscore'])
    if hasattr(app.builder, 'script_files'):
        app.builder.script_files = [x for x in app.builder.script_files
                                    if remove(x)]

def setup(app):
    app.connect('builder-inited', remove_jquery_and_underscore)

Using my setup I am able to correctly show plotly plots, for example https://adaptive.readthedocs.io/en/latest/docs.html#adaptive-learnernd.

@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented Nov 3, 2018

I used @Blendify branch at readthedocs/sphinx_rtd_theme#545 and it worked:

https://rtd-js-test.readthedocs.io/en/fork-theme/

No JS errors whatsoever, all the plots work. It would be great to have this in master but I understand ther might be some reservations. In any case, I'm stuck with this (until at least someone rebases it).

@mgeier
Copy link
Contributor

@mgeier mgeier commented Nov 4, 2018

I tried rebasing PR readthedocs/sphinx_rtd_theme#545, but this is indeed quite hard. But the end result should really be just moving a few lines around, so I made a new PR doing just that: readthedocs/sphinx_rtd_theme#696.

Can you please check out if that works for you?

@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented Nov 5, 2018

@mgeier thanks! @Blendify already rebased his and it's working, so I will use that instead.

@stsewd
Copy link
Member

@stsewd stsewd commented Dec 12, 2018

readthedocs/sphinx_rtd_theme#545 was merged, should we wait for a new release of the theme or close this now?

@Blendify
Copy link
Contributor

@Blendify Blendify commented Dec 13, 2018

We can close.

@Blendify Blendify closed this Dec 13, 2018
@dgasmith
Copy link

@dgasmith dgasmith commented Mar 26, 2019

Rebuilding @Juanlu001's repo locally with sphinx_rtd_theme:0.4.3 does not appear to work. Trying the above fixes do not appear to work either. Oddly, inspecting the debugger shows no issues, the image simply does not appear. Has something changed in the last several months that would prevent this?

@basnijholt
Copy link

@basnijholt basnijholt commented Mar 26, 2019

@dgasmith, we had the same problems in Adaptive.

I am not a javascript expert but we solved it in Adaptive, basically by copying theme.js to _static/js and commenting out the first line.

I am not sure whether the presence of that line (loading jQuery with require.js when it's already loaded) is a bug or not. Maybe @stsewd or @Blendify would know?

@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented Mar 26, 2019

Before deeming this a problem with sphinx_rtd_theme, let me repeat the tests. The ipywidgets ecosystem is evolving very fast, sometimes in a not so organic way, and perhaps there's something that broke on the other side.

@basnijholt
Copy link

@basnijholt basnijholt commented Mar 26, 2019

@Juanlu001, in Adaptive we use ipywidgets, plotly and holoviews plots and most of them broke. I solved it by what I wrote above.

Again, I don't know whether this is the best solution.

@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented Mar 26, 2019

Glad to know it's working for you @basnijholt. For me, however, things look a bit more complicated because I tested several combinations of plotly and sphinx-rtd-theme with the latest versions of all the packages, including ipywidgets, and... Now nothing seems to work.

@jonmmease is reworking Plotly rendering in plotly/plotly.py#1474, so perhaps that has the potential of fixing these issues (once and for all?)

In the meanwhile, I will try to go back to some known configuration that worked and identify where the problem is, although it will take me some time.

Amend: In alabaster everything seems to work, but now I can't find any version of plotly + sphinx-rtd-theme that does, not even the fork I used to use.

@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented Mar 26, 2019

(Clarified previous comment with respect to alabaster)

@dgasmith
Copy link

@dgasmith dgasmith commented Mar 26, 2019

@basnijholt Your solve work great! One small addendum, we need to to add https://cdnjs.cloudflare.com/ajax/libs/require.js/2.3.4/require.min.js to the JS headers as well. You pick this up via holoviews, but will not work for everyone.

dgasmith added a commit to MolSSI/QCArchiveExamples that referenced this issue Mar 26, 2019
@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented Apr 10, 2019

Plotly merged a significant refactor of their render machinery, so it would be nice to repeat the tests with the master version (or wait for the next release if you're patient enough):

plotly/plotly.py#1474 (comment)

@manycoding
Copy link

@manycoding manycoding commented May 1, 2019

I am using alabaster, and graphs are not shown. I am not sure if it's a rtd or nbsphinx issue.
If I export the notebook with jupyter nbconvert --to html --execute docs/source/nbs/in-short.ipynb graphs are visible.
https://arche.readthedocs.io/en/docs/nbs/in-short.html

@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented May 2, 2019

@manycoding I see a ReferenceError: require is not defined in the web developer tools. I'm a bit lost on what are the best practices now, but perhaps just including require.js will fix your issue.

@manycoding
Copy link

@manycoding manycoding commented May 2, 2019

@Juanlu001 Yeah, that's probably nbsphinx (or plotly) issue since I see that nbconvert includes require.js.

@astrojuanlu
Copy link
Author

@astrojuanlu astrojuanlu commented Jun 3, 2019

An update after #4367 (comment): I didn't rebuild rtd-js-test, but now things appear to be working in my upstream project poliastro/poliastro#530 (comment). I feel bad bothering everybody about this already, it's also tiring for me :) I don't have time now to know exactly what changed in the middle or what versions are the good ones, but in any case, I consider this done (hope it lasts!)

@manycoding
Copy link

@manycoding manycoding commented Jun 3, 2019

My issue is being fixed in spatialaudio/nbsphinx#302

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.