Skip to content

Obey prefer_full_module setting when finding backreferences #1364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Aug 3, 2024
Merged

Obey prefer_full_module setting when finding backreferences #1364

merged 2 commits into from
Aug 3, 2024

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Aug 1, 2024

This fixes several cases of missing backreferences in Matplotlib's mpl_toolkits namespace, which need the full module name for referencing into Sphinx.

For example, Axes3D.plot only has one back reference:
image

but with this change we have all these:
image

@larsoner larsoner added the bug label Aug 1, 2024
@larsoner
Copy link
Contributor

larsoner commented Aug 1, 2024

Would be great to have a test that would have caught this. @QuLogic WDYT about adding an Axes3D to one of the sphinx_gallery/tests/tinybuild examples? On master that wouldn't backref / minigallery properly but it would on this PR, right? If so then adding it there you could check in sphinx_gallery/tests/test_full.py somewhere that it works. (Maybe you'd need to add a custom minigallery to the build as well?)

@larsoner larsoner mentioned this pull request Aug 1, 2024
Merged
@QuLogic
Obey prefer_full_module setting when finding backreferences
d127c5e 
This fixes several cases of missing backreferences in Matplotlib's
`mpl_toolkits` namespace, which need the full module name for
referencing into Sphinx.
@QuLogic
Copy link
Contributor Author

QuLogic commented Aug 3, 2024

But for back references, it would have to be something in the documented module, so from sphinx_gallery for tinybuild, wouldn't it?

@lucyleeow
Copy link
Contributor

I think any module listed in doc_module would work, see: https://sphinx-gallery.github.io/dev/configuration.html#add-mini-galleries-for-api-documentation

@QuLogic QuLogic force-pushed the backref-full-module branch from ee5bba4 to cf5e6d1 Compare August 3, 2024 12:42
@QuLogic
Copy link
Contributor Author

QuLogic commented Aug 3, 2024

I added a _dummy module, and moved DummyClass there, along with a nested NestedDummyClass. And the added tests should use that and cover this and #1363.

QuLogic
QuLogic commented Aug 3, 2024
View reviewed changes
sphinx_gallery/tests/test_full.py
@@ -548,25 +548,49 @@ def test_embed_links_and_styles(sphinx_app):
# issue 617 (regex '-'s)
# instance
dummy_class_inst = re.search(
r'sphinx_gallery.backreferences.html#sphinx[_,-]gallery[.,-]backreferences[.,-][D,d]ummy[C,c]lass" title="sphinx_gallery.backreferences.DummyClass" class="sphx-glr-backref-module-sphinx_gallery-backreferences sphx-glr-backref-type-py-class sphx-glr-backref-instance"><span class="n">dc</span>', # noqa: E501
r'sphinx_gallery._dummy.html#sphinx[_-]gallery[.-]_dummy[.-][Dd]ummy[Cc]lass" title="sphinx_gallery._dummy.DummyClass" class="sphx-glr-backref-module-sphinx_gallery-_dummy sphx-glr-backref-type-py-class sphx-glr-backref-instance"><span class="n">dc</span>', # noqa: E501
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also fixed the regex here, as #617 seems to be saying that _ or . may be - instead, but these seem to be also allowing , which seems unlikely.

@QuLogic
Add tests for prefer_full_module
7743497 
These failed before the previous commit.
@QuLogic QuLogic force-pushed the backref-full-module branch from cf5e6d1 to 7743497 Compare August 3, 2024 12:49
@larsoner
Copy link
Contributor

larsoner commented Aug 3, 2024

Yikes I thought that would be simpler, thanks for working through it @QuLogic !

@larsoner larsoner merged commit 2d8401c into sphinx-gallery:master Aug 3, 2024
18 checks passed
@QuLogic QuLogic deleted the backref-full-module branch August 3, 2024 20:33
@lucyleeow
Copy link
Contributor

lucyleeow commented Aug 7, 2024 •
edited
Loading

Stupid question, could we have also tested this by adding mpl mpl_toolkits.mplot3d to doc_modules and adding a minigallery directive with Axes3D.plot ? (obv this seems more difficult than amending dummy)

clrpackages referenced this pull request in clearlinux-pkgs/pypi-sphinx_gallery Aug 9, 2024
@fenrus75
pypi-sphinx_gallery: Autospec creation for update from version 0.17.0…
90cfa79 
… 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)
@StefanieSenger StefanieSenger mentioned this pull request Sep 18, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@QuLogic @larsoner @lucyleeow