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

fix drawing error of fancyarrow of simple style #1129

Closed
wants to merge 1 commit into from

Conversation

leejjoon
Copy link
Contributor

fix drawing error of fancyarrow of simple style when two points are too close.
This addresses #566. And the initial patch was submitted by Rolf Barinka.

Current PR includes a file "examples/pylab_examples/arrow_test.py". I am planning to make it as a test before merge if others find this patch work okay.

…oo close. This addresses matplotlib#566. Initial patch submitted by Rolf Barinka.
@dmcdougall
Copy link
Member

@leejjoon This is what the output from your example looks like with the gtkagg backend:

example

Is this what you expect? The arrows look fine to me. If you make that example a test (and also maybe have it as an example as well), then this PR gets my +1 and it should probably be cherry-picked into v1.2.x.

Also, I think you might need to rebase against current master.

@efiring
Copy link
Member

efiring commented Oct 1, 2012

On 2012/09/30 11:14 PM, Damon McDougall wrote:

Also, I think you might need to rebase against current master.

Or, if it is supposed to go into 1.2.x, simply generate a new PR against
current 1.2.x.

@dmcdougall
Copy link
Member

@efiring Even better. Thanks.

@pelson
Copy link
Member

pelson commented Oct 3, 2012

Good round-up @dmcdougall - though I am less sure it is a bugfix and not a feature enhancement. Given that we have already got through two release candidates successfully (or 1 on windows), I would see this as a risk for 1.2 inclusion. @efiring : since you mentioned it, would you be in favour of it making 1.2?

@mdboom
Copy link
Member

mdboom commented Oct 3, 2012

I'm not sure on the bug vs. feature request issue. For me, it could go either way. I'm happy to go with the consensus of the crowd on this one.

@dmcdougall
Copy link
Member

In any case, there should be a test for this. If there isn't a test by the weekend it is likely to miss 1.2.

@efiring
Copy link
Member

efiring commented Oct 4, 2012

If @leejjoon can finish this off, and if it doesn't break any tests or examples, then I don't see too much risk to squeaking it into 1.2. This opinion is based on only a quick look at the PR and issue history, and a very quick look at the code. It has been described generally as a bugfix, and the original issue seemed to have involved an exception, not just suboptimal output, hence my impression that it really is a bugfix.

@leejjoon
Copy link
Contributor Author

leejjoon commented Oct 7, 2012

I opened a new PR (for v.1.2.x) : #1340
As far as I can see, this is a bug fix. All tests passes in my side.

@leejjoon leejjoon closed this Oct 7, 2012
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

5 participants