Conversation
|
👍 in principle, but the docs need to build cleanly! |
| 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)) |
There was a problem hiding this comment.
Not reviewing, but I don't think you want print in here do you? I'd do _log.info or _log.debug?
There was a problem hiding this comment.
Yes, that's probably what I want, but I didn't know about them. I'll update.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
If its not already in this module, then up at the top:
# ... other imports
import logging
# ... other imports
_log = logging.getLogger(__name__)|
Eliminated some spaces |
|
@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 |
|
Wouldn't |
| 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)) |
There was a problem hiding this comment.
we are py36+ so we can use f-strings 😄
e222429 to
e238368
Compare
|
sorry, I read the year when this was opened wrong and thought it was from this January, not from 2 years ago. |
I did the re-base and added the leading spaces
e9fe4f7 to
725f25d
Compare
2c10b86 to
134aaf9
Compare
|
I took the liberty of pushing an update to fix the doc issues. |
timhoffm
left a comment
There was a problem hiding this comment.
Needs a rebase on master. Then move whats new entry to our new whats new scheme.
| def render_figures(code, code_path, output_dir, output_base, context, | ||
| function_name, config, context_reset=False, | ||
| close_figs=False): | ||
| close_figs=False, outname=''): |
There was a problem hiding this comment.
Would go with outname=None, which is IMHO a more standard way for "not defined" than an empty string.
| @@ -0,0 +1,61 @@ | |||
| Plot Directive `outname` and `plot_preserve_dir` | |||
There was a problem hiding this comment.
I propose to rename plot_preserve_dir to plot_cache_dir.
|
See #25091 (comment) for context on closing. |
PR Summary
The Sphinx plot directive can be used to automagically generate figures for documentation like so:
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:propertyThese 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:Without
:outname:, the figure generated above would normally be called, e.g.docfile3-4-01.pngor something equally mysterious. With:outname:the figure generated will instead be namedstinkbug_plot-01.pngor evenstinkbug_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_dirconfiguration valueSetting the
plot_preserve_dirconfiguration 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_dirwhen 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