sphinx plot directive: sources relative to rst file

[183amir]

PR Summary

According to the documentation when plot_basedir is empty or None.
The source-code files are found relative to the document that contains
them. However, this was not true in the actual implementation.
Fixes #10350

PR Checklist

• Has Pytest style unit tests
• Code is Flake 8 compliant
• New features are documented, with examples if plot related
• Documentation is sphinx and numpydoc compliant
• Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
• Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

[anntzer]

This needs at least a note in the api_changes explaining the change and how people relying on the old behavior need to update their code.
In practice I think(?) "absolute" paths should still be detected and understood relative to srcdir; this is consistent with how sphinx treats

:doc:`/absolute/path`

[183amir]

In practice I think(?) "absolute" paths should still be detected and understood relative to srcdir

What happens when you have both an "absolute" path and have also set the plot_basedir option as well?

[anntzer]

Uh.

I guess the following behavior would have been reasonable, and covers all use cases relatively easily, with a relatively simple upgrade path (just use absolute paths):

• treat empty plot_basedir as plot_basedir = srcdir
• treat relative paths as relative to the current file's directory
• treat absolute paths as relative to plot_basedir

But that's neither what was previously used, nor what was previously documented. Moreover, this means that docs won't be buildable with both old and new versions of mpl. (The design proposed in the PR has the same issue I think?)

One standard-ish way to handle the transition period would be to introduce a global plot_path_resolution_method = "old" / "new" (names up to bikeshedding) in conf.py and switch the behavior based on that, then we can add deprecation for the "old" method and default to "new". A bit painful, but heh...

[183amir]

I don't know where to put the API changes text so I'll just post it here for now:

Fixed a bug in the sphinx plot directive (.. plot:: path/to/plot_script.py)
where the source Python file was not being found relative to the directory of
the file containing the directive.

Documents that were using this feature may need to adjust the path argument
given to the plot directive. Two options are available:
1. Use absolute paths to find the file relative the ``plot_basedir`` (which
   defaults to the directory where conf.py is).
2. Use relative paths and the file is found relative to the directory of the
   file containing the directive.

[183amir]

I have done the new method here and provided fixes to the broken documentation. I'm not sure if I can implement the deprecation mechanism.

[anntzer]

The api changes note needs to go to this directory: https://github.com/matplotlib/matplotlib/tree/master/doc/api/next_api_changes.

I'm fine with the change but let's give other devs the time to chime in especially re: behavior change.

[tacaswell]

I am a bit concerned about this, I expect it to break the docs of a fair number of down-stream libraries that use the plot directive as if they are using paths at all they will need to be changed. Atleast scipy and basemap use it (ex

(bleeding) ✔ ~/source 
20:42 $ ack --rst ".. plot:: "
other_source/scipy/doc/source/tutorial/stats.rst
592:.. plot:: tutorial/examples/normdiscr_plot1.py
597:.. plot:: tutorial/examples/normdiscr_plot2.py
953:.. plot:: tutorial/stats/plots/kde_plot2.py
967:.. plot:: tutorial/stats/plots/kde_plot3.py
1010:.. plot:: tutorial/stats/plots/kde_plot4.py
1064:.. plot:: tutorial/stats/plots/kde_plot5.py

[183amir]

I understand and I think having a deprecation period and being able to specify the old and new method is important here (as suggested).

[jklymak]

Not sure if I see this being resolved before 3.1. Not quite sure I understand why this isn't simply a documentation issue...

[183amir]

@jklymak this is an implementation issue as explained #10350 and we have a fix for it here. The problem is this fix is going to break a lot of packages that depended on this bug (even matplotlib itself). So we'll have to have a deprecation period and support both methods for a while and then remove the old method after a while. Now, I have implemented the new method but was hoping a maintainer would take it from here on and implement the deprecation.

[anntzer]

@tacaswell Can you make the call regarding the API change? I think the new behavior is nicer, but will be a bit disruptive for downstream projects.

[tacaswell]

I agree that an absolute path out to the host file system is probably not what anyone wants.

We may want to add a "old-but-dont-warn" option. For down-stream projects they will want to be able to build their docs with a range of mpl versions, at least for a while.

Maybe rename the two modes to file-relative and root-relative? They are more descriptive and less judgmental.

I am 👍 on moving our docs to use file-relative.

[anntzer]

How does this interplay (if at all) with #13858?

[jklymak]

Converting to draft. This seems like it will not get fixed, but rather we should properly document the existing behaviour?