Skip to content

Fix typo in Axes3D.set_autoscalez_on. #7859

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

Merged
merged 3 commits into from
Jan 28, 2017
Merged

Fix typo in Axes3D.set_autoscalez_on. #7859

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bcb7976
Fix typo in Axes3D.set_autoscalez_on.
anntzer Jan 17, 2017
5347d49
Whitespace cleanup of axes3d.
anntzer Jan 18, 2017
effe642
Fix passing kwargs only to margins; add test.
anntzer Jan 18, 2017
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
Prev Previous commit
Fix passing kwargs only to margins; add test.
  • Loading branch information
@anntzer
anntzer committed Jan 27, 2017
commit effe64264f5361bf8ea8adeee4be900e469df008
30 changes: 18 additions & 12 deletions lib/mpl_toolkits/mplot3d/axes3d.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,15 +405,21 @@ def margins(self, *args, **kw):
mx = kw.pop('x', None)
my = kw.pop('y', None)
mz = kw.pop('z', None)
if len(args) == 1:
if not args:
pass
elif len(args) == 1:
mx = my = mz = args[0]
elif len(args) == 2:
# Maybe put out a warning because mz is not set?
warnings.warn(
"Passing exactly two positional arguments to Axes3D.margins "
"is deprecated. If needed, pass them as keyword arguments "
"instead", cbook.mplDeprecation)
mx, my = args
elif len(args) == 3:
mx, my, mz = args
else:
raise ValueError("more than three arguments were supplied")
raise ValueError(
"Axes3D.margins takes at most three positional arguments")
Copy link
Member

Choose a reason for hiding this comment

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

I raised a ValueError here originally because that what the 2d Axes.margins() does when given more than two arguments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

May or may not be a gratuitious backwards compatible change but arguments mismatching the signature normally raise a TypeError. Should we change it in Axes.margins too? (hem hem)

Copy link
Member

Choose a reason for hiding this comment

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

along the same line of thinking, wouldn't the 2d version of this function suffer the same problem that you are trying to fix here with an empty args list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. Haven't checked the impl. but it must have been fixed at some point.

Copy link
Member

Choose a reason for hiding this comment

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

just took a look, it seems like the 2d version doesn't have this else-clause, so it silently allows more arguments than expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open a separate issue? :)

Copy link
Member

Choose a reason for hiding this comment

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

grrr, misread the code... it doesn't have an else clause... it has a "len(args) > 2" clause that then raises the ValueError, but also skips the case of no positional args. We are fine, then.

As for ValueError vs. TypeError, I really don't have a preference except that the 2d and 3d version be the same.

if mx is not None:
self.set_xmargin(mx)
if my is not None:
Expand Down Expand Up @@ -444,18 +450,18 @@ def autoscale(self, enable=True, axis='both', tight=None):
scaley = True
scalez = True
else:
scalex = False
scaley = False
scalez = False
if axis in ['x', 'both']:
self._autoscaleXon = bool(enable)
scalex = self._autoscaleXon
self._autoscaleXon = scalex = bool(enable)
else:
scalex = False
if axis in ['y', 'both']:
self._autoscaleYon = bool(enable)
scaley = self._autoscaleYon
self._autoscaleYon = scaley = bool(enable)
else:
scaley = False
if axis in ['z', 'both']:
self._autoscaleZon = bool(enable)
scalez = self._autoscaleZon
self._autoscaleZon = scalez = bool(enable)
else:
scalez = False
self.autoscale_view(tight=tight, scalex=scalex, scaley=scaley,
scalez=scalez)

Expand Down
12 changes: 12 additions & 0 deletions lib/mpl_toolkits/tests/test_mplot3d.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,3 +468,15 @@ def test_lines_dists():

ax.set_xlim(-50, 150)
ax.set_ylim(0, 300)


@cleanup
def test_autoscale():
fig, ax = plt.subplots(subplot_kw={"projection": "3d"})
ax.margins(x=0, y=.1, z=.2)
ax.plot([0, 1], [0, 1], [0, 1])
assert ax.get_w_lims() == (0, 1, -.1, 1.1, -.2, 1.2)
ax.autoscale(False)
ax.set_autoscalez_on(True)
ax.plot([0, 2], [0, 2], [0, 2])
assert ax.get_w_lims() == (0, 1, -.1, 1.1, -.4, 2.4)