The Wayback Machine - https://web.archive.org/web/20210918081519/https://github.com/matplotlib/matplotlib/pull/10149
Skip to content

/ matplotlib Public

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 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

Plot directive preserve #10149

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Plot directive preserve #10149

wants to merge 13 commits into from

Conversation

@r-barnes
Copy link

@r-barnes r-barnes commented Jan 3, 2018

PR Summary

The Sphinx plot directive can be used to automagically generate figures for documentation like so:

.. plot::

   import matplotlib.pyplot as plt
   import matplotlib.image as mpimg
   import numpy as np
   img = mpimg.imread('_static/stinkbug.png')
   imgplot = plt.imshow(img)

But, if you reorder the figures in the documentation then all the figures may need to be rebuilt. This takes time. The names given to the figures are also fairly meaningless, making them more difficult to index by search engines or to find on a filesystem.

Alternatively, if you are compiling on a limited-resource service like ReadTheDocs, you may wish to build imagery locally to avoid hitting resource limits on the server. Using the new changes allows extensive dynamically generated imagery to be used on services like ReadTheDocs.

The :outname: property

These problems are addressed through two new features in the plot directive. The first is the introduction of the :outname: property. It is used like so:

.. plot::
   :outname: stinkbug_plot

   import matplotlib.pyplot as plt
   import matplotlib.image as mpimg
   import numpy as np
   img = mpimg.imread('_static/stinkbug.png')
   imgplot = plt.imshow(img)

Without :outname:, the figure generated above would normally be called, e.g. docfile3-4-01.png or something equally mysterious. With :outname: the figure generated will instead be named stinkbug_plot-01.png or even stinkbug_plot.png. This makes it easy to understand which output image is which and, more importantly, uniquely keys output images to code snippets.

The plot_preserve_dir configuration value

Setting the plot_preserve_dir configuration value to the name of a directory will cause all images with :outname: set to be copied to this directory upon generation.

If an image is already in plot_preserve_dir when documentation is being generated, this image is copied to the build directory thereby pre-empting generation and reducing computation time in low-resource environments.

Related issues were raised on ReadTheDocs (link) and on StackOverflow (link).

PR Checklist

  • Has Pytest style unit tests
  • It would, but I'm not sure how to go about writing them. Sorry :-/
  • Code is PEP 8 compliant
  • Leastwise, nothing new is added to the numerous pre-existing violations. PEP8 adherence makes code difficult to read :-/
  • 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
@tacaswell tacaswell added this to the v2.2 milestone Jan 3, 2018
@tacaswell
Copy link
Member

@tacaswell tacaswell commented Jan 3, 2018

👍 in principle, but the docs need to build cleanly!

jklymak
jklymak reviewed Jan 3, 2018
View changes
lib/matplotlib/sphinxext/plot_directive.py Outdated
@@ -656,6 +674,9 @@ def render_figures(code, code_path, output_dir, output_base, context,
for format, dpi in formats:
try:
figman.canvas.figure.savefig(img.filename(format), dpi=dpi)
if config.plot_preserve_dir and outname:
print("Preserving '{0}' into '{1}'".format(img.filename(format), config.plot_preserve_dir))
Copy link
Contributor

@jklymak jklymak Jan 3, 2018

Not reviewing, but I don't think you want print in here do you? I'd do _log.info or _log.debug?

Copy link
Author

@r-barnes r-barnes Jan 3, 2018

Yes, that's probably what I want, but I didn't know about them. I'll update.

Copy link
Author

@r-barnes r-barnes Jan 3, 2018

Actually, I don't see _log as a variable in the plot_directive.py namespace, so I'm not sure how to make that adjustment.

Copy link
Contributor

@jklymak jklymak Jan 3, 2018 •
edited

If its not already in this module, then up at the top:

# ... other imports
import logging
# ... other imports

_log = logging.getLogger(__name__)

Copy link
Author

@r-barnes r-barnes Jan 3, 2018

Thanks: got that up.

@r-barnes
Copy link
Author

@r-barnes r-barnes commented Jan 22, 2018

Eliminated some spaces

@jklymak jklymak removed this from the needs sorting milestone Mar 16, 2018
@jklymak jklymak added this to the v3.0 milestone Mar 16, 2018
@jklymak jklymak removed this from the v3.0 milestone Jul 9, 2018
@jklymak jklymak added this to the v3.1 milestone Jul 9, 2018
@tacaswell tacaswell removed this from the v3.1 milestone Feb 1, 2019
@tacaswell tacaswell added this to the v3.2 milestone Feb 1, 2019
@jklymak
Copy link
Contributor

@jklymak jklymak commented Jul 24, 2019

@r-barnes sorry this didn't get a second review yet. If you give it a rebase and ping me I'll give it a quick look and merge on the basis of @tacaswell 's approval. We are about to branch 3.2, so it'd be nice to get this in

@timhoffm
Copy link
Member

@timhoffm timhoffm commented Jul 24, 2019

Wouldn't plot_preserve_dir better be named plot_cache_dir?

tacaswell
tacaswell reviewed Aug 11, 2019
View changes
Copy link
Member

@tacaswell tacaswell left a comment

Mostly the indentation needs to be fixed.

@r-barnes I am going to do the rebase and force-push to your branch

lib/matplotlib/sphinxext/plot_directive.py Outdated Show resolved Hide resolved
lib/matplotlib/sphinxext/plot_directive.py Outdated
if config.plot_preserve_dir and outname:
outfiles = glob.glob(os.path.join(config.plot_preserve_dir, outname) + '*')
for of in outfiles:
_log.info("Copying preserved copy of '{0}' into '{1}'".format(of, build_dir))
Copy link
Member

@tacaswell tacaswell Aug 11, 2019

we are py36+ so we can use f-strings 😄

@tacaswell tacaswell force-pushed the plot_directive_preserve branch from e222429 to e238368 Aug 12, 2019
@tacaswell
Copy link
Member

@tacaswell tacaswell commented Aug 12, 2019

sorry, I read the year when this was opened wrong and thought it was from this January, not from 2 years ago.

@tacaswell tacaswell dismissed their stale review Aug 12, 2019

I did the re-base and added the leading spaces

@tacaswell tacaswell requested a review from alexrudy Aug 12, 2019
anntzer
anntzer approved these changes Aug 19, 2019
View changes
Copy link
Contributor

@anntzer anntzer left a comment

modulo tests passing

r-barnes added 3 commits Aug 19, 2019
@r-barnes @tacaswell
Added `:outname:` option and `plot_preserve_dir` configuration value
b8c5559
@r-barnes @tacaswell
Added What's New for plot directive changes
d4c41ff
@r-barnes @tacaswell
Futzing with PEP8 whitespace stuff.
69356da
r-barnes and others added 9 commits Aug 19, 2019
@r-barnes @tacaswell
Fix grammer
8b477a4
@r-barnes @tacaswell
Fix bug with adding null string to outname_list
fddd490
@r-barnes @tacaswell
Added a test of `:outname:`
872faa1
@r-barnes @tacaswell
Examples of using rst need to be code blocks
0ee71f0
@r-barnes @tacaswell
Use logging instead of print
082298e
@r-barnes @tacaswell
Satisfy PEP8 checks
73858c1
@tacaswell
FIX: issue with rebase due to re-naming of local variables
74e9fbc
@tacaswell
STY: fix line length and indentation issues
cd78416
@tacaswell
STY: trim whitespace and add trailing comma
725f25d
@tacaswell tacaswell force-pushed the plot_directive_preserve branch from e9fe4f7 to 725f25d Aug 19, 2019
@tacaswell tacaswell removed this from the v3.2.0 milestone Sep 4, 2019
@tacaswell tacaswell added this to the v3.3.0 milestone Sep 4, 2019
@timhoffm
Fix doc style
134aaf9
@timhoffm timhoffm force-pushed the plot_directive_preserve branch from 2c10b86 to 134aaf9 Nov 24, 2019
@timhoffm
Copy link
Member

@timhoffm timhoffm commented Nov 24, 2019

I took the liberty of pushing an update to fix the doc issues.

timhoffm
timhoffm reviewed Nov 24, 2019
View changes
Copy link
Member

@timhoffm timhoffm left a comment

Needs a rebase on master. Then move whats new entry to our new whats new scheme.

lib/matplotlib/sphinxext/plot_directive.py
@@ -519,7 +548,7 @@ def get_plot_formats(config):

def render_figures(code, code_path, output_dir, output_base, context,
function_name, config, context_reset=False,
close_figs=False):
close_figs=False, outname=''):
Copy link
Member

@timhoffm timhoffm Nov 24, 2019

Would go with outname=None, which is IMHO a more standard way for "not defined" than an empty string.

doc/users/next_whats_new/2018_12_03_sphinx_plot_preserve.rst
@@ -0,0 +1,61 @@
Plot Directive `outname` and `plot_preserve_dir`
Copy link
Member

@timhoffm timhoffm Nov 24, 2019

I propose to rename plot_preserve_dir to plot_cache_dir.

@QuLogic QuLogic removed this from the v3.3.0 milestone May 1, 2020
@QuLogic QuLogic added this to the v3.4.0 milestone May 1, 2020
@jklymak jklymak marked this pull request as draft Aug 24, 2020
@QuLogic QuLogic removed this from the v3.4.0 milestone Jan 21, 2021
@QuLogic QuLogic added this to the v3.5.0 milestone Jan 21, 2021
@QuLogic QuLogic removed this from the v3.5.0 milestone Aug 23, 2021
@QuLogic QuLogic added this to the v3.6.0 milestone Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivanov ivanov

@has2k1 has2k1

@jklymak jklymak

@timhoffm timhoffm

@anntzer anntzer

@tacaswell tacaswell

@alexrudy alexrudy

Assignees
No one assigned
Labels
status: needs rebase status: needs revision topic: sphinx extension
Projects
None yet
Milestone
v3.6.0
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
@r-barnes @tacaswell @jklymak @timhoffm @ivanov @has2k1 @anntzer @QuLogic