-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Add an image-basename option to the Sphinx plot directive #28187
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
base: main
Are you sure you want to change the base?
Changes from 7 commits
615017a
b432962
426abc7
8485bfd
f94a932
19daf49
8f05ba6
1fa88dd
a22fcc3
86fb167
e0be21e
f322125
fc33c38
7d416cf
ce23c88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -47,6 +47,12 @@ | |||||||
|
||||||||
The ``.. plot::`` directive supports the following options: | ||||||||
|
||||||||
``:output-base-name:`` : str | ||||||||
The base name (without the extension) of the outputted image files. The | ||||||||
default is to use the same name as the input script, or the name of | ||||||||
the RST document if no script is provided. The output-base-name for each | ||||||||
plot directive must be unique. | ||||||||
|
||||||||
``:format:`` : {'python', 'doctest'} | ||||||||
The format of the input. If unset, the format is auto-detected. | ||||||||
|
||||||||
|
@@ -165,6 +171,7 @@ | |||||||
and *TEMPLATE_SRCSET*. | ||||||||
""" | ||||||||
|
||||||||
from collections import defaultdict | ||||||||
import contextlib | ||||||||
import doctest | ||||||||
from io import StringIO | ||||||||
|
@@ -182,6 +189,7 @@ | |||||||
from docutils.parsers.rst.directives.images import Image | ||||||||
import jinja2 # Sphinx dependency. | ||||||||
|
||||||||
from sphinx.environment.collectors import EnvironmentCollector | ||||||||
from sphinx.errors import ExtensionError | ||||||||
|
||||||||
import matplotlib | ||||||||
|
@@ -265,6 +273,7 @@ | |||||||
'scale': directives.nonnegative_int, | ||||||||
'align': Image.align, | ||||||||
'class': directives.class_option, | ||||||||
'output-base-name': directives.unchanged, | ||||||||
'include-source': _option_boolean, | ||||||||
'show-source-link': _option_boolean, | ||||||||
'format': _option_format, | ||||||||
|
@@ -312,9 +321,37 @@ | |||||||
app.connect('build-finished', _copy_css_file) | ||||||||
metadata = {'parallel_read_safe': True, 'parallel_write_safe': True, | ||||||||
'version': matplotlib.__version__} | ||||||||
app.connect('builder-inited', init_filename_registry) | ||||||||
app.add_env_collector(_FilenameCollector) | ||||||||
return metadata | ||||||||
|
||||||||
|
||||||||
# ----------------------------------------------------------------------------- | ||||||||
# Handle Duplicate Filenames | ||||||||
# ----------------------------------------------------------------------------- | ||||||||
|
||||||||
def init_filename_registry(app): | ||||||||
env = app.builder.env | ||||||||
if not hasattr(env, 'mpl_custom_base_names'): | ||||||||
env.mpl_custom_base_names = defaultdict(set) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name is a bit generic. Maybe
Suggested change
Or similar. This naming is library_directive_option. |
||||||||
|
||||||||
|
||||||||
class _FilenameCollector(EnvironmentCollector): | ||||||||
def process_doc(self, app, doctree): | ||||||||
pass | ||||||||
|
||||||||
def clear_doc(self, app, env, docname): | ||||||||
if docname in env.mpl_custom_base_names: | ||||||||
del env.mpl_custom_base_names[docname] | ||||||||
|
||||||||
def merge_other(self, app, env, docnames, other): | ||||||||
for docname in docnames: | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to loop over the docnames? AFAICS, we only want to merge There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I simplified this function. I'm not completely sure about it, but the functionality still seems to work based on manual testing. |
||||||||
if docname in other.mpl_custom_base_names: | ||||||||
if docname not in env.mpl_custom_base_names: | ||||||||
env.mpl_custom_base_names[docname] = set() | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can be left out because mpl_custom_base_names is a defaultdict. |
||||||||
env.mpl_custom_base_names[docname].update( | ||||||||
other.mpl_custom_base_names[docname]) | ||||||||
|
||||||||
# ----------------------------------------------------------------------------- | ||||||||
# Doctest handling | ||||||||
# ----------------------------------------------------------------------------- | ||||||||
|
@@ -600,6 +637,22 @@ | |||||||
return srcset | ||||||||
|
||||||||
|
||||||||
def check_output_base_name(env, output_base): | ||||||||
docname = env.docname | ||||||||
|
||||||||
for d in env.mpl_custom_base_names: | ||||||||
if output_base in env.mpl_custom_base_names[d]: | ||||||||
if d == docname: | ||||||||
raise PlotError( | ||||||||
f"The output-base-name " | ||||||||
f"{output_base}' is used multiple times.") | ||||||||
raise PlotError(f"The output-base-name " | ||||||||
f"'{output_base}' is used multiple times " | ||||||||
f"(it is also used in {env.doc2path(d)}).") | ||||||||
|
||||||||
env.mpl_custom_base_names[docname].add(output_base) | ||||||||
|
||||||||
|
||||||||
def render_figures(code, code_path, output_dir, output_base, context, | ||||||||
function_name, config, context_reset=False, | ||||||||
close_figs=False, | ||||||||
|
@@ -723,6 +776,7 @@ | |||||||
def run(arguments, content, options, state_machine, state, lineno): | ||||||||
document = state_machine.document | ||||||||
config = document.settings.env.config | ||||||||
env = document.settings.env | ||||||||
Comment on lines
781
to
+782
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
nofigs = 'nofigs' in options | ||||||||
|
||||||||
if config.plot_srcset and setup.app.builder.name == 'singlehtml': | ||||||||
|
@@ -734,6 +788,7 @@ | |||||||
|
||||||||
options.setdefault('include-source', config.plot_include_source) | ||||||||
options.setdefault('show-source-link', config.plot_html_show_source_link) | ||||||||
options.setdefault('output-base-name', None) | ||||||||
|
||||||||
if 'class' in options: | ||||||||
# classes are parsed into a list of string, and output by simply | ||||||||
|
@@ -775,14 +830,22 @@ | |||||||
function_name = None | ||||||||
|
||||||||
code = Path(source_file_name).read_text(encoding='utf-8') | ||||||||
output_base = os.path.basename(source_file_name) | ||||||||
if options['output-base-name']: | ||||||||
output_base = options['output-base-name'] | ||||||||
check_output_base_name(env, output_base) | ||||||||
else: | ||||||||
output_base = os.path.basename(source_file_name) | ||||||||
else: | ||||||||
source_file_name = rst_file | ||||||||
code = textwrap.dedent("\n".join(map(str, content))) | ||||||||
counter = document.attributes.get('_plot_counter', 0) + 1 | ||||||||
document.attributes['_plot_counter'] = counter | ||||||||
base, ext = os.path.splitext(os.path.basename(source_file_name)) | ||||||||
asmeurer marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
output_base = '%s-%d.py' % (base, counter) | ||||||||
if options['output-base-name']: | ||||||||
output_base = options['output-base-name'] | ||||||||
timhoffm marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
check_output_base_name(env, output_base) | ||||||||
else: | ||||||||
counter = document.attributes.get('_plot_counter', 0) + 1 | ||||||||
document.attributes['_plot_counter'] = counter | ||||||||
output_base = '%s-%d.py' % (base, counter) | ||||||||
function_name = None | ||||||||
caption = options.get('caption', '') | ||||||||
|
||||||||
|
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.
Is this really only the filename base or can it be a relative or absolute path without extension? Either case should be documented. Also, since you start giving users control over the created files, you probably should mention where they go.
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.
It's just the file name. The files go wherever Sphinx puts them (it looks like it's in the
_images
directory, but I don't know if that's configurable).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 clarification. In that case
*-base-name
makes sense.Two further thoughts:
image-base-name
be better (because more concrete) thanoutput-base-name
?:output-base-name: ../escaped_from_image_dir/myimage
ornested_dir/myimage
?