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 graphs do not show in docs #281

Closed
astrojuanlu opened this issue Nov 15, 2017 · 16 comments
Closed

Plotly graphs do not show in docs #281

astrojuanlu opened this issue Nov 15, 2017 · 16 comments

Comments

@astrojuanlu
Copy link
Member

@astrojuanlu astrojuanlu commented Nov 15, 2017

See spatialaudio/nbsphinx#128

I wonder if there's some sort of dirty workaround we can have in place prior the release...

@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Nov 15, 2017

I'd love to have this in 0.8.0, but I doubt it will arrive in time.

@astrojuanlu astrojuanlu modified the milestones: Future, 0.9 Nov 15, 2017
@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Nov 17, 2017

In fact, this is a problem of the default RTD theme: readthedocs/sphinx_rtd_theme#328 I'm not switching the theme to accomodate the notebooks, so we will either include require.js in the quick 'n' dirty way or wait for an upstream fix.

@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Nov 18, 2017

This pull request fixes the issue readthedocs/sphinx_rtd_theme#467

@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented May 13, 2018

Reopening, as it's the case again.

@astrojuanlu astrojuanlu reopened this May 13, 2018
@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Jun 28, 2018

Being tracked at readthedocs/sphinx_rtd_theme#545, but it will probably take a while to ship.

@astrojuanlu astrojuanlu modified the milestones: 0.9, 0.10 Jun 29, 2018
@shreyasbapat
Copy link
Member

@shreyasbapat shreyasbapat commented Jul 6, 2018

The PR is still open. Do you think it is a local issue and we can solve it in poliastro?

@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Jul 6, 2018

It's indeed an upstream issue that will take a while, but in the meanwhile we have a workaround, which is adding some extra cells at the top (and running them!)

# Temporary hack, see https://github.com/poliastro/poliastro/issues/281
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>')
@shreyasbapat
Copy link
Member

@shreyasbapat shreyasbapat commented Jul 6, 2018

Oh! Okay. I will add these lines and run the notebooks again

@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Jul 13, 2018

I explored several hacks and ended up opening an issue on RTD:

readthedocs/readthedocs.org#4367

@shreyasbapat
Copy link
Member

@shreyasbapat shreyasbapat commented Jul 14, 2018

I thought it was not really an issue. Even I experienced this multiple times. That the plots were showing fine locally, while building in RTD stopped them. However, even Firefox does not load the plots all the time 😅

shreyasbapat added a commit to shreyasbapat/poliastro that referenced this issue Aug 5, 2018
@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Sep 12, 2018

This is still an issue :( Conversations restarted on the sphinx_rtd_theme, which is where jQuery should go. An alternative would be to change Plotly to load jQuery asynchronously. In any case, I want to fix this upstream.

@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Sep 23, 2018

I spent some time setting up a test case to display the behavior, see my comment at

readthedocs/readthedocs.org#4367 (comment)

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 don't want workarounds 2 and 3, and widgets have still some rough edges (see jupyter-widgets/ipywidgets#1632, jupyterlab/jupyterlab#5200 and jupyter/nbconvert#751, not to mention the slightly related jupyter/nbconvert#878).

There is no perfect solution but having empty boxes where there should be plots is not acceptable either, so I will wait for some weeks to progress to happen on any of these fronts and I will make a decision before the next poliastro release.

@astrojuanlu astrojuanlu added this to the 0.12 milestone Sep 23, 2018
astrojuanlu added a commit to astrojuanlu/poliastro that referenced this issue Oct 28, 2018
Addresses poliastro#338.

Now trying to import OrbitPlotter directly from poliastro.plotting
fails, because it's the "old" backend and users are encouraged to
use OrbitPlotter2D and OrbitPlotter3D instead, which use plotly
and not matplotlib behind the scenes.

This does not address:

* Updates in documentation
* Failing test because OrbitPlotter2D has no set_frame method (poliastro#480)
* Test simplification and refactoring (poliastro#448 and poliastro#447)

Also, notice that poliastro#281 should be fixed first.
astrojuanlu added a commit to astrojuanlu/poliastro that referenced this issue Nov 3, 2018
Addresses poliastro#338.

Now trying to import OrbitPlotter directly from poliastro.plotting
fails, because it's the "old" backend and users are encouraged to
use OrbitPlotter2D and OrbitPlotter3D instead, which use plotly
and not matplotlib behind the scenes.

This does not address:

* Updates in documentation
* Failing test because OrbitPlotter2D has no set_frame method (poliastro#480)
* Test simplification and refactoring (poliastro#448 and poliastro#447)

Also, notice that poliastro#281 should be fixed first.
@ghost ghost added 2 - In Progress and removed 1 - Ready labels Nov 3, 2018
@ghost ghost removed the 2 - In Progress label Nov 3, 2018
astrojuanlu added a commit to astrojuanlu/poliastro that referenced this issue Nov 9, 2018
Addresses poliastro#338.

Now trying to import OrbitPlotter directly from poliastro.plotting
fails, because it's the "old" backend and users are encouraged to
use OrbitPlotter2D and OrbitPlotter3D instead, which use plotly
and not matplotlib behind the scenes.

This does not address:

* Updates in documentation
* Failing test because OrbitPlotter2D has no set_frame method (poliastro#480)
* Test simplification and refactoring (poliastro#448 and poliastro#447)

Also, notice that poliastro#281 should be fixed first.
@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Jul 16, 2019

After #728, here we are again!

@astrojuanlu astrojuanlu reopened this Jul 16, 2019
@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Jul 20, 2019

@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Jul 20, 2019

Even though @shreyasbapat has suggested several times to change the theme... I'd still would love to work upstream to fix this. Even though it's been two years already...

@astrojuanlu
Copy link
Member Author

@astrojuanlu astrojuanlu commented Jul 21, 2019

This time it was easy: readthedocs/sphinx_rtd_theme#788

With #741, now we are really closing this issue, and after almost two years I can say the Plotly + nbsphinx + RTD drama is over!

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
2 participants