Skip to content

Turn autoscale into a contextmanager. #5538

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

Closed
wants to merge 4 commits into from
Closed
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
13 changes: 13 additions & 0 deletions doc/api/api_changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,19 @@ sources of the changes you are experiencing.
For new features that were added to matplotlib, please see
:ref:`whats-new`.

Changes in 2.1.0
================

Code changes
Copy link
Member

Choose a reason for hiding this comment

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

Can you put these is a file in api_changes/*.rst. This helps to prevent the sort of conflicts this branch has.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather close this PR and make margin a tristate as discussed above. In fact, making autoscale a contextmanager is a bit misleading given that at the exit of the contextmanager (assuming it temporarily disabled autoscaling), all the artists, including those created while the cm was active, will participate in the next autoscaling... which defeats the purpose of the cm IMO. Thoughts?

The fix for z autoscaling should go in its own PR.

Copy link
Member

Choose a reason for hiding this comment

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

@anntzer That sounds like a good plan to me.

Wish we were 3 only and could use enums 😞

Copy link
Contributor Author

@anntzer anntzer Sep 24, 2016

Choose a reason for hiding this comment

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

In this case I think the best is to use strings everywhere (IMO the problem of using, say, True, False and a third value is that it is too easy to accidentally test for the boolean value and forget about the third possibility). Say "default" (current True), "remove" (current False) and "ignore"?

attn @mdboom as the author of #5583.

------------

autoscale is now a context manager
``````````````````````````````````

Instead of returning None, ``plt.autoscale`` and ``Axes.autoscale``
now return a context manager, which allows one to restore the previous
autoscaling status upon exiting the block.

Changes in 2.0.0
================

Expand Down
12 changes: 12 additions & 0 deletions doc/users/whats_new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ revision, see the :ref:`github-stats`.
.. contents:: Table of Contents
:depth: 3

.. _whats-new-2-1:

new in matplotlib-2.1
=====================

autoscale is now a context manager
----------------------------------

Instead of returning None, ``plt.autoscale`` and ``Axes.autoscale``
now return a context manager, which allows one to restore the previous
autoscaling status upon exiting the block.

.. _whats-new-1-5:

new in matplotlib-1.5
Expand Down
30 changes: 20 additions & 10 deletions lib/matplotlib/axes/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from matplotlib.externals import six
from matplotlib.externals.six.moves import xrange

from contextlib import contextmanager
import itertools
import warnings
import math
Expand Down Expand Up @@ -2103,22 +2104,31 @@ def autoscale(self, enable=True, axis='both', tight=None):
The *tight* setting is retained for future autoscaling
until it is explicitly changed.

This returns a context-manager/decorator that allows temporarily
setting the autoscale status, i.e.::

Returns None.
with axes.autoscale(enable): ...

will keep the autoscale status to `enable` for the duration of the
block only.
"""
if enable is None:
scalex = True
scaley = True
else:
scalex = False
scaley = False
orig_autoscale = self._autoscaleXon, self._autoscaleYon
if enable is not None:
if axis in ['x', 'both']:
self._autoscaleXon = bool(enable)
scalex = self._autoscaleXon
if axis in ['y', 'both']:
self._autoscaleYon = bool(enable)
scaley = self._autoscaleYon
self.autoscale_view(tight=tight, scalex=scalex, scaley=scaley)
self.autoscale_view(
Copy link
Member

Choose a reason for hiding this comment

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

This does not behave the same if enable is None. Previously, ax.autoscale(None) would autoscale both axis, but leave the _autoscale*on state unchanged, now it scale the axis dependent on the current values of _autoscale*on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, so I misunderstood the code (I never used autoscale(None) so I had no idea what it did). I still don't understand the actual behavior: autoscaling (in autoscale_view) is protected by the conditional if scalex and self._autoscaleXon, so I thought that just passing scalex=True would not perform an autoscale as long as self._autoscaleXon is False.
Where did I go wrong?

Copy link
Member

Choose a reason for hiding this comment

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

I was just reading this code locally and sorting through what inputs are going to the autoscale_view, you are correct this does not actually change the behaviour. Sorry for the noise.

tight=tight, scalex=self._autoscaleXon, scaley=self._autoscaleYon)

@contextmanager
def restore_autoscaling():
try:
yield
finally:
self._autoscaleXon, self._autoscaleYon = orig_autoscale
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 mark the axes object as stale?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(I have no idea of what you're talking about so I'll leave the discussion to the specialists.)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this needs to set stale because changing this internal state does not invalidate the drawn figure (that is the next call to draw will not be different)....unless I am miss understanding how this works.


return restore_autoscaling()

def autoscale_view(self, tight=None, scalex=True, scaley=True):
"""
Expand Down
16 changes: 16 additions & 0 deletions lib/matplotlib/tests/test_axes.py
Original file line number Diff line number Diff line change
Expand Up @@ -4118,6 +4118,22 @@ def test_axisbg_warning():
("The axisbg attribute was deprecated in version 2.0.")))


@cleanup
def test_autoscale():
plt.plot([0, 1], [0, 1])
assert_equal(plt.xlim(), (0, 1))
plt.autoscale(False)
plt.plot([2, 3], [2, 3])
assert_equal(plt.xlim(), (0, 1))
plt.autoscale(None)
assert_equal(plt.xlim(), (0, 3))
with plt.autoscale(False):
plt.plot([3, 4], [3, 4])
assert_equal(plt.xlim(), (0, 3))
plt.plot([4, 5], [4, 5])
assert_equal(plt.xlim(), (0, 5))


if __name__ == '__main__':
import nose
import sys
Expand Down
33 changes: 19 additions & 14 deletions lib/mpl_toolkits/mplot3d/axes3d.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"""
from __future__ import (absolute_import, division, print_function,
unicode_literals)
from contextlib import contextmanager
import math

from matplotlib.externals import six
Expand Down Expand Up @@ -342,7 +343,7 @@ def set_autoscalez_on(self, b) :
.. versionadded :: 1.1.0
This function was added, but not tested. Please report any bugs.
"""
self._autoscalez_on = b
self._autoscaleZon = b

def set_zmargin(self, m) :
"""
Expand Down Expand Up @@ -438,25 +439,29 @@ def autoscale(self, enable=True, axis='both', tight=None) :
.. versionadded :: 1.1.0
This function was added, but not tested. Please report any bugs.
"""
if enable is None:
scalex = True
scaley = True
scalez = True
else:
scalex = False
scaley = False
scalez = False
orig_autoscale = (
self._autoscaleXon, self._autoscaleYon, self._autoscaleZon)
if enable is not None:
if axis in ['x', 'both']:
self._autoscaleXon = bool(enable)
scalex = self._autoscaleXon
if axis in ['y', 'both']:
self._autoscaleYon = bool(enable)
scaley = self._autoscaleYon
if axis in ['z', 'both']:
self._autoscaleZon = bool(enable)
scalez = self._autoscaleZon
self.autoscale_view(tight=tight, scalex=scalex, scaley=scaley,
scalez=scalez)
self.autoscale_view(tight=tight,
scalex=self._autoscaleXon,
scaley=self._autoscaleYon,
scalez=self._autoscaleZon)

@contextmanager
def restore_autoscaling():
try:
yield
finally:
self._autoscaleXon, self._autoscaleYon, self._autoscaleZon = (
orig_autoscale)

return restore_autoscaling()

def auto_scale_xyz(self, X, Y, Z=None, had_data=None):
x, y, z = list(map(np.asarray, (X, Y, Z)))
Expand Down