Skip to content

Fix polar facecolor #6001

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

Merged
merged 6 commits into from
Feb 15, 2016
Merged

Fix polar facecolor #6001

merged 6 commits into from
Feb 15, 2016

Conversation

tacaswell
Copy link
Member

Closes #5996

 - change name of first kwarg (third arg) of `_AxesBase.__init__`
   from 'axisbg' -> 'facecolor'.
 - added 'axisbg' back at the end of the listed kwargs
 - add handling from axisbg / facecolor conflicts

This adds a minor, but nasty, API wart in that there are now two
positional arguements to set the background color of the axes and API
wise we are locked into one positional arg.  However, if any non-None
value is passed for `axisbg` a warning will be raised and this is the
`__init__` on a private base class so should have reatively little
user exposure.
_axisbg -> _facecolor

Finishing the change from axisbg -> facecolor
@tacaswell tacaswell added this to the 2.0 (style change major release) milestone Feb 14, 2016
else:
if axisbg is not None and facecolor is not None:
raise TypeError('Both axisbg and facecolor are not None. '
'These keywords are aliases, only one maybe '
Copy link
Member

Choose a reason for hiding this comment

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

maybe -> may be

@@ -2722,7 +2727,7 @@ def set_axis_bgcolor(self, color):
warnings.warn(
Copy link
Member

Choose a reason for hiding this comment

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

Is this warning still needed, given the decorator?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you are correct.

@tacaswell
Copy link
Member Author

Also noticed that we do not even really need that private variable, so fully removed it.

@mdboom
Copy link
Member

mdboom commented Feb 15, 2016

👍 I think this may already be on master, though: 5f1127c

@tacaswell
Copy link
Member Author

well, drat.

@tacaswell
Copy link
Member Author

This PR looks like it touches a bit more than #5501 does. This adds face_color as an explicit kwarg that the _BaseAxes.__init__ pulls off rather than letting it fall all the way through to the update method.

I need to rip out half of the last commit.

 - the decorator takes care of the warning
 - fall back got get/set_facecolor instead of private internal state
@tacaswell
Copy link
Member Author

There is a sutble API difference between set_axis_bgcolor and set_facecolor, the later (used to) set the internal self._facecolor (self._axisbg) attribute which is what cla() uses to restore the color.

When setting the facecolor stash the value so that future calls to `cla`
will restore the facecolor to this value.
mdboom added a commit that referenced this pull request Feb 15, 2016
@mdboom mdboom merged commit 47b70ea into matplotlib:v2.x Feb 15, 2016
@tacaswell tacaswell deleted the fix_polar_facecolor branch May 12, 2016 21:41
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.

3 participants