FIX: Avoid copying source script when plot_html_show_source_link is False in plot directive
#20814
Conversation
|
Thank you for opening your first PR into Matplotlib! If you have not heard from us in a while, please feel free to ping You can also join us on gitter for real-time discussion. For details on testing, writing docs, and our review process, please see the developer guide We strive to be a welcoming and open project. Please follow our Code of Conduct. |
|
This seems reasonable and correct to me. Could you possibly add a few more asserts to https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/tests/test_sphinxext.py to make sure the source is and is not copied in the relevant cases? The rst we are running against is https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/tests/tinypages/some_plots.rst |
|
Thank you for working on this and welcome |
|
@tacaswell Added a few asserts. Could you take another look? |
| # `plot_html_show_source_link=0` is equivalent to | ||
| # `plot_html_show_source_link=False` |
| # `plot_html_show_source_link=False` | ||
| build_sphinx_html(source_dir, doctree_dir, html_dir, | ||
| extra_args=["-D", "plot_html_show_source_link=0"]) | ||
| assert "some_plots-1.py" not in [p.name for p in html_dir.iterdir()] |
|
This seems reasonable, however I have a concern that this is going to add time to the test process as tinypages was already the slowest test: On a recent default-branch run: https://github.com/matplotlib/matplotlib/runs/3301881275?check_suite_focus=true#step:12:149 and on this PR https://github.com/matplotlib/matplotlib/pull/20814/checks?check_run_id=3301939622#step:12:149 I assume we are hitting some caching in sphinx so the second run does not double the run time, but are still adding 5s (I'm a bit torn how much to trust these times though as CI runtime is notoriously variable). I think it would be better to add another plot to the example file? |
PR Summary
In the plot directive, if
plot_html_show_source_linkisFalse, it's not necessary to copy the source scripts (sorry if I'm missing something). This change slightly decreases disk usage.PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).