Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor and bugfixes for plot_directive #3257

Merged
merged 5 commits into from Jul 17, 2014

Conversation

matthew-brett
Copy link
Contributor

Fix ugly output when not linking to source
Remove copy of relpath as no-longer needed
Fix some typos
Remove unused imports
Some PEP8 whitespace

pep8 for plot directive.
Matplotlib now depends on Python 2.6, which has relpath
Well, behaviour -> behavior is against my origins, but hey, we're all
American now.
plot_directive displaying empty parens when no figures and no source
required.  Was also prepending a comma to list of images when there was
source links not required.
@mdboom
Copy link
Member

mdboom commented Jul 14, 2014

Cool. I think @dmcdougall will probably also want to review this, since he's been digging into this file pretty deeply in the past couple of days.

@tacaswell tacaswell added this to the v1.4.0 milestone Jul 14, 2014
@@ -111,7 +111,7 @@

plot_apply_rcparams
By default, rcParams are applied when `context` option is not used in
a plot directive. This configuration option overrides this behaviour
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdboom git blame informs me that you wrote this line, but it's too early for me to think of anything facetious to say regarding the use of superfluous 'u's.

@dmcdougall
Copy link
Member

Looks good to me.

@dmcdougall
Copy link
Member

@matthew-brett Was there anywhere specific in the docs that didn't link to source? I'd like to do a local build to sanity check it works, and then I'll merge it.

@matthew-brett
Copy link
Contributor Author

I'm sorry, I'm ashamed to say I didn't test this other than on a doc I
was writing myself, with:

plot_include_source = True
plot_html_show_source_link = False

in the conf.py. I think you'll see the problem if you write a tiny
test rst file with plot directive with and without plots (one with
:nofigs:)...

On 7/15/14, Damon McDougall notifications@github.com wrote:

@matthew-brett Was there anywhere specific in the docs that didn't link to
source? I'd like to do a local build to sanity check it works, and then
I'll merge it.


Reply to this email directly or view it on GitHub:
#3257 (comment)

@dmcdougall
Copy link
Member

This doesn't break the links fixed in #3253; merging.

dmcdougall added a commit that referenced this pull request Jul 17, 2014
MRG: refactor and bugfixes for plot_directive
@dmcdougall dmcdougall merged commit 674044e into matplotlib:master Jul 17, 2014
@dmcdougall
Copy link
Member

@tacaswell Trying to cherry-pick the merge commit into v1.4.x and get this error message:

$ git cherry-pick 674044e
error: Commit 674044e8f0f8c4c9ef13d6a5b5fa15e4d7fbc53f is a merge but no -m option was given.
fatal: cherry-pick failed

I'm happy to cherry-pick each commit, unless there's a procedure you'd like me take to address this.

@tacaswell
Copy link
Member

Look at the parent list on the merge commit. Counting from 1 get the I'd of the main line (which is normally 1) and pass that to -m.

I think what is going on is that git could walk down either side of the loop to generate the diff, -m tells it which side is most like the other branch you want to pick too.

dmcdougall added a commit that referenced this pull request Jul 17, 2014
MRG: refactor and bugfixes for plot_directive
@tacaswell
Copy link
Member

Cherry-picked in 88fc024

@dmcdougall
Copy link
Member

@tacaswell Thanks!

jbmohler pushed a commit to jbmohler/matplotlib that referenced this pull request Aug 14, 2014
…irective

MRG: refactor and bugfixes for plot_directive
@QuLogic QuLogic changed the title MRG: refactor and bugfixes for plot_directive refactor and bugfixes for plot_directive Dec 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants