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

Annotate bbox darrow #3194

Merged
merged 3 commits into from Jul 24, 2014
Merged

Annotate bbox darrow #3194

merged 3 commits into from Jul 24, 2014

Conversation

lkilcher
Copy link
Contributor

@lkilcher lkilcher commented Jul 7, 2014

This adds a double arrow to BoxStyle class.

@pelson
Copy link
Member

pelson commented Jul 7, 2014

Good idea. @leejjoon - would you mind taking a look?


return path

_style_list['darrow']=DArrow
Copy link
Member

Choose a reason for hiding this comment

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

needs spaces

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is fixed now.

@tacaswell tacaswell added this to the v1.5.x milestone Jul 7, 2014
@lkilcher
Copy link
Contributor Author

lkilcher commented Jul 7, 2014

I think I've added the spaces you were looking for.

@lkilcher lkilcher changed the title Annotate bbox d arrow Annotate bbox darrow Jul 7, 2014
@lkilcher
Copy link
Contributor Author

Is there anything I need to do to move this pull request forward? Do I need to do anything about the Travis CI error?

@tacaswell
Copy link
Member

I restarted the tests. The 1.5.x stuff is a bit on hold until we finish up 1.4.0, although now that we have created the 1.4.x branch we can start merging new stuff again.

@tacaswell
Copy link
Member

And yes, please fix the pep8 failures

@lkilcher
Copy link
Contributor Author

Thank you for getting back to me so quickly. I'm having trouble interpreting the Travis CI output. Does it tell me where/if I have pep8 failures? I see warnings/errors in other files, but none in files I have committed. Thank you. I've done my best to make my changes pep8 compliant.

@jenshnielsen
Copy link
Member

I think these issues were present at the time where you branched from master. You should probably rebase on current master to fix the errors if possible.

@lkilcher
Copy link
Contributor Author

Thanks for the help everyone!

@tacaswell
Copy link
Member

Could you and an image test to go with this?

@lkilcher
Copy link
Contributor Author

OK, I think I've added an image test, but the test is failing in the TCI build. I used the figures that were generated on my machine when I ran the test. I think I've disabled all of my matplotlib customizations. Is there something else going on here, or do I need to create a much cleaner (i.e. using virtualenv) matplotlib install and create the images with that? Do I need to change my backend? Other ideas?

@tacaswell
Copy link
Member

The issue isn't that the images are not matching, it's that you are getting errors during rendering


======================================================================
ERROR: matplotlib.tests.test_arrow_patches.test_boxarrow.test
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/testing/decorators.py", line 51, in failer
    result = f(*args, **kwargs)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/testing/decorators.py", line 183, in do_test
    figure.savefig(actual_fname, **self._savefig_kwarg)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/figure.py", line 1470, in savefig
    self.canvas.print_figure(*args, **kwargs)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/backend_bases.py", line 2192, in print_figure
    **kwargs)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/backends/backend_agg.py", line 513, in print_png
    FigureCanvasAgg.draw(self)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/backends/backend_agg.py", line 461, in draw
    self.figure.draw(self.renderer)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/artist.py", line 59, in draw_wrapper
    draw(artist, renderer, *args, **kwargs)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/figure.py", line 1079, in draw
    func(*args)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/artist.py", line 59, in draw_wrapper
    draw(artist, renderer, *args, **kwargs)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/text.py", line 551, in draw
    self._draw_bbox(renderer, posx, posy)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/text.py", line 522, in _draw_bbox
    self._bbox_patch.draw(renderer)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/artist.py", line 59, in draw_wrapper
    draw(artist, renderer, *args, **kwargs)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/patches.py", line 470, in draw
    path = self.get_path()
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/patches.py", line 2455, in get_path
    self.get_mutation_aspect())
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/patches.py", line 1883, in __call__
    return self.transmute(x0, y0, width, height, mutation_size)
  File "/home/travis/virtualenv/python2.7.6/lib/python2.7/site-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/patches.py", line 2028, in transmute
    x1, y1 = x0 + width, y0 + height
TypeError: unsupported operand type(s) for +: 'float' and 'tuple'

The other two are the same, just for the other backends.

We probably only need the png version of this test.

@lkilcher
Copy link
Contributor Author

OK. Now I'm really confused. Does anyone have any ideas as to why the python2 image tests are passing, but not python3?

FYI, I was getting the following warning when running tests on my local machine:

/usr/local/lib/python2.7/dist-packages/matplotlib-1.4.x-py2.7-linux-x86_64.egg/matplotlib/font_manager.py:1289: UserWarning: findfont: Could not match :family=Bitstream Vera Sans:style=normal:variant=normal:weight=normal:stretch=normal:size=medium. Returning /usr/share/fonts/truetype/jsmath/jsMath-cmti10.ttf

in order to create the test image I had to set USE_FONTCONFIG = True in the font_manager.py file. This seemed to fix that warning, and produce an image that passed the test on TCI, but it made a bunch of my local tests fail (because it couldn't find glyphs).

Anyway, I guess I'm talking about two issues here:

  1. Why isn't my install finding Bitstream Vera Sans, and
  2. Why does my image test pass for python 2, but not python3.

As always, thanks for all the help. I'm catching up slowly...

@tacaswell
Copy link
Member

That is odd.

Can you re-write history so that the pdf and svg tests never existed?

@lkilcher
Copy link
Contributor Author

I'm not entirely sure how to do that, if you don't mind providing an approximate list of steps I'd be grateful. How do you anticipate that would be helpful?

@jenshnielsen
Copy link
Member

@lkilcher I can reproduce the error on python3. Basically the problem is that get_styles returns a dictionary of the styles. This no longer has a fixed order and thus the order within the plot changes from run to run on python3. You should probably just make list of the keys and sort this before using this to iterate over the styles to ensure that the order is preserved.

The expected image
boxarrow_test_image-expected
The result image from python 3.4
boxarrow_test_image

@jenshnielsen
Copy link
Member

The simplest is probably

for i, stylename in enumerate(sorted(styles.keys())):

since you are as far as I can see only using the keys.

@tacaswell
Copy link
Member

The benefit of re writing history so that the other files were never there is to keep the repo size down

@lkilcher
Copy link
Contributor Author

Great! Any ideas on my font issue? I've deleted my ~/.matplotlib/fontList.cache, but that hasn't changed anything. That file doesn't get rebuilt when I import matplotlib (as it sounds like it should). Currently this is the file I'm getting, which clearly won't work.
boxarrow_test_image

lkilcher and others added 2 commits July 23, 2014 15:28
All bbox's in patches.BoxStyle are included in the test image.
@lkilcher
Copy link
Contributor Author

Holey smokes! It worked.

I went ahead and squashed all of my pep8 fixes and other silly bugs into the main commit. So, now there are just the two commits: the DArrow, and the test.

I created the test image with the USE_FONTCONFIG = True trick.

If anyone has ideas on why I don't have the Bitstream Vera Sans font without this trick, I'd be grateful.

@WeatherGod
Copy link
Member

@mdboom probably would know about the font issues, but he is out for the
week, I think, so I would ping him next week.

On Wed, Jul 23, 2014 at 5:53 PM, Levi Kilcher notifications@github.com
wrote:

Holey smokes! It worked.

I went ahead and squashed all of my pep8 fixes and other silly bugs into
the main commit. So, now there are just the two commits: the DArrow, and
the test.

I created the test image with the USE_FONTCONFIG = True trick.

If anyone has ideas on why I don't have the Bitstream Vera Sans font
without this trick, I'd be grateful.


Reply to this email directly or view it on GitHub
#3194 (comment)
.

@lkilcher
Copy link
Contributor Author

Thanks, I'll do that. Also, note that I've made a few additional minor changes with this last commit. I think they're pretty logical, but if anyone objects I can remove them.

Added Circle to annotations_guide.rst. Moved DArrow up so that entries
are sorted. Made fancybox_demo2.py outputs sorted box demo (so that
ordering matches annotations_guide). Increased spacing between boxes in
fancybox_demo2 to prevent overlap of images.
@lkilcher
Copy link
Contributor Author

I increased the spacing between bbox's in fancybox_demo2.py so that the boxes don't overlap. This is what that demo produces now.
fancybox_demo2

@lkilcher
Copy link
Contributor Author

OK, unless anyone has additional requests I think this is ready to go. Thanks again for taking the time to teach me how to contribute to a real project. Hopefully I'll be able to contribute more substantially in the future.

@WeatherGod
Copy link
Member

Everything looks good to me. @tacaswell , did you want to wait a bit for the 1.4.0 dust to clear or just go ahead?

@tacaswell
Copy link
Member

We have created a branch, we can start doing what ever we want on master again. I'm going to go ahead and merge this.

tacaswell added a commit that referenced this pull request Jul 24, 2014
@tacaswell tacaswell merged commit 9e9a3cd into matplotlib:master Jul 24, 2014
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

6 participants