Repository: sphinx-gallery/sphinx-gallery

Issue:
Minigallery directive file checking
It's unclear what we want to be checking with:

https://github.com/sphinx-gallery/sphinx-gallery/blob/2fc5067f519ee17de066471cf1e1f2eeffb51add/sphinx_gallery/directives.py#L136-L146

It seems originally the check was for cases where `dirs > 1` (see https://github.com/sphinx-gallery/sphinx-gallery/pull/1226#discussion_r1416586216) BUT we don't seem to be able to make this happen (see https://github.com/sphinx-gallery/sphinx-gallery/pull/1355#issuecomment-2251500148). It would be great if we could work out what we are meant to be checking here but :shrug:  if not.


We **can** reach this error by making `dirs = 0`. I think this check would be to ensure the glob files we find live in the examples file, which is a reasonable check to have.

Even this is not working though. Using `../../tinybuild/examples/*/*.py` as a minigallery entry in tinybuild (as suggested: https://github.com/sphinx-gallery/sphinx-gallery/pull/1355#issuecomment-2251484364), `dirs=0` even though the examples found do live within an `examples_dirs`. This is because `'../../tinybuild/examples/*/*.py'.find('../examples/')` gives -1.

We can fix this by amending the check to:

```python
if (obj.find(Path(e).name) != -1)
```

BUT then we get this error:

> ValueError: '/home/<>/sphinx-gallery/sphinx_gallery/tests/tinybuild/doc/../../tinybuild/examples_README_header/plot_exp.py' is not in the subpath of '/home/<>/sphinx-gallery/sphinx_gallery/tests/tinybuild/doc/../examples' OR one path is relative and the other is absolute.

If I recall correctly, @story645 , we wanted to keep the glob paths we find to be relative, to allow sorting. BUT can we pass the relative paths to the sortkey but use abs paths otherwise in the directive? I think this would make the above check (ensuring the glob paths found live within an `examples_dir`) much easier/better as well as avoiding errors like the one above.

Would you be up for tacking this? I have spent more time than I should on SG :grimacing: 






Discussion:
larsoner: > Would this be the best way to do this @larsoner ? I thought about making a new fixture, like sphinx_app_wrapper but copying different src files, but I think if I can just copy over files using the same sphinx_app_wrapper fixture, it would be better?

Yeah I think reusing the existing `sphinx_app_wrapper` is reasonable
lucyleeow: This is ready for merge, would be good to get your eye over this @larsoner !

* added tests for the minigallery cases where the file is not in an examples dir and when its from a file in a gallery nested inside another gallery (here the file path matches 2 examples_dirs)
* moved things to use pathlib

lucyleeow: Figuring out `sphinx_app_wrapper` was a wild ride.

For posterity, it was confusing that the `conf_file` fixture and the `conf_file` pytest mark are called the same thing. They are separate and would work fine if they were name differently.

Patch:
diff --git a/doc/configuration.rst b/doc/configuration.rst
index 0cc1e3f..c32f6e9 100644
--- a/doc/configuration.rst
+++ b/doc/configuration.rst
@@ -44,6 +44,7 @@ file, inside a ``sphinx_gallery_conf`` dictionary.
 - ``abort_on_example_error`` (:ref:`abort_on_first`)
 - ``expected_failing_examples`` (:ref:`dont_fail_exit`)
 - ``only_warn_on_example_error`` (:ref:`warning_on_error`)
+- ``reset_modules`` and ``reset_modules_order`` (:ref:`reset_modules`)
 - ``parallel`` (:ref:`parallel`)
 
 **Cross-referencing**
@@ -86,7 +87,6 @@ file, inside a ``sphinx_gallery_conf`` dictionary.
 
 **Miscellaneous**
 
-- ``reset_modules`` and ``reset_modules_order`` (:ref:`reset_modules`)
 - ``recommender`` (:ref:`recommend_examples`)
 - ``log_level`` (:ref:`log_level`)
 - ``show_api_usage`` and ``api_usage_ignore`` (:ref:`show_api_usage`)
@@ -2155,6 +2155,17 @@ If an ``int``, then that number of jobs will be passed to :class:`joblib.Paralle
 If ``True``, then the same number of jobs will be used as the ``-j`` flag for
 Sphinx.
 
+Warnings emitted by :mod:`joblib` during gallery generation (e.g., the ``UserWarning``
+about a `worker restarting <https://github.com/joblib/joblib/issues/883>`_),
+will be captured by Sphinx-Gallery in a similar way to warnings during example
+execution. This can be filtered out with ``warnings.filterwarnings``
+(see :ref:`removing_warnings`). This can be particularly important
+to do if you have tweaked warning handling in your doc build
+to treat warnings as errors, e.g., with a line like
+``warnings.filterwarnings("error")``. Note that this
+differs from warnings affected by the ``- W`` / ``--fail-on-warning`` ``sphinx-build``
+flag, which converts warnings during documentation building into errors.
+
 .. warning::
     Some packages might not play nicely with parallel processing, so this feature
     is considered **experimental**!
diff --git a/pyproject.toml b/pyproject.toml
index a2110d7..b354880 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -167,4 +167,5 @@ filterwarnings = [
 junit_family = "xunit2"
 markers = [
   "conf_file: Configuration file.",
+  "add_rst: Add rst file to src.",
 ]
diff --git a/sphinx_gallery/directives.py b/sphinx_gallery/directives.py
index 4b00ec9..ff5a91a 100644
--- a/sphinx_gallery/directives.py
+++ b/sphinx_gallery/directives.py
@@ -53,6 +53,52 @@ class MiniGallery(Directive):
         "heading-level": directives.single_char_or_unicode,
     }
 
+    def _get_target_dir(self, config, src_dir, path, obj):
+        """Get thumbnail target directory, errors when ambiguous/not in example dir."""
+        examples_dirs = config.sphinx_gallery_conf["examples_dirs"]
+        if not isinstance(examples_dirs, list):
+            examples_dirs = [examples_dirs]
+        gallery_dirs = config.sphinx_gallery_conf["gallery_dirs"]
+        if not isinstance(gallery_dirs, list):
+            gallery_dirs = [gallery_dirs]
+
+        gal_matches = []
+        for e, g in zip(examples_dirs, gallery_dirs):
+            e = Path(src_dir, e).resolve(strict=True)
+            g = Path(src_dir, g).resolve(strict=True)
+            try:
+                gal_matches.append((path.relative_to(e), g))
+            except ValueError:
+                continue
+
+        # `path` inside one `examples_dirs`
+        if (n_match := len(gal_matches)) == 1:
+            ex_parents, target_dir = (
+                gal_matches[0][0].parents,
+                gal_matches[0][1],
+            )
+        # `path` inside several `examples_dirs`, take the example dir with
+        # longest match (shortest parents after relative)
+        elif n_match > 1:
+            ex_parents, target_dir = min(
+                [(match[0].parents, match[1]) for match in gal_matches],
+                key=lambda x: len(x[0]),
+            )
+        # `path` is not inside a `examples_dirs`
+        else:
+            raise ExtensionError(
+                f"minigallery directive error: path input '{obj}' found file:"
+                f" '{path}' but this does not live inside any examples_dirs: "
+                f"{examples_dirs}"
+            )
+
+        # Add subgallery path, if present
+        subdir = ""
+        if (ex_p := ex_parents[0]) != Path("."):
+            subdir = ex_p
+        target_dir = target_dir / subdir
+        return target_dir
+
     def run(self):
         """Generate mini-gallery from backreference and example files."""
         from .gen_rst import _get_callables
@@ -73,7 +119,7 @@ class MiniGallery(Directive):
         if backreferences_dir is None:
             logger.warning(
                 "'backreferences_dir' config is None, minigallery "
-                "directive will only add example file paths."
+                "directive will resolve all inputs as file paths or globs."
             )
 
         # Retrieve source directory
@@ -108,9 +154,9 @@ class MiniGallery(Directive):
         file_paths = []
         for obj in obj_list:
             if backreferences_dir and (path := has_backrefs(obj)):
-                file_paths.append((obj, path))
+                file_paths.append((obj, path.resolve()))
             elif paths := Path(src_dir).glob(obj):
-                file_paths.extend([(obj, p) for p in paths])
+                file_paths.extend([(obj, p.resolve()) for p in paths])
 
         if len(file_paths) == 0:
             return []
@@ -126,7 +172,7 @@ class MiniGallery(Directive):
             )
         for obj, path in sorted(
             set(file_paths),
-            key=((lambda x: sortkey(os.path.abspath(x[-1]))) if sortkey else None),
+            key=((lambda x: sortkey(str(x[-1]))) if sortkey else None),
         ):
             if path.suffix == ".examples":
                 # Insert the backreferences file(s) using the `include` directive.
@@ -138,27 +184,8 @@ class MiniGallery(Directive):
     :end-before: thumbnail-parent-div-close"""
                 )
             else:
-                examples_dirs = config.sphinx_gallery_conf["examples_dirs"]
-                if not isinstance(examples_dirs, list):
-                    examples_dirs = [examples_dirs]
-                gallery_dirs = config.sphinx_gallery_conf["gallery_dirs"]
-                if not isinstance(gallery_dirs, list):
-                    gallery_dirs = [gallery_dirs]
-                dirs = [
-                    (e, g)
-                    for e, g in zip(examples_dirs, gallery_dirs)
-                    if (obj.find(e) != -1)
-                ]
-                if len(dirs) != 1:
-                    raise ExtensionError(
-                        f"Error in minigallery file lookup: input={obj}, "
-                        f"matches={dirs}, examples_dirs={examples_dirs}"
-                    )
-
-                example_dir, target_dir = [Path(src_dir, d) for d in dirs[0]]
-
-                # finds thumbnails in subdirs
-                target_dir = target_dir / path.relative_to(example_dir).parent
+                target_dir = self._get_target_dir(config, src_dir, path, obj)
+                # Get thumbnail
                 _, script_blocks = split_code_and_text_blocks(
                     str(path), return_node=False
                 )
diff --git a/sphinx_gallery/docs_resolv.py b/sphinx_gallery/docs_resolv.py
index 6f0c5c0..ae1ce6e 100644
--- a/sphinx_gallery/docs_resolv.py
+++ b/sphinx_gallery/docs_resolv.py
@@ -199,14 +199,12 @@ class SphinxDocLinkResolver:
 
     def _get_link_type(self, cobj, use_full_module=False):
         """Get a valid link and type_, False if not found."""
-        module_type = "module_short"
-        if use_full_module:
-            module_type = "module"
+        module_type = "module" if use_full_module else "module_short"
         first, second = cobj[module_type], cobj["name"]
         match = self._get_index_match(first, second)
         if match is None and "." in second:  # possible class attribute
             first, second = second.split(".", 1)
-            first = ".".join([cobj["module_short"], first])
+            first = ".".join([cobj[module_type], first])
             match = self._get_index_match(first, second)
         if match is None:
             link = type_ = None
diff --git a/sphinx_gallery/gen_gallery.py b/sphinx_gallery/gen_gallery.py
index 5237eb6..6b3937f 100644
--- a/sphinx_gallery/gen_gallery.py
+++ b/sphinx_gallery/gen_gallery.py
@@ -567,6 +567,12 @@ def _prepare_sphx_glr_dirs(gallery_conf, srcdir):
     if not isinstance(gallery_dirs, list):
         gallery_dirs = [gallery_dirs]
 
+    if len(examples_dirs) != len(gallery_dirs):
+        logger.warning(
+            "'examples_dirs' and 'gallery_dirs' are of different lengths. "
+            "Surplus entries will be ignored."
+        )
+
     if bool(gallery_conf["backreferences_dir"]):
         backreferences_dir = os.path.join(srcdir, gallery_conf["backreferences_dir"])
         os.makedirs(backreferences_dir, exist_ok=True)


Test patch:
diff --git a/sphinx_gallery/tests/conftest.py b/sphinx_gallery/tests/conftest.py
index 8bd44ab..3e65cbe 100644
--- a/sphinx_gallery/tests/conftest.py
+++ b/sphinx_gallery/tests/conftest.py
@@ -1,9 +1,9 @@
 """Pytest fixtures."""
 
-import os
 import shutil
 from contextlib import contextmanager
 from io import StringIO
+from pathlib import Path
 from unittest.mock import Mock
 
 import pytest
@@ -16,6 +16,23 @@ from sphinx_gallery import docs_resolv, gen_gallery, gen_rst, py_source_parser
 from sphinx_gallery.scrapers import _import_matplotlib
 from sphinx_gallery.utils import _get_image
 
+NESTED_PY = """\"\"\"
+Header
+======
+
+Text.
+\"\"\"
+
+a = 1
+"""
+
+GALLERY_HEADER = """
+Gallery header
+==============
+
+Some text.
+"""
+
 
 def pytest_report_header(config, startdir=None):
     """Add information to the pytest run header."""
@@ -133,6 +150,16 @@ def conf_file(request):
     return result
 
 
+@pytest.fixture
+def add_rst(request):
+    try:
+        env = request.node.get_closest_marker("add_rst")
+    except AttributeError:  # old pytest
+        env = request.node.get_marker("add_rst")
+    file = env.kwargs["file"] if env else ""
+    return file
+
+
 class SphinxAppWrapper:
     """Wrapper for sphinx.application.Application.
 
@@ -181,13 +208,24 @@ class SphinxAppWrapper:
 
 
 @pytest.fixture
-def sphinx_app_wrapper(tmpdir, conf_file, req_mpl, req_pil):
-    _fixturedir = os.path.join(os.path.dirname(__file__), "testconfs")
-    srcdir = os.path.join(str(tmpdir), "config_test")
+def sphinx_app_wrapper(tmpdir, conf_file, add_rst, req_mpl, req_pil):
+    _fixturedir = Path(__file__).parent / "testconfs"
+    srcdir = Path(tmpdir) / "config_test"
     shutil.copytree(_fixturedir, srcdir)
-    shutil.copytree(
-        os.path.join(_fixturedir, "src"), os.path.join(str(tmpdir), "examples")
-    )
+    # Copy files to 'examples/' as well because default `examples_dirs` is
+    # '../examples' - for tests where we don't update config
+    shutil.copytree((_fixturedir / "src"), (Path(tmpdir) / "examples"))
+    if add_rst:
+        with open((srcdir / "minigallery_test.rst"), "w") as rstfile:
+            rstfile.write(add_rst)
+        # Add nested gallery
+        if "sub_folder/sub_sub_folder" in add_rst:
+            dir_path = srcdir / "src" / "sub_folder" / "sub_sub_folder"
+            dir_path.mkdir(parents=True)
+            with open((dir_path / "plot_nested.py"), "w") as pyfile:
+                pyfile.write(NESTED_PY)
+            with open((dir_path / "GALLERY_HEADER.rst"), "w") as rstfile:
+                rstfile.write(GALLERY_HEADER)
 
     base_config = f"""
 import os
@@ -199,14 +237,14 @@ master_doc = 'index'
 # General information about the project.
 project = 'Sphinx-Gallery <Tests>'\n\n
 """
-    with open(os.path.join(srcdir, "conf.py"), "w") as conffile:
+    with open((srcdir / "conf.py"), "w") as conffile:
         conffile.write(base_config + conf_file["content"])
 
     return SphinxAppWrapper(
         srcdir,
         srcdir,
-        os.path.join(srcdir, "_build"),
-        os.path.join(srcdir, "_build", "toctree"),
+        (srcdir / "_build"),
+        (srcdir / "_build" / "toctree"),
         "html",
         warning=StringIO(),
         status=StringIO(),
diff --git a/sphinx_gallery/tests/test_gen_gallery.py b/sphinx_gallery/tests/test_gen_gallery.py
index c048e9d..a28f2d3 100644
--- a/sphinx_gallery/tests/test_gen_gallery.py
+++ b/sphinx_gallery/tests/test_gen_gallery.py
@@ -100,6 +100,20 @@ def test_no_warning_simple_config(sphinx_app_wrapper):
     assert build_warn == ""
 
 
+@pytest.mark.conf_file(
+    content="""
+sphinx_gallery_conf = {
+    'examples_dirs': ['src', 'excess'],
+    'gallery_dirs': 'ex',
+}"""
+)
+def test_unequal_examples_gallery_dirs(sphinx_app_wrapper):
+    """Check warning when 'examples_dirs' and 'gallery_dirs' unequal lengths."""
+    sphinx_app = sphinx_app_wrapper.create_sphinx_app()
+    build_warn = sphinx_app._warning.getvalue()
+    assert "'examples_dirs' and 'gallery_dirs' are of different lengths" in build_warn
+
+
 # This changed from passing the ValueError directly to
 # raising "sphinx.errors.ConfigError" with "threw an exception"
 @pytest.mark.parametrize(
@@ -259,7 +273,7 @@ def _check_order(sphinx_app, key):
 
     Used to test that these keys (in index.rst) appear in a specific order.
     """
-    index_fname = os.path.join(sphinx_app.outdir, "..", "ex", "index.rst")
+    index_fname = Path(sphinx_app.outdir, "..", "ex", "index.rst")
     order = list()
     regex = f".*:{key}=(.):.*"
     with open(index_fname, "r", encoding="utf-8") as fid:
@@ -387,7 +401,7 @@ def test_collect_gallery_files_ignore_pattern(tmpdir, gallery_conf):
     expected_files = {
         ap.strpath
         for ap in abs_paths
-        if re.search(r"one", os.path.basename(ap.strpath)) is None
+        if re.search(r"one", Path(ap.strpath).name) is None
     }
 
     assert collected_files == expected_files
@@ -412,19 +426,17 @@ def test_binder_copy_files(sphinx_app_wrapper):
     sphinx_app = sphinx_app_wrapper.create_sphinx_app()
     gallery_conf = sphinx_app.config.sphinx_gallery_conf
     # Create requirements file
-    with open(os.path.join(sphinx_app.srcdir, "requirements.txt"), "w"):
+    with open(Path(sphinx_app.srcdir, "requirements.txt"), "w"):
         pass
     copy_binder_files(sphinx_app, None)
 
     for i_file in ["plot_1", "plot_2", "plot_3"]:
-        assert os.path.exists(
-            os.path.join(
-                sphinx_app.outdir,
-                "ntbk_folder",
-                gallery_conf["gallery_dirs"][0],
-                i_file + ".ipynb",
-            )
-        )
+        assert Path(
+            sphinx_app.outdir,
+            "ntbk_folder",
+            gallery_conf["gallery_dirs"][0],
+            i_file + ".ipynb",
+        ).exists()
 
 
 @pytest.mark.conf_file(
@@ -435,7 +447,7 @@ sphinx_gallery_conf = {
 }"""
 )
 def test_failing_examples_raise_exception(sphinx_app_wrapper):
-    example_dir = os.path.join(sphinx_app_wrapper.srcdir, "src")
+    example_dir = Path(sphinx_app_wrapper.srcdir, "src")
     bad_line = "print(f'{a[}')"  # never closed bracket inside print -> SyntaxError
     bad_code = f"""\
 '''
@@ -451,7 +463,7 @@ Should emit a syntax error in the second code block.
 {bad_line}
 """
     bad_line_no = bad_code.split("\n").index(bad_line) + 1
-    with open(os.path.join(example_dir, "plot_3.py"), "w", encoding="utf-8") as fid:
+    with open(Path(example_dir, "plot_3.py"), "w", encoding="utf-8") as fid:
         fid.write(bad_code)
     with pytest.raises(ExtensionError) as excinfo:
         sphinx_app_wrapper.build_sphinx_app()
@@ -614,6 +626,59 @@ def test_backreferences_dir_pathlib_config(sphinx_app_wrapper):
     fill_gallery_conf_defaults(app, app.config, check_keys=False)
 
 
+@pytest.mark.conf_file(
+    content="""
+sphinx_gallery_conf = {
+    'examples_dirs': 'src',
+    'gallery_dirs': 'ex',
+}"""
+)
+@pytest.mark.add_rst(
+    file="""
+Header
+======
+
+.. minigallery:: index.rst
+"""
+)
+def test_minigallery_not_in_examples_dirs(sphinx_app_wrapper):
+    """Check error when minigallery directive's path input not in `examples_dirs`."""
+    msg = "minigallery directive error: path input 'index.rst'"
+    with pytest.raises(ExtensionError, match=msg):
+        sphinx_app_wrapper.build_sphinx_app()
+
+
+@pytest.mark.conf_file(
+    content="""
+sphinx_gallery_conf = {
+    'examples_dirs': ['src', 'src/sub_folder/sub_sub_folder'],
+    'gallery_dirs': ['ex', 'ex/sub_folder/sub_sub_folder'],
+}"""
+)
+@pytest.mark.add_rst(
+    file="""
+Header
+======
+
+.. minigallery:: src/sub_folder/sub_sub_folder/plot_nested.py
+"""
+)
+def test_minigallery_multi_match(sphinx_app_wrapper):
+    """Check minigallery directive's path input resolution in nested `examples_dirs`.
+
+    When a examples gallery is nested inside another examples gallery, path inputs
+    from the nested gallery should resolve to the nested gallery.
+    """
+    sphinx_app = sphinx_app_wrapper.build_sphinx_app()
+    minigallery_html = Path(sphinx_app.outdir) / "minigallery_test.html"
+    with open(minigallery_html, "r") as fid:
+        mg_html = fid.read()
+    # Check thumbnail correct
+    assert "_images/sphx_glr_plot_nested_thumb.png" in mg_html
+    # Check href correct
+    assert "sphx-glr-ex-sub-folder-sub-sub-folder-plot-nested-py" in mg_html
+
+
 def test_write_computation_times_noop(sphinx_app_wrapper):
     app = sphinx_app_wrapper.create_sphinx_app()
     write_computation_times(app.config.sphinx_gallery_conf, None, [])
@@ -673,14 +738,12 @@ def test_create_jupyterlite_contents(sphinx_app_wrapper):
     create_jupyterlite_contents(sphinx_app, exception=None)
 
     for i_file in ["plot_1", "plot_2", "plot_3"]:
-        assert os.path.exists(
-            os.path.join(
-                sphinx_app.srcdir,
-                "jupyterlite_contents",
-                gallery_conf["gallery_dirs"][0],
-                i_file + ".ipynb",
-            )
-        )
+        assert Path(
+            sphinx_app.srcdir,
+            "jupyterlite_contents",
+            gallery_conf["gallery_dirs"][0],
+            i_file + ".ipynb",
+        ).exists()
 
 
 @pytest.mark.conf_file(
@@ -703,14 +766,12 @@ def test_create_jupyterlite_contents_non_default_contents(sphinx_app_wrapper):
     create_jupyterlite_contents(sphinx_app, exception=None)
 
     for i_file in ["plot_1", "plot_2", "plot_3"]:
-        assert os.path.exists(
-            os.path.join(
-                sphinx_app.srcdir,
-                "this_is_the_contents_dir",
-                gallery_conf["gallery_dirs"][0],
-                i_file + ".ipynb",
-            )
-        )
+        assert Path(
+            sphinx_app.srcdir,
+            "this_is_the_contents_dir",
+            gallery_conf["gallery_dirs"][0],
+            i_file + ".ipynb",
+        ).exists()
 
 
 @pytest.mark.conf_file(
@@ -729,7 +790,7 @@ def test_create_jupyterlite_contents_without_jupyterlite_sphinx_loaded(
     sphinx_app = sphinx_app_wrapper.create_sphinx_app()
 
     create_jupyterlite_contents(sphinx_app, exception=None)
-    assert not os.path.exists(os.path.join(sphinx_app.srcdir, "jupyterlite_contents"))
+    assert not Path(sphinx_app.srcdir, "jupyterlite_contents").exists()
 
 
 @pytest.mark.conf_file(
@@ -754,7 +815,7 @@ def test_create_jupyterlite_contents_with_jupyterlite_disabled_via_config(
     sphinx_app = sphinx_app_wrapper.create_sphinx_app()
 
     create_jupyterlite_contents(sphinx_app, exception=None)
-    assert not os.path.exists(os.path.join(sphinx_app.outdir, "jupyterlite_contents"))
+    assert not Path(sphinx_app.outdir, "jupyterlite_contents").exists()
 
 
 @pytest.mark.conf_file(
@@ -788,13 +849,13 @@ def test_create_jupyterlite_contents_with_modification(sphinx_app_wrapper):
     create_jupyterlite_contents(sphinx_app, exception=None)
 
     for i_file in ["plot_1", "plot_2", "plot_3"]:
-        notebook_filename = os.path.join(
+        notebook_filename = Path(
             sphinx_app.srcdir,
             "jupyterlite_contents",
             gallery_conf["gallery_dirs"][0],
             i_file + ".ipynb",
         )
-        assert os.path.exists(notebook_filename)
+        assert notebook_filename.exists()
 
         with open(notebook_filename) as f:
             notebook_content = json.load(f)