Skip to content

Deprecate "hold" kwarg and machinery. #7516

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 7 commits into from
Nov 27, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions boilerplate.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,19 @@
@_autogen_docstring(Axes.%(func)s)
def %(func)s(%(argspec)s):
%(ax)s = gca()
# allow callers to override the hold state by passing hold=True|False
%(washold)s = %(ax)s.ishold()
# Deprecated: allow callers to override the hold state
# by passing hold=True|False
%(washold)s = %(ax)s._hold
%(sethold)s
if hold is not None:
%(ax)s.hold(hold)
%(ax)s._hold = hold
from matplotlib.cbook import mplDeprecation
warnings.warn("The 'hold' keyword argument is deprecated since 2.0.",
mplDeprecation)
Copy link
Member

Choose a reason for hiding this comment

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

Should we use stacklevel=2, so that the warning references the caller instead of pyplot.py?

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 tried that just now, but it gives puzzling result when using pyplot commands from the command line, or from a function defined on the command line. I think we are better off leaving the default in place.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, really?

>>> import numpy as np
>>> import matplotlib.pyplot as plt
>>> plt.plot(np.arange(10), hold=False)
/home/elliott/code/matplotlib/lib/matplotlib/pyplot.py:3315: MatplotlibDeprecationWarning: The 'hold' keyword argument is deprecated since 2.0.
  mplDeprecation)
[<matplotlib.lines.Line2D object at 0x7f3722d1f320>]

vs.

>>> import numpy as np
>>> import matplotlib.pyplot as plt
>>> plt.plot(np.arange(10), hold=False)
__main__:1: MatplotlibDeprecationWarning: The 'hold' keyword argument is deprecated since 2.0.
[<matplotlib.lines.Line2D object at 0x7f83598b5c50>]

looks correct to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is with ipython and stacklevel=2:

In [1]: plot([1,2,3], hold=True)
/Users/efiring/anaconda/envs/test/bin/ipython:1: MatplotlibDeprecationWarning: The 'hold' keyword argument is deprecated since 2.0.
  #!/bin/bash /Users/efiring/anaconda/envs/test/bin/python.app
[<matplotlib.lines.Line2D at 0x1151f1f98>]

In [2]: def make_warning():
   ...:     plot([1,2,3], hold=True)
   ...:

In [3]: make_warning()
/Users/efiring/anaconda/envs/test/bin/ipython:2: MatplotlibDeprecationWarning: The 'hold' keyword argument is deprecated since 2.0.
  if __name__ == '__main__':

and with the default:

In [1]: plot([1,2,3], hold=True)
/Users/efiring/work/programs/py/mpl/matplotlib/lib/matplotlib/pyplot.py:3315: MatplotlibDeprecationWarning: The 'hold' keyword argument is deprecated since 2.0.
  mplDeprecation)
[<matplotlib.lines.Line2D at 0x117dce0b8>]

In [2]: def make_warning():
    plot([1,2,3], hold=True)
   ...:

In [3]: make_warning()
/Users/efiring/work/programs/py/mpl/matplotlib/lib/matplotlib/pyplot.py:3315: MatplotlibDeprecationWarning: The 'hold' keyword argument is deprecated since 2.0.
  mplDeprecation)

For this use, the default looks better to me. What would really be needed for someone to debug a hold kwarg buried in a library is a full stack trace, but we are not going to put that in.

try:
%(ret)s = %(ax)s.%(func)s(%(call)s)
finally:
%(ax)s.hold(%(washold)s)
%(ax)s._hold = %(washold)s
%(mappable)s
return %(ret)s
"""
Expand Down
11 changes: 10 additions & 1 deletion doc/api/api_changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ Color of Axes
The ``axisbg`` and ``axis_bgcolor`` properties on ``Axes`` have been
deprecated in favor of ``facecolor``.


GTK and GDK backends deprecated
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The GDK and GTK backends have been deprecated. These obsolete backends
Expand All @@ -41,6 +40,16 @@ CocoaAgg backend removed
~~~~~~~~~~~~~~~~~~~~~~~~
The deprecated and not fully functional CocoaAgg backend has been removed.

'hold' functionality deprecated
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The 'hold' keyword argument and all functions and methods related
to it are deprecated, along with the 'axes.hold' `rcParams` entry.
The behavior will remain consistent with the default ``hold=True``
state that has long been in place. Instead of using a function
or keyword argument (``hold=False``) to change that behavior,
explicitly clear the axes or figure as needed prior to subsequent
Copy link
Member

Choose a reason for hiding this comment

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

Maybe link these two things?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I don't understand what you are suggesting.

Copy link
Member

Choose a reason for hiding this comment

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

A cross-reference to the relevant functions for clearing the Axes/Figure.

plotting commands.


`Artist.update` has return value
--------------------------------
Expand Down
11 changes: 10 additions & 1 deletion lib/matplotlib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -870,8 +870,13 @@ def matplotlib_fname():
}

_obsolete_set = set(['tk.pythoninspect', 'legend.isaxes'])

# The following may use a value of None to suppress the warning.
_deprecated_set = set(['axes.hold']) # do NOT include in _all_deprecated

_all_deprecated = set(chain(_deprecated_ignore_map,
_deprecated_map, _obsolete_set))
_deprecated_map,
_obsolete_set))


class RcParams(dict):
Expand All @@ -887,6 +892,8 @@ class RcParams(dict):
six.iteritems(defaultParams)
if key not in _all_deprecated)
msg_depr = "%s is deprecated and replaced with %s; please use the latter."
msg_depr_set = ("%s is deprecated. Please remove it from your "
"matplotlibrc and/or style files.")
msg_depr_ignore = "%s is deprecated and ignored. Use %s"
msg_obsolete = ("%s is obsolete. Please remove it from your matplotlibrc "
"and/or style files.")
Expand All @@ -903,6 +910,8 @@ def __setitem__(self, key, val):
warnings.warn(self.msg_depr % (key, alt_key))
key = alt_key
val = alt_val(val)
elif key in _deprecated_set and val is not None:
warnings.warn(self.msg_depr_set % key)
elif key in _deprecated_ignore_map:
alt = _deprecated_ignore_map[key]
warnings.warn(self.msg_depr_ignore % (key, alt))
Expand Down
18 changes: 11 additions & 7 deletions lib/matplotlib/axes/_axes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1665,7 +1665,7 @@ def acorr(self, x, **kwargs):

x : sequence of scalar

hold : boolean, optional, default: True
hold : boolean, optional, *deprecated*, default: True

detrend : callable, optional, default: `mlab.detrend_none`
x is detrended by the `detrend` callable. Default is no
Expand Down Expand Up @@ -1713,6 +1713,8 @@ def acorr(self, x, **kwargs):
.. plot:: mpl_examples/pylab_examples/xcorr_demo.py

"""
if "hold" in kwargs:
warnings.warn("the 'hold' kwarg is deprecated", mplDeprecation)
return self.xcorr(x, x, **kwargs)

@unpack_labeled_data(replace_names=["x", "y"], label_namer="y")
Expand All @@ -1730,7 +1732,7 @@ def xcorr(self, x, y, normed=True, detrend=mlab.detrend_none,

y : sequence of scalars of length n

hold : boolean, optional, default: True
hold : boolean, optional, *deprecated*, default: True
Copy link
Member

Choose a reason for hiding this comment

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

The "correct" way to document deprecation with numpydoc is through the deprecation notes section, but I don't think this matters much.

Copy link
Member Author

Choose a reason for hiding this comment

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

This puts it out front, so I will consider it at least good enough for now.


detrend : callable, optional, default: `mlab.detrend_none`
x is detrended by the `detrend` callable. Default is no
Expand Down Expand Up @@ -1769,6 +1771,8 @@ def xcorr(self, x, y, normed=True, detrend=mlab.detrend_none,
The cross correlation is performed with :func:`numpy.correlate` with
`mode` = 2.
"""
if "hold" in kwargs:
warnings.warn("the 'hold' kwarg is deprecated", mplDeprecation)

Nx = len(x)
if Nx != len(y):
Expand Down Expand Up @@ -2136,7 +2140,7 @@ def make_iterable(x):
patches.append(r)

holdstate = self._hold
self.hold(True) # ensure hold is on before plotting errorbars
self._hold = True # ensure hold is on before plotting errorbars

if xerr is not None or yerr is not None:
if orientation == 'vertical':
Expand All @@ -2158,7 +2162,7 @@ def make_iterable(x):
else:
errorbar = None

self.hold(holdstate) # restore previous hold state
self._hold = holdstate # restore previous hold state

if adjust_xlim:
xmin, xmax = self.dataLim.intervalx
Expand Down Expand Up @@ -2380,7 +2384,7 @@ def stem(self, *args, **kwargs):
remember_hold = self._hold
if not self._hold:
self.cla()
self.hold(True)
self._hold = True

# Assume there's at least one data array
y = np.asarray(args[0])
Expand Down Expand Up @@ -2464,7 +2468,7 @@ def stem(self, *args, **kwargs):
color=basecolor, linestyle=basestyle,
marker=basemarker, label="_nolegend_")

self.hold(remember_hold)
self._hold = remember_hold

stem_container = StemContainer((markerline, stemlines, baseline),
label=label)
Expand Down Expand Up @@ -3786,7 +3790,7 @@ def dopatch(xs, ys, **kwargs):
setlabels(datalabels)

# reset hold status
self.hold(holdStatus)
self._hold = holdStatus

return dict(whiskers=whiskers, caps=caps, boxes=boxes,
medians=medians, fliers=fliers, means=means)
Expand Down
16 changes: 15 additions & 1 deletion lib/matplotlib/axes/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,9 @@ def __init__(self, fig, rect,
self._rasterization_zorder = None

self._hold = rcParams['axes.hold']
if self._hold is None:
self._hold = True

self._connected = {} # a dict from events to (id, func)
self.cla()
# funcs used to format x and y - fall back on major formatters
Expand Down Expand Up @@ -1191,14 +1194,25 @@ def set_color_cycle(self, clist):
else:
self.set_prop_cycle('color', clist)

@cbook.deprecated("2.0")
def ishold(self):
"""return the HOLD status of the axes"""
"""return the HOLD status of the axes

The `hold` mechanism is deprecated and will be removed in
v3.0.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we not commit to a specific version here? Or at least make it one of 2.2, 2.3 or 2.4, which should be far away enough... Same comments below.

(mpl 3.0 sounds a bit like python 4 to me)

Copy link
Member

Choose a reason for hiding this comment

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

That is the correct version if we wish to follow SemVer.

Copy link
Contributor

Choose a reason for hiding this comment

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

... which we haven't been doing so far?

Copy link
Member

Choose a reason for hiding this comment

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

It was not fully concluded in #3070.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would just say "in a future feature release" (as opposed to "bugfix release") in that case.

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 thought this would generate some controversy... I would like to see "hold" go away ASAP, and I don't have strong opinions about version numbering. It seemed like the discussion in #3070 was leaning in the direction of going wild with major version numbers--why not, Firefox is up to 50.0! Seriously, regardless of how the numbering controversy is resolved, we need to find a way to get releases out more frequently.

Copy link
Member

Choose a reason for hiding this comment

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

We will at least have a 3.0 when we drop python2 support (so 2018).

Copy link
Contributor

Choose a reason for hiding this comment

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

We should be following SemVer, and we certainly are with 2.0. Therefore, when we remove hold it will be ++major_version. Is there any reason this wouldn't make 3.0?

"""

return self._hold

@cbook.deprecated("2.0")
def hold(self, b=None):
"""
Set the hold state

The ``hold`` mechanism is deprecated and will be removed in
v3.0. The behavior will remain consistent with the
long-time default value of True.

If *hold* is *None* (default), toggle the *hold* state. Else
set the *hold* state to boolean value *b*.

Expand Down
12 changes: 6 additions & 6 deletions lib/matplotlib/colorbar.py
Original file line number Diff line number Diff line change
Expand Up @@ -501,10 +501,10 @@ def _add_solids(self, X, Y, C):
# Save, set, and restore hold state to keep pcolor from
# clearing the axes. Ordinarily this will not be needed,
# since the axes object should already have hold set.
_hold = self.ax.ishold()
self.ax.hold(True)
_hold = self.ax._hold
self.ax._hold = True
col = self.ax.pcolormesh(*args, **kw)
self.ax.hold(_hold)
self.ax._hold = _hold
#self.add_observer(col) # We should observe, not be observed...

if self.solids is not None:
Expand Down Expand Up @@ -1262,8 +1262,8 @@ def _add_solids(self, X, Y, C):
# Save, set, and restore hold state to keep pcolor from
# clearing the axes. Ordinarily this will not be needed,
# since the axes object should already have hold set.
_hold = self.ax.ishold()
self.ax.hold(True)
_hold = self.ax._hold
self.ax._hold = True

kw = {'alpha': self.alpha, }

Expand Down Expand Up @@ -1309,7 +1309,7 @@ def _add_solids(self, X, Y, C):
linewidths=(0.5 * mpl.rcParams['axes.linewidth'],))
self.ax.add_collection(self.dividers)

self.ax.hold(_hold)
self.ax._hold = _hold


def colorbar_factory(cax, mappable, **kwargs):
Expand Down
8 changes: 7 additions & 1 deletion lib/matplotlib/figure.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,9 @@ def __init__(self,
self.patch.set_aa(False)

self._hold = rcParams['axes.hold']
if self._hold is None:
self._hold = True

self.canvas = None
self._suptitle = None

Expand Down Expand Up @@ -569,6 +572,7 @@ def set_canvas(self, canvas):
"""
self.canvas = canvas

@cbook.deprecated("2.0")
def hold(self, b=None):
"""
Set the hold state. If hold is None (default), toggle the
Expand All @@ -579,6 +583,8 @@ def hold(self, b=None):
hold() # toggle hold
hold(True) # hold is on
hold(False) # hold is off

All "hold" machinery is deprecated.
"""
if b is None:
self._hold = not self._hold
Expand Down Expand Up @@ -1591,7 +1597,7 @@ def colorbar(self, mappable, cax=None, ax=None, use_gridspec=True, **kw):
cax, kw = cbar.make_axes_gridspec(ax, **kw)
else:
cax, kw = cbar.make_axes(ax, **kw)
cax.hold(True)
cax._hold = True
cb = cbar.colorbar_factory(cax, mappable, **kw)

self.sca(current_ax)
Expand Down
1 change: 0 additions & 1 deletion lib/matplotlib/mpl-data/stylelib/classic.mplstyle
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ mathtext.default : it # The default font to use for math.
# default face and edge color, default tick sizes,
# default fontsizes for ticklabels, and so on. See
# http://matplotlib.org/api/axes_api.html#module-matplotlib.axes
axes.hold : True # whether to clear the axes by default on
axes.facecolor : w # axes background color
axes.edgecolor : k # axes edge color
axes.linewidth : 1.0 # edge linewidth
Expand Down
2 changes: 0 additions & 2 deletions lib/matplotlib/pylab.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,12 @@
getp - get a graphics property
grid - set whether gridding is on
hist - make a histogram
hold - set the axes hold state
ioff - turn interaction mode off
ion - turn interaction mode on
isinteractive - return True if interaction mode is on
imread - load image file into array
imsave - save array as an image file
imshow - plot image data
ishold - return the hold state of the current axes
legend - make an axes legend
locator_params - adjust parameters used in locating axis ticks
loglog - a log log plot
Expand Down
Loading