Improve minigallery directive path input resolution#1360
Improve minigallery directive path input resolution#1360larsoner merged 8 commits intosphinx-gallery:masterfrom
Conversation
lucyleeow
left a comment
There was a problem hiding this comment.
I've described how I think we'd get to the errors below.
I am not sure how best to test it, as this would error on a build. I think I could add a test to test_gen_gallery.py where I copy over a new .rst file that has a minigallery directive that would error (using the sphinx_app_wrapper.srcdir to get the right path).
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?
sphinx_gallery/directives.py
Outdated
| ) | ||
| # `path` is not inside a `examples_dirs` | ||
| else: | ||
| raise ExtensionError( |
There was a problem hiding this comment.
This is the case where the glob match is not inside an examples_dirs (i.e., in old code when len(dirs) = 0).
sphinx_gallery/directives.py
Outdated
| elif n_match > 1: | ||
| ex_parents, target_dir = max( | ||
| [(match[0].parents, match[1]) for match in gal_matches], | ||
| key=lambda x: len(x[0]), | ||
| ) |
There was a problem hiding this comment.
I think this may have been what the old len(dirs)>1 was testing before. The only case where I think this could come about is e.g.,
file: /a/b/c/d/file.py
examples dirs (abs path):
/a/b//a/b/c/d(i.e. you have a gallery within a sub-sub-folder of another gallery - this may be done if someone wanted to use more nesting for their galleries)
In this case, I think it makes sense that we just use the longest matching example dir.
There was a problem hiding this comment.
Hmm, everything in Matplotlib is nested at least 3 deep (eg galleries/gallery_folder/subsection) so that could have been it...
There was a problem hiding this comment.
Hmm I don't think that is the same. E.g., examples_dirs in mpl would be e.g., ../galleries/examples, ../galleries/plot_types/. These folders have sub-gallaries/sub-folders, but these subfolderes do not need to be added to examples_dirs as SG auto checks for 1 level of sub-folders.
It's only in the case where you want 2 levels of sub-folders e.g., you also have ../galleries/examples/animation/moreanimation/ in your examples_dirs.
It would be unusual case. And I don't even know if I can be bothered adding a test for this... 🤔
Yeah I think reusing the existing |
|
This is ready for merge, would be good to get your eye over this @larsoner !
|
|
Figuring out For posterity, it was confusing that the |
Feel free to open a PR to rename one of them! |
|
Thanks @lucyleeow ! |
|
Good idea, somehow I am just hesitant about changing existing 'convention'. |
… to version 0.17.1 v0.17.1 ------- **Fixed bugs:** - FIX: Fix stability of stored compiled regex `#1369 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1369>`__ (`larsoner <https://github.com/larsoner>`__) - ENH: Improve \_sanitize_rst `#1366 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1366>`__ (`timhoffm <https://github.com/timhoffm>`__) - Obey prefer_full_module setting when finding backreferences `#1364 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1364>`__ (`QuLogic <https://github.com/QuLogic>`__) - Fix linking to class attributes with prefer_full_module `#1363 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1363>`__ (`QuLogic <https://github.com/QuLogic>`__) - Improve minigallery directive path input resolution `#1360 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1360>`__ (`lucyleeow <https://github.com/lucyleeow>`__) - FIX Allow str path minigallery entries when backreferences off `#1355 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1355>`__ (`lucyleeow <https://github.com/lucyleeow>`__) - FIX generate zipfiles when index passed by user `#1353 <https://github.com/sphinx-gallery/sphinx-gallery/pull/1353>`__ (`lucyleeow <https://github.com/lucyleeow>`__) **Documentation** (NEWS truncated at 15 lines)
closes #1356
Uses
relative_toto get target gallery path - in particular improves the part where we figure out if the file is in a subgallery/subfolder. I think therelative_tois more robust and I think it should work as intended.e.g.,
one glob match:
'.../sphinx-gallery/sphinx_gallery/tests/tinybuild/examples_rst_index/examp_subdir2/plot_sub2.py'example dir path:
.../sphinx_gallery/sphinx_gallery/tests/tinybuild/examples_rst_index/Uses
resolveto get abs paths.cc @story645