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.

Sign up for GitHub

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

Jump to bottom

BUG/API : tweaked how AnchoredSizeBar handles font properties #2951

Merged
merged 2 commits into from Jul 6, 2014
Merged

BUG/API : tweaked how AnchoredSizeBar handles font properties #2951

merged 2 commits into from Jul 6, 2014

Conversation

tacaswell
Copy link
Member

  • removed the explicit prop from
    mpl_toolkits.axes_grid1.anchored_artists.AnchoredSizeBar
  • pass fontproperties through to the base class in place of prop
  • if prop is not None and fontproperties is None, then use
    prop in place of fontproperties.

close #2394

@tacaswell tacaswell mentioned this pull request Apr 3, 2014
Closed
@tacaswell tacaswell added this to the v1.4.0 milestone Apr 3, 2014
@Tillsten
Copy link
Contributor

Tillsten commented Apr 5, 2014

Seems fine, maybe also deprecate props?

@tacaswell
Copy link
Member Author

The problem with deprcating props is that props is a kwarg of the parent class and there in kwarg pass-through so that would involve touching a bunch of other code.

Thinking about this again, it may be that fontproperties should have never been added and props used instead....

@tacaswell
Copy link
Member Author

@efiring I would like some feed back on this one.

@efiring
Copy link
Member

efiring commented Apr 27, 2014

@tacaswell, I have to punt this over to @leejjoon; axes_grid is entirely his, I have no familiarity with it.

@tacaswell
Copy link
Member Author

@leejjoon Can you review this?

@tacaswell
BUG/API : tweaked how AnchoredSizeBar handles font properties
57ea00c 
 - removed the explicit `prop` from
    `mpl_toolkits.axes_grid1.anchored_artists.AnchoredSizeBar`
 - pass `fontproperties` through to the base class in place of `prop`
 - if `prop` is not None and `fontproperties` is None, then use
   `prop` in place of `fontproperties`.
pelson
pelson reviewed Jun 18, 2014
View reviewed changes
doc/api/api_changes.rst
@@ -179,6 +185,8 @@ Code removal
a long time) and was not the standard form of the Levy distribution.
``scipy.stats.levy`` should be used instead


>>>>>>> BUG/API : tweaked how AnchoredSizeBar handles font properties
Copy link
Member

Choose a reason for hiding this comment

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

Woops 😉

@pelson
Copy link
Member

pelson commented Jun 18, 2014

I'd merge this - I'll give @leejjoon a few more days to comment, but otherwise 👍

@leejjoon
Copy link
Contributor

This looks good. Please go ahead and merge it.
I apologize for my long silence.

@pelson
Copy link
Member

pelson commented Jun 18, 2014

Thanks @leejjoon - and no worries - we're all pretty busy and some long silences are inevitable at points.

@tacaswell - I'll merge one the conflict header has been removed form the whats new.

@tacaswell
Copy link
Member Author

@pelson Done.

WeatherGod added a commit that referenced this pull request Jul 6, 2014
@WeatherGod
Merge pull request #2951 from tacaswell/anchoredsizebox_text
e0be268 
BUG/API : tweaked how AnchoredSizeBar handles font properties
@WeatherGod WeatherGod merged commit e0be268 into matplotlib:master Jul 6, 2014
@tacaswell tacaswell deleted the anchoredsizebox_text branch July 6, 2014 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.4.0
Development

Successfully merging this pull request may close these issues.

AnchoredSizeBar does not respect FontProperties size setting.
7 participants
@tacaswell @Tillsten @efiring @pelson @leejjoon @WeatherGod @QuLogic