disable calls to plotly.io.show when running on Azure

[grlee77]

Description

It has been a common occurance lately for the CI job on Azure that involves gallery testing to timeout.

I have a suspicion that this is somehow related to Plotly, but haven't confirmed that for sure yet.

In this PR, I try defining environment variable: SKIMAGE_DISABLE_PLOTLY_SHOW to disable it

This environment variable is only exported during Gallery testing on Azure, so users should still get the plots popping up in their browser if running the scripts from the command line.

Checklist

• Docstrings for all functions
• Gallery example in ./doc/examples (new features only)
• Benchmark in ./benchmarks, if your changes aren't covered by an
  existing benchmark
• Unit tests
• Clean style in the spirit of PEP8

For reviewers

• Check that the PR title is short, concise, and will make sense 1 year
  later.
• Check that new functions are imported in corresponding __init__.py.
• Check that new features, API changes, and deprecations are mentioned in
  doc/release/release_dev.rst.

[grlee77]

This seems to have worked. I manually started the Azure job a second time here to check that we didn't just get lucky.

@mkcor, the timeouts that have been common on Azure in recent weeks seem to have been related to the plotly.io.show calls I had asked you to add to the examples. This PR bypasses those when running the Azure gallery testing job. I tried this with a gallery example, but let me know if you are aware of anything else in Plotly itself.

[grlee77]

A second option would be replacing calls to show() with fig.write_html("<filename>"), but this would leave a files on disk that the user would have to manually go and open with a browser if they wanted to see it, so I am not a big fan of that.

[jni]

@grlee77 The problem I have with this is that it makes the gallery examples uglier, for something that is a mild problem from the developer side. I'm not sure on balance it is a good change. I think I would prefer writing a tool in our /tools folder that would crawl all our examples and comment-out lines with plotly.io.show. What do you think? (And maybe it doesn't need a dedicated script, just a grep/find/sed one-liner, but my shell-fu is not good enough for that!)

[grlee77]

I agree that it makes those examples more ugly. I reverted the environment variable and instead use a one line sed-based solution in 0e2dc2a.

[grlee77]

CI is green for the sed-based version as well.

The only change to an example script now is adding the show commands to doc/examples/applications/plot_3d_structure_tensor.py where they were missing.

[grlee77]

ping @jni

[mkcor]

I like this non-invasive workaround 🙂

[mkcor]

@mkcor, the timeouts that have been common on Azure in recent weeks seem to have been related to the plotly.io.show calls I had asked you to add to the examples.

I think these timeouts would happen (sometimes) even without the plotly.io.show calls: Plotly figures by themselves can make the generated HTML files pretty heavy. When building the example gallery locally, I've noticed the slowdown caused by Plotly figures, regardless of the renderer.

This PR bypasses those when running the Azure gallery testing job.

Okay.

I tried this with a gallery example, but let me know if you are aware of anything else in Plotly itself.

Not really; I haven't followed closely beyond plotly/plotly.py#1577 by @emmanuelle. I didn't know of plotly.io.to_html and plotly.io.write_html before you pointed out the second option (shame on me).

I know that, here,

scikit-image/doc/source/conf.py

Line 155 in 6ff11f6

pio.renderers.default = 'sphinx_gallery_png'

we want 'sphinx_gallery_png' rather than 'sphinx_gallery' so that gallery thumbnails display well. The other renderer candidates don't seem fitting.

[grlee77]

@mkcor: I have addressed the comments. CI is still running, but the relevant Azure jobs have already passed.

[mkcor]

@mkcor: I have addressed the comments. CI is still running, but the relevant Azure jobs have already passed.

Yay! Thanks. Since this is a CI+docs PR, can I go ahead and merge?

[grlee77]

We addressed Juan's original objection to the environment variable, so I say go for it!