8000 Set figure width and height with set_size_inches by jhamrick · Pull Request #4677 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Set figure width and height with set_size_inches #4677

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
Jul 15, 2015
Merged

Set figure width and height with set_size_inches #4677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee1a599
Have set_figheight and set_figwidth call set_size_inches
jhamrick Jul 13, 2015
ddcf618
Add a test for setting the figure width/height
jhamrick Jul 13, 2015
8524138
Remove *args and **kwargs to set_size_inches
jhamrick Jul 13, 2015
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
21 changes: 9 additions & 12 deletions lib/matplotlib/figure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ def figimage(self, X,
self.stale = True
return im

def set_size_inches(self, *args, **kwargs):
def set_size_inches(self, w, h=None, forward=False):
"""
set_size_inches(w,h, forward=False)

Expand All @@ -682,11 +682,10 @@ def set_size_inches(self, *args, **kwargs):
matplotlib.Figure.get_size_inches
"""

forward = kwargs.get('forward', False)
if len(args) == 1:
w, h = args[0]
else:
w, h = args
# the width and height have been passed in as a tuple to the first
Copy link
Member

Choose a reason for hiding this comment

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

if the width...

Do we also want to add a comment that we don't recommend this? I remember from reading comments in the backend that we considered this bad practice (so I guess we may remove this 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.

A comment here wouldn't help much. None of our code is using the tuple argument form. Ideally, set_width and set_height would not be needed at all; they could be handled in set_size_inches using keywords, with both w and h defaulted to None.
Does anything actually use set_width and/or set_height at all? I imagine that something must use them, or the issue of changing them to go through set_size_inches would not have arisen. Nevertheless, it seems like this is a place where we have the opportunity for some longer-term deprecation and code condensation to reign in our sprawling API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I just made it so it would be backwards compatible for how it works currently, but I agree it's kind of strange to support both the tuple and the arguments. I don't know if anything actually uses the tuple notation for this.

I personally use set_figwidth and set_figheight a lot, which is how this came up, but that's also partially because I didn't know about set_size_inches. I like the suggestion of having both width and height be keyword arguments in set_size_inches and allow setting either or both, and then deprecating set_figwidth and set_figheight. Is there a reason, though, for it being set_size_inches rather than just set_size?

Copy link
Member

Choose a reason for hiding this comment

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

set_xlim/set_ylim supports this notation, I think.

On Mon, Jul 13, 2015 at 3:55 PM, Jessica B. Hamrick <
notifications@github.com> wrote:

In lib/matplotlib/figure.py
#4677 (comment):

@@ -682,11 +682,10 @@ def set_size_inches(self, _args, *_kwargs):
matplotlib.Figure.get_size_inches
"""

  •    forward = kwargs.get('forward', False)

  •    if len(args) == 1:

  •        w, h = args[0]

  •    else:

  •        w, h = args

  •    # the width and height have been passed in as a tuple to the first

Yeah, I just made it so it would be backwards compatible for how it works
currently, but I agree it's kind of strange to support both the tuple and
the arguments. I don't know if anything actually uses the tuple notation
for this.

I personally use set_figwidth and set_figheight a lot, which is how this
came up, but that's also partially because I didn't know about
set_size_inches. I like the suggestion of having both width and height be
keyword arguments in set_size_inches and allow setting either or both,
and then deprecating set_figwidth and set_figheight. Is there a reason,
though, for it being set_size_inches rather than just set_size?

—
Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/4677/files#r34504229.

Copy link
Member

Choose a reason for hiding this comment

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

@efiring Yup I agree internal code so not much point here, but what about on the lines above in the function definition? See FigureManagerTkAgg.resize for the comment I referred to, should we do something similar here? I have also just done a search for sfw and sfh and only these methods come up.

@jhamrick I think set_size sounds confusing as it contains no units, it inches, centimetres, pixels, etcetera. I guess we do it by inches because of dpi (dots per square inch), a rather odd unit, but used everywhere in graphics, I have never seen dpcm :(. Perhaps you should also take a look at

Copy link
Member

Choose a reason for hiding this comment

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

True, and it has its good points; but I'm not sure it's worth the added API complexity to support this form everywhere. set_xlim and set_ylim are higher level than set_size_inches, so it is less disruptive to simplify the latter than the former.
As for set_size_inches versus set_size: I suspect that like much in mpl, it is an accident of the way it evolved. Whoever put it in decided to make it very clear what the units are.

# argument, so unpack them
if h is None:
w, h = w

dpival = self.dpi
self.bbox_inches.p1 = w, h
Expand Down Expand Up @@ -766,23 +765,21 @@ def set_dpi(self, val):
self.dpi = val
self.stale = True

def set_figwidth(self, val):
def set_figwidth(self, val, forward=False):
"""
Set the width of the figure in inches

ACCEPTS: float
"""
self.bbox_inches.x1 = val
self.stale = True
self.set_size_inches(val, self.get_figheight(), forward=forward)

def set_figheight(self, val):
def set_figheight(self, val, forward=False):
"""
Set the height of the figure in inches

ACCEPTS: float
"""
self.bbox_inches.y1 = val
self.stale = True
self.set_size_inches(self.get_figwidth(), val, forward=forward)

def set_frameon(self, b):
"""
Expand Down
23 changes: 23 additions & 0 deletions lib/matplotlib/tests/test_figure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,29 @@ def _as_mpl_axes(self):
plt.close(fig)


@cleanup
def test_set_fig_size():
Copy link
Member

Choose a reason for hiding this comment

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

hurray tests!

fig = plt.figure()

# check figwidth
fig.set_figwidth(5)
assert_equal(fig.get_figwidth(), 5)

# check figheight
fig.set_figheight(1)
assert_equal(fig.get_figheight(), 1)

# check using set_size_inches
fig.set_size_inches(2, 4)
assert_equal(fig.get_figwidth(), 2)
assert_equal(fig.get_figheight(), 4)

# check using tuple to first argument
fig.set_size_inches((1, 3))
assert_equal(fig.get_figwidth(), 1)
assert_equal(fig.get_figheight(), 3)


if __name__ == "__main__":
import nose
nose.runmodule(argv=['-s', '--with-doctest'], exit=False)
0