Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Sphinx extension: support captions in inline plots.#15304
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ca36e55 to99e2e27CompareForce-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... |
This commit adds a :caption: option to the plot directive provided by theSphinx extension (matplotlib.sphinxext.plot_directive). Without thisoption, there is no way to specify a caption for a plot generated frominline content.This is fully backwards-compatible. If a plot directive with a path to asource file has both a :caption: option and content provided, thecontent is used for the caption and the option is ignored.
99e2e27 to4c9917eCompareAnd another force-push to fix a flake8 error due to the above move of the import. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
| 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(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What are these warnings? I feel like long term we don't want to just ignore these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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; seepallets/jinja#1028. The other warning (inbuild_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.
With#18426 merged, is there anything here left to merge? |
The way of testing in this PR would be a plus... |
Closing in favour of#18426, Sorry for the PR overlap, but I think it was an honest mistake... |
PR Summary
This PR adds a
:caption:option to the plot directive provided by the Sphinx extensionmatplotlib.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