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

remove some redunancy in _BaseAxes #2424

Closed
wants to merge 1 commit into from
Closed

remove some redunancy in _BaseAxes #2424

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 9 additions & 11 deletions lib/matplotlib/axes/_base.py
View file
Open in desktop
Expand Up @@ -329,9 +329,6 @@ def __init__(self, fig, rect,
frameon=True,
sharex=None, # use Axes instance's xaxis info
sharey=None, # use Axes instance's yaxis info
label='',
xscale=None,
yscale=None,
**kwargs
):
"""
Copy link
Member

Choose a reason for hiding this comment

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

I am ambivalent about removing these three. Technically speaking, it is an API change (although a very far removed edge case of someone using those three arguments as positional arguments). I don't really see any benefits in changing this part.

The main idea of the kwargs, IIRC, was to be a catch-all for artist properties, not for axes properties (although I think "label" is an artist property...). So folding xscale and yscale into kwargs seems "wrong" to me.

Expand Down Expand Up @@ -412,7 +409,8 @@ def __init__(self, fig, rect,
#warnings.warn(
# 'shared axes: "adjustable" is being changed to "datalim"')
self._adjustable = 'datalim'
self.set_label(label)

kwargs.setdefault('label','')
Copy link
Member

Choose a reason for hiding this comment

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

PEP8: space after comma

self.set_figure(fig)

self.set_axes_locator(kwargs.get("axes_locator", None))
Expand All @@ -422,14 +420,18 @@ def __init__(self, fig, rect,
# this call may differ for non-sep axes, eg polar
self._init_axis()

# These duplicate parameters should be removed.
## axisbg ~ axis_bgcolor (shouln't actually be axes?)
## frameon == frame_on
if axisbg is None:
axisbg = rcParams['axes.facecolor']
self._axisbg = axisbg
self._frameon = frameon
## push both to kwargs, which will be processed by update.
## _axisbg must be assignd before cla().
self._axisbg = kwargs.setdefault('axis_bgcolor', axisbg)
kwargs.setdefault('frame_on', frameon)
Copy link
Member

Choose a reason for hiding this comment

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

Don't we still need to assign to self._frameon?

Copy link
Author

Choose a reason for hiding this comment

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

I didn't find any direct users of it except for get_frame_on(),
and since it is "private" (starts with _) I thought it wouldn't matter

Copy link
Member

Choose a reason for hiding this comment

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

it is used in draw(), and it is used in axes3d

self._axisbelow = rcParams['axes.axisbelow']

self._rasterization_zorder = None

self._hold = rcParams['axes.hold']
self._connected = {} # a dict from events to (id, func)
self.cla()
Expand All @@ -443,10 +445,6 @@ def __init__(self, fig, rect,
self.set_navigate(True)
self.set_navigate_mode(None)

if xscale:
self.set_xscale(xscale)
if yscale:
self.set_yscale(yscale)

if len(kwargs):
self.update(kwargs)
Expand Down