GeezerVison

Hi @timhoffm, thanks for the feedback!

One question for understanding: Is the following correct?

Before we just rendered every format in plot_formats and the renderer just picked the suitable one - and exactly only one. In practice a user would put all formats they need in plot_format. Exception: With plot_html_show_formats the HTML builder uses all formats because it embeds them as links.

Yes, this is correct!

A per-builder dict seems sensible to me, too, so I added that in 8a12b54, along with a warning for unsupported file formats in 5b9a15b. Could you PTAL when you have a chance?

I kept the default as the current list instead of the dict, because there are currently many projects using a list (see code search results, though there is a lot of noise too), and I think it might be better to not bother them with a warning :D The dict is purely additive, so that projects can switch to it at their convenience. However, I can still add a warning suggesting they switch their lists to a dict if you insist.

Additionally, I asked Claude Code to review my diff, and it caught a latent regression that the PR's behaviour surfaced, namely that we now need to record which documents contain plot directives. This is because it found that a PDF build which ran after an HTML build, i.e., with a reused doctree directory, failed to find previously generated PDFs (when we were building all formats, prior to this PR), and as a result ended up embedding low-quality raster PNGs instead of vector PDF figures into the PDF output. This is the case, for example, for users building documentation with plot directives on Read the Docs with PDF outputs enabled, since builds there rely on such caching. I have fixed this in 9c07cff.

Thanks again!

There's a decent amount of complexity here. I'll try to think this through from scratch, because I feel incrementally attaching more and more logic makes this hard to comprehend.

Simple/standard case: one build

1. Depending on the builder, we may need no, one or multiple files for each plot. Absent user configuration, we'd want to provide builder-specific defaults. A dict is a reasonable representation:

   DEFAULT_PLOT_FORMATS = {
       'html': ['png', 'pdf'],
       'latex': ['pdf'],
       'text': [],
       ...
   }

   Most builders will only need one, but html is the prominent exception, which motivates the more general approach.

   Discarded alternative

   I briefly thought that it may be better to attach the default plot format to the builder - this would third-party builders give the option to specify default formats. But that doesn't work. We don't have authority to attach an attribute to the builtin sphinx builders, so the default configuration has to live in the plot_directive extension.

   Details

2. Users should be able to override them, using the same dict format in conf.py

   plot_formats = {
       'html': ['svg'],
   }

3. Third-party builders are not in DEFAULT_PLOT_FORMATS. We should decide what to do in that case - either render nothing or raise or for backward-compatibility the current default. I'd go with the current default: ('png', 'hires.png', 'pdf'). We may later deprecate this and decide to choose one of the other alternatives.

   In general, users of third-party builders may need to specify their builder in plot_formats to get proper output. We can't magically guess what the builder needs.

4. Backward compatibility for explicitly set plot_formats lists: Such a list would take precedence over DEFAULT_PLOT_FORMATS - effectively as if one had set plot_formats = {setup.app.builder: explicit_list}.
   This usage is discouraged but currently not deprecated.

Validation

For the selected builder, it should be verified that plot_formats[builder] only contains supported formats.
We may later decide whether a pure list should be checked as well. - Possibly not right now.

Multiple builders

The cache reuse across builders complicates the situation. I can see two approaches:

a. as proposed above, track the files and build missing files in the second run.
b. alternatively, add an expert option to generate plots for more builders than only the current one. Advantage is that we don't need the complexity of tracking files, and saving to multiple output formats is faster. If all images can be generated in the first build, we only create the figure once and call savefig multiple times. The second buider can then rely on the cache. In contrast, generating additional image files in the second builder on demand can be substantially more costly. We have to recreate the whole figure.

I'm undecided here. (a) is a bit cleaner, but more complex and costly. (b) would be as simple as an optional plot_for_builder = ["html", "latex"] (or most likely -Dplot_for_builder=html,latex, because you'd typlically run this in scripts.
This would save the union of the plot_formats of all given builders.
The cache key would then simply be the set of saved formats. And the cache-usage criterion would be whether the requested formats is a subset of the saved formats.

I'm leaning towards (b)

What do you think?