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

Adding plotly support to gallery examples. #76

Closed
wants to merge 2 commits into from

Conversation

germa89
Copy link
Contributor

@germa89 germa89 commented Jun 14, 2022

@germa89 germa89 added documentation enhancement General improvements to existing features labels Jun 14, 2022
@germa89 germa89 self-assigned this Jun 14, 2022
@germa89
Copy link
Contributor Author

germa89 commented Jun 14, 2022

Output look like:

image

@github-actions github-actions bot added the bug Defects or glitches reported by users or developers label Jun 14, 2022
@@ -3,6 +3,8 @@
{# Append our custom CSS after the bootstrap stylesheet so we can override where necessary #}
{%- block css %}
{{ super() }}
{# adding support for plotly in gallery examples #}
Copy link
Member

Choose a reason for hiding this comment

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

Can we hide this behind a feature flag in the conf.py? It's adding 3.3MB to the page size, and presumably eats some performance on page load to execute the JS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's a very valid point.

I could create a flag in the conf.py file, so it will load it only if activated. Would it work for you?

(btw, who checks the size of the packages?? hahaah #Joking)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thought the same. This basically requires plotly for each and every page.

I think the best approach is to run a "cleanup" script that simply swaps the position of the two lines while submitting an issue (and ideally PR) to the sphinx-gallery team (or applicable repo) to clean up the actual issue.

Copy link
Member

Choose a reason for hiding this comment

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

I could create a flag in the conf.py file, so it will load it only if activated. Would it work for you?

Yep, that would be great! I suspect plotly will be in relatively common use, but for some of the simpler pages it would be nice not having to load it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not something like:

{# adding support for plotly in gallery examples #}
    {% if theme_use_plotly %}
        <script src="https://cdn.plot.ly/plotly-latest.min.js"></script>
    {% endif %}

and then add:

use_plotly=True

to the theme.conf file???

Copy link
Member

Choose a reason for hiding this comment

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

@akaszynski is your concern that this still loads on every page of the documentation (if enabled)? In terms of size I think that's less of a concern (after it's loaded once it should be cached?), but still needs to execute the JS (take that with a grain of salt, I'm not an expert on these things).

Of course, if we can fix it "upstream" that would be the ideal solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

@akaszynski is your concern that this still loads on every page of the documentation (if enabled)? In terms of size I think that's less of a concern (after it's loaded once it should be cached?), but still needs to execute the JS (take that with a grain of salt, I'm not an expert on these things).

Of course, if we can fix it "upstream" that would be the ideal solution.

Should have been clearer.

I'm no expert either, but and I'm assuming that it's using the cache. Still, it appears the JS still is executed/loaded.

pydata-sphinx-theme won't fix the issue. As far as I can tell, it's in:
https://github.com/jupyter-widgets/ipywidgets/blob/9326d28c474cb6d2b58876d469489a73e98f903a/python/ipywidgets/ipywidgets/embed.py#L244-L319

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not something like:

{# adding support for plotly in gallery examples #}
    {% if theme_use_plotly %}
        <script src="https://cdn.plot.ly/plotly-latest.min.js"></script>
    {% endif %}

and then add:

use_plotly=True

to the theme.conf file???

That will still require that each and every page executes the plotly JS regardless of if it's used. Still feels like a bad solution relative to simply patching the HTML until it's fixed upstream.

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'm not expert either... @akaszynski should I take the time to propose something in ipywidgets repo?

Copy link
Contributor

Choose a reason for hiding this comment

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

Try first with the sphinx-gallery

@akaszynski
Copy link
Contributor

@germa89
Copy link
Contributor Author

germa89 commented Jun 14, 2022

@akaszynski I did try, and I guess @andreas-hilti did too.

@jorgepiloto
Copy link
Member

The approach suggested by @akaszynski is the one I knew for rendering plotly in notebooks using Sphinx.

@andreas-hilti
Copy link

I have seen that example, but the difference is that on this page "require.js" is not loaded at all. I haven't figured out what actually injects require.js in my case.

@germa89
Copy link
Contributor Author

germa89 commented Jun 14, 2022

Yes, I do get a white empty box using at the top of the file (also in conf.py just in case)

import plotly.io as pio
pio.renderers.default = 'sphinx_gallery'

image

@germa89
Copy link
Contributor Author

germa89 commented Jun 14, 2022

Finally, I would merge this until we fix this upstream (it could take weeks).

But it is not my call @akaszynski @MaxJPRey @Revathyvenugopal162

@jorgepiloto
Copy link
Member

Checking locally a minimum configuration for this.

@jorgepiloto
Copy link
Member

Within a new venv, everything works as expected just by adding:

import plotly.io as pio
pio.renderers.default = 'sphinx_gallery'

@germa89
Copy link
Contributor Author

germa89 commented Jun 20, 2022

Within a new venv, everything works as expected just by adding:

import plotly.io as pio
pio.renderers.default = 'sphinx_gallery'

It seems this has been tested in https://github.com/plotly/plotly-sphinx-gallery , not against PyMAPDL.

I'm going to create an issue out of this (in PyMAPDL repo) so I can have a look. But this is minor priority for me.

@jorgepiloto
Copy link
Member

I've been working on this and experienced the same issue as @germa89. I finally managed to get "sphinx-gallery" working with "plotly" with the following packages:

alabaster==0.7.12 ansys-sphinx-theme==0.4.2 argon2-cffi==21.3.0 argon2-cffi-bindings==21.2.0 astroid==2.12.2 asttokens==2.0.5 attrs==21.4.0 Babel==2.10.3 backcall==0.2.0 beautifulsoup4==4.11.1 bleach==5.0.1 certifi==2022.6.15 cffi==1.15.1 charset-normalizer==2.1.0 cycler==0.11.0 debugpy==1.6.2 decorator==5.1.1 defusedxml==0.7.1 docutils==0.18.1 entrypoints==0.4 executing==0.8.3 fastjsonschema==2.16.1 fonttools==4.34.4 greenlet==1.1.2 idna==3.3 imagesize==1.4.1 ipykernel==6.15.1 ipython==8.4.0 ipython-genutils==0.2.0 jedi==0.18.1 Jinja2==3.1.2 jsonschema==4.7.2 jupyter-client==7.3.4 jupyter-core==4.11.1 jupyterlab-pygments==0.2.2 jupytext==1.14.0 kaleido==0.2.1 kiwisolver==1.4.4 lazy-object-proxy==1.7.1 lxml==4.9.1 markdown-it-py==2.1.0 MarkupSafe==2.1.1 matplotlib==3.5.2 matplotlib-inline==0.1.3 mdit-py-plugins==0.3.0 mdurl==0.1.1 mistune==0.8.4 msgpack==1.0.4 myst-parser==0.18.0 nbclient==0.6.6 nbconvert==6.5.0 nbformat==5.4.0 nbsphinx==0.8.9 nest-asyncio==1.5.5 notebook==6.4.12 numexpr==2.8.3 numpy==1.23.1 numpydoc==1.4.0 packaging==21.3 pandas==1.4.3 pandocfilters==1.5.0 parso==0.8.3 pexpect==4.8.0 pickleshare==0.7.5 Pillow==9.2.0 plotly==5.9.0 prometheus-client==0.14.1 prompt-toolkit==3.0.30 psutil==5.9.1 ptyprocess==0.7.0 pure-eval==0.2.2 pycparser==2.21 pydata-sphinx-theme==0.9.0 Pygments==2.12.0 pynvim==0.4.3 pyparsing==3.0.9 pyrsistent==0.18.1 python-dateutil==2.8.2 pytz==2022.1 PyYAML==6.0 pyzmq==23.2.0 requests==2.28.1 Send2Trash==1.8.0 six==1.16.0 snowballstemmer==2.2.0 soupsieve==2.3.2.post1 Sphinx==5.0.2 sphinx-autoapi==1.8.4 sphinx-copybutton==0.5.0 sphinx-gallery==0.7.0 sphinxcontrib-applehelp==1.0.2 sphinxcontrib-devhelp==1.0.2 sphinxcontrib-email==0.3.5 sphinxcontrib-htmlhelp==2.0.0 sphinxcontrib-jsmath==1.0.1 sphinxcontrib-qthelp==1.0.3 sphinxcontrib-serializinghtml==1.1.5 stack-data==0.3.0 tenacity==8.0.1 terminado==0.15.0 tinycss2==1.1.1 toml==0.10.2 toolz==0.12.0 tornado==6.2 traitlets==5.3.0 typing_extensions==4.3.0 Unidecode==1.3.4 urllib3==1.26.10 wcwidth==0.2.5 webencodings==0.5.1 wrapt==1.14.1

The kaleido==0.2.1 was required in my case.

@germa89
Copy link
Contributor Author

germa89 commented Jul 22, 2022

The kaleido==0.2.1 was required in my case.

@jorgepiloto , so just adding that library will make it work?

I don't really understand why that library is needed since it converts interactive plotly plots to static images (https://pypi.org/project/kaleido/).

@RobPasMue
Copy link
Member

BTW,,, what's the plan with this PR @germa89? Are you planning on merging it? Is there anything blocking it?

@germa89
Copy link
Contributor Author

germa89 commented Jan 4, 2023

BTW,,, what's the plan with this PR @germa89? Are you planning on merging it? Is there anything blocking it?

This PR should be closed because the approach is not the best.

@germa89 germa89 closed this Jan 4, 2023
@germa89 germa89 deleted the fix/adding-plotly-to-gallery-examples branch January 4, 2023 17:06
@germa89
Copy link
Contributor Author

germa89 commented Sep 18, 2023

Soooo... do we have an alternative for this then?? @ansys/pyansys-core

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defects or glitches reported by users or developers enhancement General improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Plotly capabilities to gallery examples
7 participants