BUG: plot_directive: look for plot script files relative to the .rst file #385

Closed
wants to merge 1 commit into
from

5 participants

@pv
pv commented Jun 29, 2011

BUG: plot_directive: look for plot script files relative to the .rst file directory (rather than doc source root), when plot_basedir is not given

A tiny fix to the plot directive -- it should look for the plot scripts relative to the current .rst file and not the source root specified on the command line.

@pv pv BUG: plot_directive: look for plot script files relative to the .rst …
…file directory (rather than doc source root), when plot_basedir is not given
172cb0a
@mdboom
Matplotlib Developers member

This breaks matplotlib's documentation build. That could be fixed by setting plot_basedir, but I think a better approach (more consistent with Sphinx itself) would be to follow the pattern of the image directive: relative paths are relative to the rst, absolute paths are relative to the documentation root. See http://sphinx.pocoo.org/rest.html#images

@jdh2358

I believe this should be closed because it breaks mpl' documentation and has not been updated since Feb. Any objections?

@mdboom
Matplotlib Developers member

I think we should just delay it. Ideally, the plot directive should behave the same way as Sphinx/docutils built-in image directive for consistency. Once that is achieved, there are some changes to the doctree to make, of course.

@WeatherGod
Matplotlib Developers member

Does this pull request need to be redone?

@pelson
Matplotlib Developers member

Does this PR make the plot directive consistent with the sphinx directive? If so, we should get it in and fix our documentation, if not, then this issue should be closed (and not merged).

@mdboom
Matplotlib Developers member

I don't believe this PR does make it consistent with the Sphinx image directive.

@pelson
Matplotlib Developers member

@pv: Based on the previous comments, I am closing this PR. If you think that we have mis-understood your change, please feel free to show us the error. We appreciate you taking the time to make this PR and for helping us to make mpl better.

@pelson pelson closed this Sep 4, 2012
@pv
pv commented Nov 15, 2012

The image:: directive handles the path argument as a relative to the source file (which was implemented in this PR), unless it begins with / in which case it's relative to the top dir (which was not implemented).

That would be a simple change to make, but I don't think that it's worth doing any more, as it just breaks backwards compatibility for a minor consistency improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment