FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Sphinx extension: support captions in inline plots. by bcbnz · Pull Request #15304 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Sphinx extension: support captions in inline plots. - #15304

Closed
bcbnz wants to merge 1 commit into
matplotlib:masterfrom
bcbnz:sphinxext-inline-caption
ConversationCommits (1)ChecksFiles changed
Closed

Sphinx extension: support captions in inline plots.#15304
bcbnz wants to merge 1 commit into
matplotlib:masterfrom
bcbnz:sphinxext-inline-caption

Conversation

bcbnz commented Sep 19, 2019

Copy link
Copy Markdown

PR Summary

This PR adds a :caption: option to the plot directive provided by the Sphinx extension matplotlib.sphinxext.plot_directive. Without this option, there is no way to specify a caption for a plot generated from inline content.

This is fully backwards-compatible. If a plot directive with a path to a source file has both a :caption: option and content provided, the content is used for the caption and the option is ignored.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

bcbnz force-pushed the sphinxext-inline-caption branch from ca36e55 to 99e2e27 Compare September 19, 2019 12:37

bcbnz commented Sep 19, 2019

Copy link
Copy Markdown
Author

Force-pushed a new version which should fix the failing tests -- the docutils import should have been after the pytest.importorskip check that Sphinx was installed.

Incidentally, this means the CI doesn't have Sphinx installed and so these tests aren't actually run anyway...

Sphinx extension: support captions in inline plots.
4c9917e 
This commit adds a :caption: option to the plot directive provided by the
Sphinx extension (matplotlib.sphinxext.plot_directive). Without this
option, there is no way to specify a caption for a plot generated from
inline content.

This is fully backwards-compatible. If a plot directive with a path to a
source file has both a :caption: option and content provided, the
content is used for the caption and the option is ignored.
bcbnz force-pushed the sphinxext-inline-caption branch from 99e2e27 to 4c9917e Compare September 19, 2019 13:03

bcbnz commented Sep 19, 2019

Copy link
Copy Markdown
Author

And another force-push to fix a flake8 error due to the above move of the import.

dstansby reviewed Sep 27, 2019
View reviewed changes

dstansby left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for the PR 🚀 - I think this looks like a good addition, and the code looks good to me (just one question). I'm not too experienced with testing these things, so will let someone else chime in on that part.

Comment thread lib/matplotlib/tests/test_sphinxext.py
from docutils.nodes import caption, figure # noqa: E402

# Sphinx has some deprecation warnings we don't want to turn into errors.
with warnings.catch_warnings():

dstansby Sep 27, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

What are these warnings? I feel like long term we don't want to just ignore these.

bcbnz Sep 27, 2019 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It's Jinja (imported by Sphinx) giving a DeprecationWarning: "Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop." This has been fixed in Jinja's master and should be included in the next release; see pallets/jinja#1028. The other warning (in build_test_doc()) is the same warning, also from Jinja and should also be fixed in the next release.

These warnings are only raised when importing Sphinx for test purposes (not in the actual library) so I don't think matplotlib needs to convert them to test failures.

ulijh mentioned this pull request Sep 8, 2020
Merged
6 of 7 tasks

dopplershift commented Sep 12, 2020

Copy link
Copy Markdown
Contributor

With #18426 merged, is there anything here left to merge?

ulijh commented Sep 16, 2020

Copy link
Copy Markdown
Contributor

The way of testing in this PR would be a plus...

jklymak commented Apr 23, 2021

Copy link
Copy Markdown
Member

Closing in favour of #18426, Sorry for the PR overlap, but I think it was an honest mistake...

jklymak closed this Apr 23, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

dstansby dstansby left review comments

Assignees

No one assigned

Labels

topic: sphinx extension

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL