Movatterモバイル変換


[0]ホーム

URL:


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

Sign up
Appearance settings

sphinx plot directive: sources relative to rst file#12456

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
183amir wants to merge8 commits intomatplotlib:main
base:main
Choose a base branch
Loading
from183amir:patch-1

Conversation

183amir
Copy link

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 isFlake 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
Copy link
Contributor

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
Copy link
Author

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 theplot_basedir option as well?

@anntzer
Copy link
Contributor

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
Copy link
Author

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 ofthe file containing the directive.Documents that were using this feature may need to adjust the path argumentgiven 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
Copy link
Author

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
Copy link
Contributor

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

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.rst592:.. plot:: tutorial/examples/normdiscr_plot1.py597:.. plot:: tutorial/examples/normdiscr_plot2.py953:.. plot:: tutorial/stats/plots/kde_plot2.py967:.. plot:: tutorial/stats/plots/kde_plot3.py1010:.. plot:: tutorial/stats/plots/kde_plot4.py1064:.. plot:: tutorial/stats/plots/kde_plot5.py

@183amir
Copy link
Author

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

@jklymakjklymak modified the milestones:v3.1.0,v3.2.0Mar 3, 2019
@jklymak
Copy link
Member

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
Copy link
Author

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

183amirand others added6 commitsMarch 3, 2019 16:12
According to the documentation when plot_basedir is empty or None.The source-code files are found relative to the document that containsthem. However, this was not true in the actual implementation.Fixesmatplotlib#10350
Absolute paths are found relative plot_basedir which defaults tothe source directory. Relative paths are found relative to thedocument containing them.
@anntzer
Copy link
Contributor

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

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 tofile-relative androot-relative? They are more descriptive and less judgmental.

I am 👍 on moving our docs to usefile-relative.

@anntzer
Copy link
Contributor

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

@tacaswelltacaswell modified the milestones:v3.2.0,v3.3.0Sep 4, 2019
@QuLogicQuLogic modified the milestones:v3.3.0,v3.4.0May 2, 2020
@jklymakjklymak marked this pull request as draftDecember 20, 2020 19:26
@jklymak
Copy link
Member

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

@QuLogicQuLogic modified the milestones:v3.4.0,v3.5.0Jan 21, 2021
@QuLogicQuLogic modified the milestones:v3.5.0,v3.6.0Aug 21, 2021
@timhoffmtimhoffm modified the milestones:v3.6.0,unassignedApr 30, 2022
@story645story645 modified the milestones:unassigned,needs sortingOct 6, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
future releases
Development

Successfully merging this pull request may close these issues.

The sphinx plot directive does not find files relative to the document that contains them.
7 participants
@183amir@anntzer@tacaswell@jklymak@QuLogic@story645@timhoffm

[8]ページ先頭

©2009-2025 Movatter.jp