Repository: matplotlib/matplotlib
Pull request: Fix plot directive with func calls
## PR Summary

This will close #20656 .

I started but did not get through writing tests, I think I see a way to test just `render_figures` in a self-contained way, but wanted to push this up so it did not get lost.

## PR Checklist

<!-- Please mark any checkboxes that do not apply to this PR as [N/A]. -->
**Tests and Styling**
- [ ] Has pytest style unit tests (and `pytest` passes).
- [ ] Is [Flake 8](https://flake8.pycqa.org/en/latest/) compliant (install `flake8-docstrings` and run `flake8 --docstring-convention=all`).

**Documentation**
- [ ] New features are documented, with examples if plot related.
- [ ] New features have an entry in `doc/users/next_whats_new/` (follow instructions in README.rst there).
- [ ] API changes documented in `doc/api/next_api_changes/` (follow instructions in README.rst there).
- [ ] Documentation is sphinx and numpydoc compliant (the docs should [build](https://matplotlib.org/devel/documenting_mpl.html#building-the-docs) without error).


Problem statement:
Sphinx extension plot_directive not able to detect function
### Bug report

**Bug summary**

When writing a generical code to generate plots with sphinx documentation, this function cannot be called by other functions from the documentation in the same file

**Code for reproduction**

Repository layout:

- docs
   - source
     - pyplots
       - test.py
     -  conf.py
     - index.rst
- package

Content of file `test.py`

```python
import matplotlib.pyplot as plt


def generate_plot_according_to_step(step):

    plt.figure()

    plt.plot([1, 2], [2, 3])

    if step >= 1:
        plt.title("Adding a title")

    if step >= 2:
        plt.suptitle('Adding a suptitle')

    plt.show()


def step0():
    generate_plot_according_to_step(0)


def step1():
    generate_plot_according_to_step(1)


def step2():
    generate_plot_according_to_step(2)
```

Content of `conf.py` :

```python
extensions = [
        'sphinx.ext.autodoc',
        'matplotlib.sphinxext.plot_directive'
]
```

Then when referencing the steps functions in docstring anywhere in the package, such as:

```python
"""
.. plot:: pyplots/test.py step0

.. plot:: pyplots/test.py step1

.. plot:: pyplots/test.py step2

"""
```
**Actual outcome**

When building the doc through the following steps:

```powershell


cd .\docs

sphinx-quickstart

sphinx-apidoc -f -o source/ ../

.\make.bat html
```

The following message arise

```
 WARNING: Exception occurred in plotting test
 from path\to\repository\docs\source\pyplots/test.py:
Traceback (most recent call last):
  File "path\to\repository\.venv\lib\site-packages\matplotlib\sphinxext\plot_directive.py", line 485, in run_code
    exec(function_name + "()", ns)
  File "<string>", line 1, in <module>
NameError: name 'step0' is not defined
```

**Expected outcome**

It seems that it is not possible to use a generical function such as the one presented to generate documentation.
Either it is considered a bug and should be fixed or the error message should be more explicit, as it is currently inaccurate, as the step functions exists

**Matplotlib version**

  * Operating system: Windows 10
  * Matplotlib version (`import matplotlib; print(matplotlib.__version__)`):  3.4.2
  * Matplotlib backend (`print(matplotlib.get_backend())`): TkAgg
  * Python version: 3.9.5
  * Other libraries: Sphinx 4.0.2

Every library has been installed through pip

Hints from issue discussion:
I'm also having this problem.
This occurs because we currently textually split the code at instances of plt.show() that appear as literal strings in the source, and hope that the blocks are independently runnable.  Instead, in the case where a function name is given, we should execute the whole code in a context where plt.show() is patched out to do, well, whatever we need it to do.  Probably needs a bit of surgery, but hopefully manageable.
While I was writing this up @anntzer also tracked down the first bug, but there is a second bug in that the de-duplication logic in  in plot_directive means that only the first `.. plot` gets run because the outputfile is named solely based on the file name, notices the file exists and then short-circuits as done!

I'll have a PR with both fixes shortly, but I have a slightly different idea how to fix the incorrect splitting (only split on `plt.show` that is fully de-dented).   I think we are already patching out `plt.show` (via patching `FigureManagerBase.show`) in the context where we are running the code.

Commit message: MNT: pass all input to render_figures an keyword

There are too many positional arguments and the names are subtly
different.

Patch to review:
diff --git a/lib/matplotlib/sphinxext/plot_directive.py b/lib/matplotlib/sphinxext/plot_directive.py
index 671484790d66..9f4cc430d079 100644
--- a/lib/matplotlib/sphinxext/plot_directive.py
+++ b/lib/matplotlib/sphinxext/plot_directive.py
@@ -342,21 +342,25 @@ def split_code_at_show(text):
     return _split_code_at_show(text)[1]
 
 
-def _split_code_at_show(text):
+def _split_code_at_show(text, function_name):
     """Split code at plt.show()."""
-    parts = []
+
     is_doctest = contains_doctest(text)
-    part = []
-    for line in text.split("\n"):
-        if (not is_doctest and line.strip() == 'plt.show()') or \
-               (is_doctest and line.strip() == '>>> plt.show()'):
-            part.append(line)
+    if function_name is None:
+        parts = []
+        part = []
+        for line in text.split("\n"):
+            if (not is_doctest and line.startswith('plt.show')) or \
+                   (is_doctest and line.strip() == '>>> plt.show()'):
+                part.append(line)
+                parts.append("\n".join(part))
+                part = []
+            else:
+                part.append(line)
+        if "\n".join(part).strip():
             parts.append("\n".join(part))
-            part = []
-        else:
-            part.append(line)
-    if "\n".join(part).strip():
-        parts.append("\n".join(part))
+    else:
+        parts = [text]
     return is_doctest, parts
 
 
@@ -566,11 +570,13 @@ def render_figures(code, code_path, output_dir, output_base, context,
     Save the images under *output_dir* with file names derived from
     *output_base*
     """
+    if function_name is not None:
+        output_base = f'{output_base}_{function_name}'
     formats = get_plot_formats(config)
 
     # Try to determine if all images already exist
 
-    is_doctest, code_pieces = _split_code_at_show(code)
+    is_doctest, code_pieces = _split_code_at_show(code, function_name)
 
     # Look for single-figure output files first
     all_exists = True
@@ -793,13 +799,13 @@ def run(arguments, content, options, state_machine, state, lineno):
 
     # make figures
     try:
-        results = render_figures(code,
-                                 source_file_name,
-                                 build_dir,
-                                 output_base,
-                                 keep_context,
-                                 function_name,
-                                 config,
+        results = render_figures(code=code,
+                                 code_path=source_file_name,
+                                 output_dir=build_dir,
+                                 output_base=output_base,
+                                 context=keep_context,
+                                 function_name=function_name,
+                                 config=config,
                                  context_reset=context_opt == 'reset',
                                  close_figs=context_opt == 'close-figs',
                                  code_includes=source_file_includes)


Code review comments:
diff_hunk: @@ -10,10 +10,31 @@
 import pytest
 
 
-pytest.importorskip('sphinx',
-                    minversion=None if sys.version_info < (3, 10) else '4.1.3')
+def skip_if_no_sphinx(func):
+    pytest.importorskip(
+        'sphinx',
+        minversion=None if sys.version_info < (3, 10) else '4.1.3')
+    return func
 
 
+def _build_sphinx_html(source_dir, doctree_dir, html_dir, extra_args=None):
path: lib/matplotlib/tests/test_sphinxext.py
text: @author:
@user1  I think you meant to comment here?  I can rename this if you want.

@user1:
Yes, i meant this. Slight preference for renaming, but as said not a big deal either way.