Fix for the 0-length Connection Path with Fancy Arrow bug #3930 #4103
Conversation
|
Thank you! I'm not familiar with this part of the code so I'm not reviewing the actual patch, but I have a couple of suggestions for the test. First, I don't think that it is necessary to include any file type other than png; the problem being fixed should not be in any way backend-dependent, should it? You can use the |
|
Thank you for the suggestions! |
|
Can you also use This is probably someplace where we need to improve our documentation as I think the best practices exist primarily in the collective heads of the current maintainers. |
|
Got it. Rebase done! Hmm... I'm having problem passing the pep8 testing on Travis CI. Most of the warnings involve files which I didn't modify. Please help. |
Fixed formatting so nothing exceeds column 80 Fixed formatting, removed .pdf and .svg for image test, added remove_text option Removed a trailing whitespace Removed another trailing whitespace in test_patches Rebased all commits into 2
|
@yangineer No worries. @tacaswell has fixed the immediate problem via #4104. |
| def test_patch_zerolength_fancy(): | ||
| # Github issue #3930 found a bug in the Fancy patch where a zero-length | ||
| # connection path would result in no plot being shown due to a | ||
| # NoneIntersectingPathException |
There was a problem hiding this comment.
typo: NonIntersectingPathException
Renamed the vertex in CLOSEPOLY so it seems more reasonable
| @@ -3758,50 +3757,64 @@ def transmute(self, path, mutation_size, linewidth): | |||
|
|
|||
| # path for head | |||
| in_f = inside_circle(x2, y2, head_length * .8) | |||
| path_out, path_in = \ | |||
| try: | |||
| path_out, path_in = \ | |||
There was a problem hiding this comment.
can you get rid of the \ or is that the only option to satisfy pep8 here?
There was a problem hiding this comment.
Yes, got rid of the backslash, managed to fit within 79 columns.
|
@efiring Who understands this bit of the code well enough to review this? |
|
I think this is @leejjoon's domain. |
|
@yangineer This needs a re-base. Does this conflict with #4173 ? |
|
@yangineer At a minimum this requires a re-base. |
|
Looks like this was fixed by #4173. |
Greetings!
I've added code to handle the NonIntersectingPathException in Fancy patches so that Connection Path of zero-length will plot as expected.
This solves the issue in #3930.
I also added an image comparison test for the fix.