Merge pull request #132 from chrisjsewell/improve-download-rebase

Fix download role

.circleci/config.yml

@@ -18,7 +18,7 @@ jobs:
           name: Build documentation
           command: |
             cd doc
-            make html
+            make html-strict
 
       - store_artifacts:
           path: doc/build/html/

doc/Makefile

@@ -16,4 +16,11 @@ help:
 # Catch-all target: route all unknown targets to Sphinx using the new
 # "make mode" option.  $(O) is meant as a shortcut for $(SPHINXOPTS).
 %: Makefile
-	@$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
+	@$(SPHINXBUILD) -M $@ "$(SOURCEDIR)" "$(BUILDDIR)" $(SPHINXOPTS) $(O)
+
+# raise warnings to errors
+html-strict:
+	@$(SPHINXBUILD) -b html -nW --keep-going "$(SOURCEDIR)" "$(BUILDDIR)/html" $(SPHINXOPTS) $(O)
+
+clean:
+	rm -r $(BUILDDIR)

doc/source/index.rst

@@ -317,14 +317,18 @@ Pygments.
 
 Downloading the code as a script
 --------------------------------
+
 Jupyter Sphinx includes 2 roles that can be used to download the code embedded in a document:
-``:jupyter-download:script:`` (for a raw script file) and ``:jupyter-download:notebook:`` (for
-a Jupyter notebook). For example, to download all the code from this document as a script we
+``:jupyter-download:script:`` (for a raw script file) and ``:jupyter-download:notebook:`` or ``:jupyter-download:nb:`` (for
+a Jupyter notebook). 
+
+These roles are equivalent to the standard sphinx `download role <https://www.sphinx-doc.org/en/master/usage/restructuredtext/roles.html#role-download>`__, **except** the extension of the file should not be given.
+For example, to download all the code from this document as a script we
 would use::
 
-    :jupyter-download:script:`index`
+    :jupyter-download:script:`click to download <index>`
 
-Which produces a link like this: :jupyter-download:script:`index`. The name that the role is
+Which produces a link like this: :jupyter-download:script:`click to download <index>`. The target that the role is
 applied to (``index`` in this case) is the name of the document for which you wish to download
 the code. If a document contains ``jupyter-kernel`` directives with ``:id:`` specified, then
 the name provided to ``:id:`` can be used to get the code for the cells belonging to the

jupyter_sphinx/__init__.py

@@ -19,7 +19,7 @@
     JupyterWidgetViewNode,
     JupyterWidgetStateNode,
     WIDGET_VIEW_MIMETYPE,
-    jupyter_download_role,
+    JupyterDownloadRole,
     CellOutputsToNodes,
 )
 from .execute import JupyterKernel, ExecuteJupyterCells
@@ -272,8 +272,9 @@ def setup(app):
     app.add_directive("jupyter-execute", JupyterCell)
     app.add_directive("jupyter-kernel", JupyterKernel)
     app.add_directive("thebe-button", ThebeButton)
-    app.add_role("jupyter-download:notebook", jupyter_download_role)
-    app.add_role("jupyter-download:script", jupyter_download_role)
+    app.add_role("jupyter-download:notebook", JupyterDownloadRole())
+    app.add_role("jupyter-download:nb", JupyterDownloadRole())
+    app.add_role("jupyter-download:script", JupyterDownloadRole())
     app.add_transform(ExecuteJupyterCells)
     app.add_post_transform(CellOutputsToNodes)
 

jupyter_sphinx/ast.py

@@ -5,8 +5,9 @@
 
 import docutils
 from docutils.parsers.rst import Directive, directives
-from docutils.nodes import math_block, image
+from docutils.nodes import math_block, image, literal
 from sphinx.util import parselinenos
+from sphinx.util.docutils import ReferenceRole
 from sphinx.addnodes import download_reference
 from sphinx.transforms import SphinxTransform
 from sphinx.environment.collectors.asset import ImageCollector
@@ -393,16 +394,19 @@ def attach_outputs(output_nodes, node, thebe_config, cm_language):
         node.children = node.children[::-1]
 
 
-def jupyter_download_role(name, rawtext, text, lineno, inliner):
-    _, filetype = name.split(":")
-    assert filetype in ("notebook", "script")
-    ext = ".ipynb" if filetype == "notebook" else ".py"
-    output_dir = sphinx_abs_dir(inliner.document.settings.env)
-    download_file = text + ext
-    node = download_reference(
-        download_file, download_file, reftarget=os.path.join(output_dir, download_file)
-    )
-    return [node], []
+class JupyterDownloadRole(ReferenceRole):
+    def run(self):
+        _, filetype = self.name.split(":")
+
+        assert filetype in ("notebook", "nb", "script")
+        ext = ".ipynb" if filetype in ("notebook", "nb") else ".py"
+        download_file = self.target + ext
+        reftarget = sphinx_abs_dir(self.env, download_file)
+        node = download_reference(self.rawtext, reftarget=reftarget)
+        self.set_source_info(node)
+        title = self.title if self.has_explicit_title else download_file
+        node += literal(self.rawtext, title, classes=["xref", "download"])
+        return [node], []
 
 
 def get_widgets(notebook):

jupyter_sphinx/execute.py

@@ -22,7 +22,6 @@
     default_notebook_names,
     output_directory,
     split_on,
-    sphinx_abs_dir,
     blank_nb,
 )
 from .ast import (

jupyter_sphinx/utils.py

@@ -69,14 +69,14 @@ def language_info(executor):
     return info_msg["content"]["language_info"]
 
 
-def sphinx_abs_dir(env):
+def sphinx_abs_dir(env, *paths):
     # We write the output files into
     # output_directory / jupyter_execute / path relative to source directory
     # Sphinx expects download links relative to source file or relative to
     # source dir and prepended with '/'. We use the latter option.
     return "/" + os.path.relpath(
         os.path.abspath(
-            os.path.join(output_directory(env), os.path.dirname(env.docname))
+            os.path.join(output_directory(env), os.path.dirname(env.docname), *paths)
         ),
         os.path.abspath(env.app.srcdir),
     )

requirements.txt

@@ -1,4 +1,4 @@
-sphinx==1.8.5
+sphinx==2.4.4
 ipywidgets>=7.0.0
 IPython
 nbconvert>=5.4

setup.py

@@ -28,7 +28,7 @@
     license="BSD",
     packages=["jupyter_sphinx"],
     install_requires=[
-        "Sphinx>=1.8",
+        "Sphinx>=2",
         "ipywidgets>=7.0.0",
         "IPython",
         "nbconvert>=5.5",

tests/test_execute.py

@@ -3,10 +3,12 @@
 import os
 import sys
 from io import StringIO
+from unittest.mock import Mock
 
-from sphinx.testing.util import SphinxTestApp, path
+from sphinx.addnodes import download_reference
+from sphinx.testing.util import assert_node, SphinxTestApp, path
 from sphinx.errors import ExtensionError
-from docutils.nodes import raw
+from docutils.nodes import literal, raw
 from nbformat import from_dict
 
 import pytest
@@ -18,6 +20,7 @@
     JupyterWidgetViewNode,
     JupyterWidgetStateNode,
     cell_output_to_nodes,
+    JupyterDownloadRole,
 )
 from jupyter_sphinx.thebelab import ThebeSourceNode, ThebeOutputNode, ThebeButtonNode
 
@@ -564,3 +567,25 @@ def test_image_mimetype_uri(doctree):
         cell = from_dict(cell)
         output_node = cell_output_to_nodes(cell["outputs"], priority, True, output_dir, None)
         assert output_node[0].attributes['uri'] == img_locs[index]
+
+
+@pytest.mark.parametrize('text,reftarget,caption', (
+    ('nb_name', '/../jupyter_execute/path/to/nb_name.ipynb', 'nb_name.ipynb'),
+    ('../nb_name', '/../jupyter_execute/path/nb_name.ipynb', '../nb_name.ipynb'),
+    ('text <nb_name>', '/../jupyter_execute/path/to/nb_name.ipynb', 'text'),
+))
+def test_download_role(text, reftarget, caption, tmp_path):
+    role = JupyterDownloadRole()
+    mock_inliner = Mock()
+    config = {
+        'document.settings.env.app.outdir': str(tmp_path),
+        'document.settings.env.docname': 'path/to/docname',
+        'document.settings.env.srcdir': str(tmp_path),
+        'document.settings.env.app.srcdir': str(tmp_path),
+        'reporter.get_source_and_line': lambda l: ('source', l)
+    }
+    mock_inliner.configure_mock(**config)
+    ret, msg = role('jupyter-download:notebook', text, text, 0, mock_inliner)
+    assert_node(ret[0], [download_reference], reftarget=reftarget)
+    assert_node(ret[0][0], [literal, caption])
+    assert msg == []