Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Open
wants to merge 15 commits into
base: main
Choose a base branch
Loading
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 67 additions & 4 deletions 71 lib/matplotlib/sphinxext/plot_directive.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@

The ``.. plot::`` directive supports the following options:

``:output-base-name:`` : str
Copy link
Member

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.

Copy link
Author

@asmeurer asmeurer Oct 14, 2024

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

Copy link
Member

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:

  • would image-base-name be better (because more concrete) than output-base-name?
  • what happens / do we need to check and error if the user tries to give more than a base name, e.g. :output-base-name: ../escaped_from_image_dir/myimage or nested_dir/myimage?

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.

Expand Down Expand Up @@ -165,6 +171,7 @@
and *TEMPLATE_SRCSET*.
"""

from collections import defaultdict
import contextlib
import doctest
from io import StringIO
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name is a bit generic. Maybe

Suggested change
env.mpl_custom_base_names = defaultdict(set)
env.mpl_plot_image_basenames = defaultdict(set)

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:
Copy link
Member

Choose a reason for hiding this comment

The 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 other.mpl_custom_base_names into env.mpl_custom_base_names. I.e. we can directly loop over that.

Copy link
Author

Choose a reason for hiding this comment

The 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()
Copy link
Member

Choose a reason for hiding this comment

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

Check warning on line 352 in lib/matplotlib/sphinxext/plot_directive.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/sphinxext/plot_directive.py#L351-L352

Added lines #L351 - L352 were not covered by tests
other.mpl_custom_base_names[docname])

# -----------------------------------------------------------------------------
# Doctest handling
# -----------------------------------------------------------------------------
Expand Down Expand Up @@ -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(

Check warning on line 646 in lib/matplotlib/sphinxext/plot_directive.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/sphinxext/plot_directive.py#L646

Added line #L646 was not covered by tests
f"The output-base-name "
f"{output_base}' is used multiple times.")
raise PlotError(f"The output-base-name "

Check warning on line 649 in lib/matplotlib/sphinxext/plot_directive.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/sphinxext/plot_directive.py#L649

Added line #L649 was not covered by tests
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,
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
config = document.settings.env.config
env = document.settings.env
env = document.settings.env
config = env.config

nofigs = 'nofigs' in options

if config.plot_srcset and setup.app.builder.name == 'singlehtml':
Expand All @@ -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
Expand Down Expand Up @@ -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)

Check warning on line 835 in lib/matplotlib/sphinxext/plot_directive.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/sphinxext/plot_directive.py#L834-L835

Added lines #L834 - L835 were not covered by tests
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', '')

Expand Down
3 changes: 3 additions & 0 deletions 3 lib/matplotlib/tests/test_sphinxext.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ def plot_directive_file(num):
assert filecmp.cmp(range_6, plot_file(17))
# plot 22 is from the range6.py file again, but a different function
assert filecmp.cmp(range_10, img_dir / 'range6_range10.png')
# plots 23 and 24 use a custom base name
assert filecmp.cmp(range_4, img_dir / 'custom-base-name-1.png')
assert filecmp.cmp(range_6, img_dir / 'custom-base-name-2.png')

# Modify the included plot
contents = (tmp_path / 'included_plot_21.rst').read_bytes()
Expand Down
12 changes: 12 additions & 0 deletions 12 lib/matplotlib/tests/tinypages/some_plots.rst
Original file line number Diff line number Diff line change
Expand Up @@ -174,3 +174,15 @@ Plot 21 is generated via an include directive:
Plot 22 uses a different specific function in a file with plot commands:

.. plot:: range6.py range10

Plots 23 and 24 use output-base-name.

.. plot::
:output-base-name: custom-base-name-1

plt.plot(range(4))

.. plot::
:output-base-name: custom-base-name-2

plt.plot(range(6))
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.