Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Plot directive preserve#10149
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
Plot directive preserve#10149
Uh oh!
There was an error while loading.Please reload this page.
Conversation
👍 in principle, but the docs need to build cleanly! |
| @@ -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)) | |||
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.
Not reviewing, but I don't think you wantprint in here do you? I'd do_log.info or_log.debug?
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.
Yes, that's probably what I want, but I didn't know about them. I'll update.
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.
Actually, I don't see_log as a variable in theplot_directive.py namespace, so I'm not sure how to make that adjustment.
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.
If its not already in this module, then up at the top:
# ... other importsimportlogging# ... other imports_log=logging.getLogger(__name__)
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: got that up.
Uh oh!
There was an error while loading.Please reload this page.
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 |
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.
Mostly the indentation needs to be fixed.
@r-barnes I am going to do the rebase and force-push to your branch
Uh oh!
There was an error while loading.Please reload this page.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
we are py36+ so we can use f-strings 😄
e222429 toe238368Comparesorry, 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
Uh oh!
There was an error while loading.Please reload this page.
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.
modulo tests passing
e9fe4f7 to725f25dCompare2c10b86 to134aaf9CompareI took the liberty of pushing an update to fix the doc issues. |
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.
Needs a rebase on master. Then move whats new entry to our new whats new scheme.
| @@ -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=''): | |||
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.
Would go withoutname=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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I propose to renameplot_preserve_dir toplot_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