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

Improvements to anchored_artists.AnchoredSizeBar #2585

Closed
wants to merge 3 commits into from

Conversation

magnunor
Copy link
Contributor

@magnunor magnunor commented Nov 8, 2013

Two changes to the axes_grid1.anchored_artists.AnchoredSizeBar function:

  • Fix: color argument was not setting the color of the textlabel
  • Add: setting the font-size through the fontsize argument

Fix: color argument was not setting the color of the textlabel
Add: setting the font-size through the fontsize argument
@@ -71,7 +71,7 @@ class AnchoredSizeBar(AnchoredOffsetbox):
def __init__(self, transform, size, label, loc,
pad=0.1, borderpad=0.1, sep=2, prop=None,
frameon=True, size_vertical=0, color='black',
label_top=False,
label_top=False, fontsize=12,
Copy link
Member

Choose a reason for hiding this comment

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

Is there an existing rcparam this should respect?

Copy link
Member

Choose a reason for hiding this comment

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

Not that I'm aware of. It might be nice to accept a fontprops argument here, rather than just the size, giving the use more control of the font details.

@iMichka
Copy link
Contributor

iMichka commented Jan 4, 2014

Hi, I just tested this as I will need this for a projet.
The color changing for the text is working well with this PR.

A fontprop would be very nice, so we can use any font we want ;)

@pelson
Copy link
Member

pelson commented Jan 6, 2014

@magnunor - this PR is good. We could do with a couple of tests and updating the keyword to accept fontprop for more generic functionality, but other than that I'd be happy to merge. If you can get this done in the next couple of days we will be able to include this improvement in the next release v1.4.

Cheers!

Removed the old fontsize parameter.
And fix: some typos in the example in the docstring
@magnunor
Copy link
Contributor Author

magnunor commented Jan 6, 2014

I added a fontprops parameter and a test. I'm not sure if any of the old test files were an appropriate place for a test for AnchoredSizeBar, so I made a new file test_anchored_artists.py.

@@ -71,10 +71,10 @@ class AnchoredSizeBar(AnchoredOffsetbox):
def __init__(self, transform, size, label, loc,
pad=0.1, borderpad=0.1, sep=2, prop=None,
frameon=True, size_vertical=0, color='black',
label_top=False,
label_top=False, fontprops=None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps the argument should be fontproperties and not fontprops
That's what is used for the set_xylabels() or text()

@pelson pelson mentioned this pull request Jan 9, 2014
@pelson
Copy link
Member

pelson commented Jan 9, 2014

I'm really sorry @magnunor - I didn't notice that this was in axes_grid1. We currently do not test any part of that toolkit, and its probably best not to start doing that in this PR, I've opened #2712 in favour of this PR.

Thanks for this change!

@pelson pelson closed this Jan 9, 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

4 participants