Repository: sphinx-gallery/sphinx-gallery

Issue:
BUG Windows relative path error with _static Binder logo
In Windows, when the package _static files are in a different drive to the `gallery_conf['src_dir']`, `os.path.relpath` gives an error:
```
  File "C:\hostedtoolcache\windows\Python\3.6.8\x64\lib\site-packages\sphinx_gallery\binder.py", line 112, in gen_binder_rst
    binder_logo_path = os.path.relpath(binder_logo, gallery_conf['src_dir'])
  File "C:\hostedtoolcache\windows\Python\3.6.8\x64\lib\ntpath.py", line 584, in relpath
    path_drive, start_drive))
ValueError: path is on mount 'C:', start on mount 'D:
```

See: https://github.com/scikit-image/scikit-image/issues/4958 and https://github.com/scikit-image/scikit-image/pull/4959

I can copy the binder logo to `target_dir`/images (or just `target_dir` ?) like I was doing in the first iter of #738 .

Also what would be the best way to add a test for this. Can we change `appveyor.yml` such that Sphinx-Gallery is installed in a different drive to where the testing occurs?

WDYT @larsoner ?

(I'll do a bug fix release when fixed)

Discussion:
larsoner: I suspect you shouldn't need to manually copy any static file, just link to it properly. The question is how to do that. Usually / hopefully you can do it by doing a relpath between the example file and the `src_dir` or so (?), or maybe get the static path from Sphinx. I can dig into it Tuesday if we can wait that long
lucyleeow: The problem is that relative paths between 2 different drives on Windows doesn't exist. It may be possible with symbolic links but we might not want to mess with that.

No hurry, take a look on Tuesday.
codecov[bot]: # [Codecov](https://codecov.io/gh/sphinx-gallery/sphinx-gallery/pull/745?src=pr&el=h1) Report
> Merging [#745](https://codecov.io/gh/sphinx-gallery/sphinx-gallery/pull/745?src=pr&el=desc) into [master](https://codecov.io/gh/sphinx-gallery/sphinx-gallery/commit/839c02b0b076f20ea7cc0ca57a619ac7f00467f5?el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/sphinx-gallery/sphinx-gallery/pull/745/graphs/tree.svg?width=650&height=150&src=pr&token=K4eytvhwU5)](https://codecov.io/gh/sphinx-gallery/sphinx-gallery/pull/745?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #745   +/-   ##
=======================================
  Coverage   97.50%   97.50%           
=======================================
  Files          32       32           
  Lines        3727     3727           
=======================================
  Hits         3634     3634           
  Misses         93       93           
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/sphinx-gallery/sphinx-gallery/pull/745?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/sphinx-gallery/sphinx-gallery/pull/745?src=pr&el=footer). Last update [839c02b...2ae3bb2](https://codecov.io/gh/sphinx-gallery/sphinx-gallery/pull/745?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

larsoner: @lucyleeow pushed a commit. The critical point is that, regardless of what `html_static_path`s users enter, they always end up in `_static/`. So we just need to link directly to `OUT_DIR/_static/image.svg`. See if it makes sense and if so, feel free to merge and cut a 0.8.1
lucyleeow: I think I've just missed something important. So all files in the package `_static` dir (i.e. `<where you installed SG>/sphinx-gallery/sphinx_gallery/_static`) are always copied over to the built html (e.g., `<where your package is>/doc/_build/html/_static/`)? 

Can you show me where this is done in the code? 
larsoner: > Can you show me where this is done in the code?

It's done by `sphinx` itself somewhere (feel free to dig through their code to figure this out). If you look at `tinybuild` I modified it to use `html_static_paths = ['_static_nonstandard']`, but when you `make html`, the static files from that dir and from `glr_path_static` (because we add it to the static paths) get copied to OUT_DIR/_static. AFAIK this is a hard-coded output path
lucyleeow: Thanks, I had no idea Sphinx did this but it makes sense.
larsoner: Feel free to fix the path error in CIs, or I can push a fix in a couple of hours. Have meetings now
larsoner: Actually I think that all of those build warnings are bogus, they say files don't exist but it renders fine:

https://1811-25860190-gh.circle-artifacts.com/0/rtd_html/auto_examples/plot_0_sin.html#sphx-glr-auto-examples-plot-0-sin-py

I'm not sure how to avoid this warning. Maybe worth asking on the Sphinx listserv or GitHub. But we do need to make these warnings go away somehow because this will cause `-nW` builds fail. :(
larsoner: This might also be relevant

https://github.com/sphinx-gallery/sphinx-gallery/issues/120

And maybe

https://stackoverflow.com/questions/52138336/sphinx-reference-to-an-image-from-different-locations#answer-52141199
lucyleeow: I'll do this tomorrow

larsoner: Actually I wonder if the correct fix is to give the path to the `sphx_glr_static()` without any `relpath` business. Sphinx in theory should recognize that this is a path to a static file and substitute OUT_DIR/static. I'll give it a shot...
larsoner: Okay @lucyleeow I ended up giving up on the static idea and ended up with something that I think is pretty close to what you had. Can you take a look and merge if you're happy?
lucyleeow: Sorry can you clarify what `glr_path_static` returns? Is it the packaged `_static`, where the package files are or the path to the the `_static` folder where the built html files are?
larsoner: `glr_path_static` is the path to sphinx-gallery's `sphinx_gallery/_static` folder. The contents will be copied to `OUT_DIR/_static` on build. The problem is that we can't actually use it (see code comments)
lucyleeow: Okay thanks. But in Windows, if the path has to be relative to the build root, and the build root is in a different drive to `glr_path_static`, I thought there was not relative path between different drives?
Sorry maybe I missed something?
larsoner: > Sorry maybe I missed something?

`relpath` is no longer used

Patch:
diff --git a/sphinx_gallery/binder.py b/sphinx_gallery/binder.py
index d4a04a6..8c59033 100644
--- a/sphinx_gallery/binder.py
+++ b/sphinx_gallery/binder.py
@@ -15,14 +15,13 @@ change in the future.
 
 """
 
-import shutil
 import os
+import shutil
 
 from sphinx.errors import ConfigError
 
 from .utils import replace_py_ipynb
-from . import sphinx_compatibility
-from . import glr_path_static
+from . import sphinx_compatibility, glr_path_static
 
 
 logger = sphinx_compatibility.getLogger('sphinx-gallery')
@@ -105,18 +104,30 @@ def gen_binder_rst(fpath, binder_conf, gallery_conf):
     rst : str
         The reStructuredText for the Binder badge that links to this file.
     """
-    binder_conf = check_binder_conf(binder_conf)
     binder_url = gen_binder_url(fpath, binder_conf, gallery_conf)
-
-    binder_logo = os.path.join(glr_path_static(), 'binder_badge_logo.svg')
-    binder_logo_path = os.path.relpath(binder_logo, gallery_conf['src_dir'])
-
+    # In theory we should be able to use glr_path_static for this, but Sphinx
+    # only allows paths to be relative to the build root. On Linux, absolute
+    # paths can be used and they work, but this does not seem to be
+    # documented behavior:
+    #     https://github.com/sphinx-doc/sphinx/issues/7772
+    # And in any case, it does not work on Windows, so here we copy the SVG to
+    # `images` for each gallery and link to it there. This will make
+    # a few copies, and there will be an extra in `_static` at the end of the
+    # build, but it at least works...
+    physical_path = os.path.join(
+        os.path.dirname(fpath), 'images', 'binder_badge_logo.svg')
+    os.makedirs(os.path.dirname(physical_path), exist_ok=True)
+    if not os.path.isfile(physical_path):
+        shutil.copyfile(
+            os.path.join(glr_path_static(), 'binder_badge_logo.svg'),
+            physical_path)
     rst = (
         "\n"
         "  .. container:: binder-badge\n\n"
-        "    .. image:: /{}\n"
+        "    .. image:: images/binder_badge_logo.svg\n"
         "      :target: {}\n"
-        "      :width: 150 px\n").format(binder_logo_path, binder_url)
+        "      :alt: Launch binder\n"
+        "      :width: 150 px\n").format(binder_url)
     return rst
 
 
@@ -129,7 +140,7 @@ def copy_binder_files(app, exception):
         return
 
     gallery_conf = app.config.sphinx_gallery_conf
-    binder_conf = check_binder_conf(gallery_conf.get('binder'))
+    binder_conf = gallery_conf['binder']
 
     if not len(binder_conf) > 0:
         return
diff --git a/sphinx_gallery/gen_gallery.py b/sphinx_gallery/gen_gallery.py
index 984d95b..baf1e28 100644
--- a/sphinx_gallery/gen_gallery.py
+++ b/sphinx_gallery/gen_gallery.py
@@ -31,7 +31,7 @@ from .scrapers import _scraper_dict, _reset_dict, _import_matplotlib
 from .docs_resolv import embed_code_links
 from .downloads import generate_zipfiles
 from .sorting import NumberOfCodeLinesSortKey
-from .binder import copy_binder_files
+from .binder import copy_binder_files, check_binder_conf
 from .directives import MiniGallery
 
 
@@ -322,6 +322,9 @@ def _complete_gallery_conf(sphinx_gallery_conf, src_dir, plot_gallery,
     if isinstance(backref, pathlib.Path):
         gallery_conf['backreferences_dir'] = str(backref)
 
+    # binder
+    gallery_conf['binder'] = check_binder_conf(gallery_conf['binder'])
+
     if not isinstance(gallery_conf['css'], (list, tuple)):
         raise ConfigError('gallery_conf["css"] must be list or tuple, got %r'
                           % (gallery_conf['css'],))


Test patch:
diff --git a/sphinx_gallery/tests/test_binder.py b/sphinx_gallery/tests/test_binder.py
index 42d9f40..152ced1 100644
--- a/sphinx_gallery/tests/test_binder.py
+++ b/sphinx_gallery/tests/test_binder.py
@@ -6,12 +6,11 @@ Testing the binder badge functionality
 """
 from __future__ import division, absolute_import, print_function
 
-import os
 from copy import deepcopy
+import os
 
 import pytest
 
-from sphinx_gallery.__init__ import glr_path_static
 from sphinx.errors import ConfigError
 from sphinx_gallery.binder import (gen_binder_url, check_binder_conf,
                                    _copy_binder_reqs, gen_binder_rst)
@@ -117,19 +116,26 @@ def test_binder():
     assert url == expected
 
 
-def test_gen_binder_rst():
+def test_gen_binder_rst(tmpdir):
     """Check binder rst generated correctly."""
-    gallery_conf_base = {'gallery_dirs': 'mydir', 'src_dir': 'blahblah'}
-    file_path = 'blahblah/mydir/myfile.py'
+    gallery_conf_base = {'gallery_dirs': [str(tmpdir)], 'src_dir': 'blahblah'}
+    file_path = str(tmpdir.join('blahblah', 'mydir', 'myfile.py'))
     conf_base = {'binderhub_url': 'http://test1.com', 'org': 'org',
                  'repo': 'repo', 'branch': 'branch',
                  'dependencies': '../requirements.txt'}
-
-    rst = gen_binder_rst(file_path, conf_base, gallery_conf_base)
-    binder_logo = os.path.join(glr_path_static(), 'binder_badge_logo.svg')
-    binder_logo_path = os.path.relpath(binder_logo, 'blahblah')
-
-    image_rst = (' .. image:: /{}').format(binder_logo_path)
+    conf_base = check_binder_conf(conf_base)
+    orig_dir = os.getcwd()
+    os.chdir(str(tmpdir))
+    try:
+        rst = gen_binder_rst(file_path, conf_base, gallery_conf_base)
+    finally:
+        os.chdir(orig_dir)
+    image_rst = ' .. image:: images/binder_badge_logo.svg'
     target_rst = ':target: http://test1.com/v2/gh/org/repo/branch?filepath=notebooks/mydir/myfile.ipynb'  # noqa E501
+    alt_rst = ':alt: Launch binder'
     assert image_rst in rst
     assert target_rst in rst
+    assert alt_rst in rst
+    image_fname = os.path.join(
+        os.path.dirname(file_path), 'images', 'binder_badge_logo.svg')
+    assert os.path.isfile(image_fname)
diff --git a/sphinx_gallery/tests/test_full.py b/sphinx_gallery/tests/test_full.py
index 2884f3e..103ea6b 100644
--- a/sphinx_gallery/tests/test_full.py
+++ b/sphinx_gallery/tests/test_full.py
@@ -18,7 +18,6 @@ import time
 import numpy as np
 from numpy.testing import assert_allclose
 
-from sphinx_gallery.__init__ import glr_path_static
 from sphinx.application import Sphinx
 from sphinx.errors import ExtensionError
 from sphinx.util.docutils import docutils_namespace
@@ -164,7 +163,7 @@ def test_thumbnail_path(sphinx_app, tmpdir):
     """Test sphinx_gallery_thumbnail_path."""
     # Make sure our thumbnail matches what it should be
     fname_orig = op.join(
-        sphinx_app.srcdir, '_static', 'demo.png')
+        sphinx_app.srcdir, '_static_nonstandard', 'demo.png')
     fname_thumb = op.join(
         sphinx_app.outdir, '_images',
         'sphx_glr_plot_second_future_imports_thumb.png')
@@ -445,8 +444,13 @@ def test_rebuild(tmpdir_factory, sphinx_app):
         new_app.build(False, [])
     status = new_app._status.getvalue()
     lines = [line for line in status.split('\n') if '0 removed' in line]
-    assert re.match('.*[0|1] added, [1-9] changed, 0 removed$.*',
-                    status, re.MULTILINE | re.DOTALL) is not None, lines
+    # XXX on AppVeyor this can be 13
+    if sys.platform.startswith('win'):
+        assert re.match('.*[0|1] added, [1-9][0-3]? changed, 0 removed$.*',
+                        status, re.MULTILINE | re.DOTALL) is not None, lines
+    else:
+        assert re.match('.*[0|1] added, [1-9] changed, 0 removed$.*',
+                        status, re.MULTILINE | re.DOTALL) is not None, lines
     want = ('.*executed 0 out of %s.*after excluding %s files.*based on MD5.*'
             % (N_FAILING, N_GOOD))
     assert re.match(want, status, re.MULTILINE | re.DOTALL) is not None
@@ -806,6 +810,7 @@ def test_jupyter_notebook_pandoc(sphinx_app):
 
 
 def test_md5_hash(sphinx_app):
+    """Test MD5 hashing."""
     src_dir = sphinx_app.srcdir
     fname = op.join(src_dir, 'auto_examples', 'plot_log.py.md5')
     expected_md5 = '0edc2de97f96f3b55f8b4a21994931a8'
@@ -816,9 +821,15 @@ def test_md5_hash(sphinx_app):
 
 
 def test_binder_logo_exists(sphinx_app):
-    """Check that the binder logo path is correct."""
-    src_dir = sphinx_app.srcdir
-    binder_logo = os.path.join(glr_path_static(), 'binder_badge_logo.svg')
-    binder_logo_path = os.path.relpath(binder_logo, src_dir)
-
-    assert op.isfile('/{}'.format(binder_logo_path))
+    """Test that the binder logo path is correct."""
+    root = op.join(sphinx_app.outdir, 'auto_examples')
+    with codecs.open(op.join(root, 'plot_svg.html'), 'r', 'utf-8') as fid:
+        html = fid.read()
+    path = re.match(r'.*<img alt="Launch binder" src="(.*)" width=.*\/>.*',
+                    html, re.DOTALL)
+    assert path is not None
+    path = path.groups()[0]
+    img_fname = op.abspath(op.join(root, path))
+    assert 'binder_badge_logo' in img_fname  # can have numbers appended
+    assert op.isfile(img_fname)
+    assert 'https://mybinder.org/v2/gh/sphinx-gallery/sphinx-gallery.github.io/master?urlpath=lab/tree/notebooks/auto_examples/plot_svg.ipynb' in html  # noqa: E501
diff --git a/sphinx_gallery/tests/test_gen_rst.py b/sphinx_gallery/tests/test_gen_rst.py
index 4a79448..447b068 100644
--- a/sphinx_gallery/tests/test_gen_rst.py
+++ b/sphinx_gallery/tests/test_gen_rst.py
@@ -555,7 +555,8 @@ def test_rst_example(gallery_conf):
                                 'dependencies': './binder/requirements.txt',
                                 'notebooks_dir': 'notebooks',
                                 'use_jupyter_lab': True,
-                                })
+                                },
+                        gallery_dirs=None)
 
     example_file = os.path.join(gallery_conf['gallery_dir'], "plot.py")
     sg.save_rst_example("example_rst", example_file, 0, 0, gallery_conf)
@@ -710,7 +711,7 @@ def _clean_output(output):
 @pytest.mark.parametrize('capture_repr, code, expected_out', [
     pytest.param(tuple(), 'a=2\nb=3', '', id='assign,()'),
     pytest.param(tuple(), 'a=2\na', '', id='var,()'),
-    pytest.param(tuple(), 'a=2\nprint(a)', '2',  id='print(var),()'),
+    pytest.param(tuple(), 'a=2\nprint(a)', '2', id='print(var),()'),
     pytest.param(tuple(), 'print("hello")\na=2\na', 'hello',
                  id='print+var,()'),
     pytest.param(('__repr__',), 'a=2\nb=3', '', id='assign,(repr)'),
diff --git a/sphinx_gallery/tests/test_notebook.py b/sphinx_gallery/tests/test_notebook.py
index 62d245d..0fa6f4e 100644
--- a/sphinx_gallery/tests/test_notebook.py
+++ b/sphinx_gallery/tests/test_notebook.py
@@ -204,7 +204,8 @@ def test_notebook_images_data_uri(gallery_conf):
         gallery_conf['src_dir'], gallery_conf['gallery_dirs'])
 
     test_image = os.path.join(
-        os.path.dirname(__file__), 'tinybuild', '_static', 'demo.png')
+        os.path.dirname(__file__), 'tinybuild',
+        '_static_nonstandard', 'demo.png')
     # Make into "absolute" path from source directory
     test_image_rel = os.path.relpath(test_image, gallery_conf['src_dir'])
     test_image_abs = '/' + test_image_rel.replace(os.sep, '/')
diff --git a/sphinx_gallery/tests/tinybuild/_static/demo.png b/sphinx_gallery/tests/tinybuild/_static_nonstandard/demo.png
similarity index 100%
rename from sphinx_gallery/tests/tinybuild/_static/demo.png
rename to sphinx_gallery/tests/tinybuild/_static_nonstandard/demo.png
diff --git a/sphinx_gallery/tests/tinybuild/binder/requirements.txt b/sphinx_gallery/tests/tinybuild/binder/requirements.txt
new file mode 100644
index 0000000..092ecbe
--- /dev/null
+++ b/sphinx_gallery/tests/tinybuild/binder/requirements.txt
@@ -0,0 +1,8 @@
+# Requirements for the documentation.
+# These are extra dependencies that aren't required to use sphinx-gallery.
+numpy
+matplotlib
+pillow
+seaborn
+joblib
+sphinx-gallery
diff --git a/sphinx_gallery/tests/tinybuild/conf.py b/sphinx_gallery/tests/tinybuild/conf.py
index dbdeadd..6f7b921 100644
--- a/sphinx_gallery/tests/tinybuild/conf.py
+++ b/sphinx_gallery/tests/tinybuild/conf.py
@@ -69,6 +69,14 @@ sphinx_gallery_conf = {
         'sphinx_gallery': None,
         'scipy': 'http://docs.scipy.org/doc/scipy/wrong_url',  # bad one
     },
+    'binder': {'org': 'sphinx-gallery',
+               'repo': 'sphinx-gallery.github.io',
+               'branch': 'master',
+               'binderhub_url': 'https://mybinder.org',
+               'dependencies': './binder/requirements.txt',
+               'notebooks_dir': 'notebooks',
+               'use_jupyter_lab': True,
+               },
     'examples_dirs': ['examples/'],
     'reset_argv': ResetArgv(),
     'reset_modules': (MockScrapeProblem(), 'matplotlib'),
@@ -88,4 +96,4 @@ sphinx_gallery_conf = {
 }
 nitpicky = True
 highlight_language = 'python3'
-html_static_path = ['_static']
+html_static_path = ['_static_nonstandard']
diff --git a/sphinx_gallery/tests/tinybuild/examples/plot_second_future_imports.py b/sphinx_gallery/tests/tinybuild/examples/plot_second_future_imports.py
index a53e91d..9bab8a9 100644
--- a/sphinx_gallery/tests/tinybuild/examples/plot_second_future_imports.py
+++ b/sphinx_gallery/tests/tinybuild/examples/plot_second_future_imports.py
@@ -6,7 +6,7 @@ This example runs after plot_future_statements.py (alphabetical ordering within
 subsection) and should be unaffected by the __future__ import in
 plot_future_statements.py.
 """
-# sphinx_gallery_thumbnail_path = '_static/demo.png'
+# sphinx_gallery_thumbnail_path = '_static_nonstandard/demo.png'
 
 import sys
 from sphinx_gallery.sorting import ExplicitOrder