Skip to content

ENH: fig.set_size to allow non-inches units #12415

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
23 changes: 23 additions & 0 deletions lib/matplotlib/cbook/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2075,3 +2075,26 @@ def _check_and_log_subprocess(command, logger, **kwargs):
.format(command, exc.output.decode('utf-8')))
logger.debug(report)
return report


def _print_unit(st, dpi=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

function name seems a bit weird?

"""
Convert from a string with a number and units into inches
"""
print_units = {'cm': 2.54, 'pt': 72.0, 'mm': 25.4, 'in': 1.0}
if len(st) > 2:
Copy link
Contributor

@anntzer anntzer Oct 7, 2018

Choose a reason for hiding this comment

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

is that a leftover of supporting "2cm"?
edit: I see that's not removed yet? (not a big deal, just checking)

Copy link
Member Author

Choose a reason for hiding this comment

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

Not leftover yet. I still think it’s an ok API to have both

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem, we can discuss this over the call.

num = float(st[:-2])
else:
num = 1
unit = st[-2:]
# special case "px"
if unit == 'px':
if dpi is not None:
return num / dpi
else:
raise ValueError('px units need dpi to be set')
elif unit in print_units:
return num / print_units[unit]
else:
# let the parent handle the errors
return st
98 changes: 87 additions & 11 deletions lib/matplotlib/figure.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,14 +344,17 @@ def __init__(self,
if frameon is None:
frameon = rcParams['figure.frameon']

if not np.isfinite(figsize).all():
raise ValueError('figure size must be finite not '
'{}'.format(figsize))
self.bbox_inches = Bbox.from_bounds(0, 0, *figsize)

self.canvas = None
self._suptitle = None
self.dpi_scale_trans = Affine2D().scale(dpi, dpi)
# do not use property as it will trigger
self._dpi = dpi

# init the bbox to dummy values:
self.bbox_inches = Bbox.from_bounds(0, 0, 1, 1)
# set properly.
self.set_size(figsize)

self.bbox = TransformedBbox(self.bbox_inches, self.dpi_scale_trans)

self.frameon = frameon
Expand All @@ -364,9 +367,6 @@ def __init__(self,
self._set_artist_props(self.patch)
self.patch.set_antialiased(False)

self.canvas = None
self._suptitle = None

if subplotpars is None:
subplotpars = SubplotParams()

Expand All @@ -391,6 +391,14 @@ def __init__(self,
# list of child gridspecs for this figure
self._gridspecs = []

def _parse_figsize(self, figsize):
if len(figsize) == 3:
unit = figsize[-1]
figsize = figsize[:2]
conv = cbook._print_unit(unit, dpi=self._dpi)
figsize = [f * conv for f in figsize]
return figsize

# TODO: I'd like to dynamically add the _repr_html_ method
# to the figure in the right context, but then IPython doesn't
# use it, for some reason.
Expand Down Expand Up @@ -862,19 +870,85 @@ def figimage(self, X, xo=0, yo=0, alpha=None, norm=None, cmap=None,
self.stale = True
return im

def set_size(self, w, h=None, units=None, forward=True):
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we're introducing a new API, let's decide whether we want set_size((w, h)) or set_size(w, h) but don't support both. (I have a mild preference for set_size((w, h)), as that fits the setter model better (figure.set(size=(w, h)))).

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 don’t think it necessarily is a good idea to make it inconsistent with the existing API. Why do we have to enforce a tuple?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not going to fight too much over this specific case as we're just following a preexisting API, but in general I think we should aim for functions that have a single well-define signature (either taking two floats as separate arguments, or a single pair); otherwise that's how we end up with the mess that's custom arg parsing for plot(), quiver() and friends...

Copy link
Member

@timhoffm timhoffm Oct 8, 2018

Choose a reason for hiding this comment

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

I agree that this should be decided. Unfortunately there are good reasons for both.

set_size((w, h)) is more difficult to type and read. It feels a bit artificial, also given that ax.set_xlim(left, right) has a similar signature and I wouldn't vote for changing that.

OTOH, set_size(size) would allow to trivially support size=(6, 4, "cm") throughout the whole library and in all figure-creating functions. If we just have set_size(w, h, unit) we might need some explicit conversion of the tuple.

All over, I have a mild preference for set_size(w, h).

Speaking of which, I'm also fine with the "both" solution set_size(self, w, h=None), basically because we already have precedence. The documentation is more cumbersome, but all use-cases can be covered. So to me, it would be ok to use this and defer a general decision.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, set_xlim((left, right)) also works...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah the boo(x, y) = boo((x, y)) API is quite prevalent. I agree it can make parsing the args a bit of a nuisance, though to me the basic problem w/ plot is the fact we allow, plot(x1, y1, x2, y2).

Copy link
Member

Choose a reason for hiding this comment

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

If it was just that. We also have plot(x1, y1, fmt1, x2, y2, fmt2) and even plot(y1, fmt1, y2, fmt2). Welcome to the Matlab way of thinking. But we are getting off-topic.

"""
Set the figure size

Parameters
----------

w: float, string, or two-tuple of either.

If a two-tuple, (width, height) of the figure.

If a float, width of figure in inches.

If a string, its a float followed by a 2-character suffix
of either 'in', 'cm', 'mm', 'pt' (inches, centimeters,
millimeters, or points). i.e. ``w='4.6cm'``.

h: float or string
As above, if a float it is the hieght of the figure in inches,
or a string decoded as above.

units : string
If specified, will be applied to floats in (w, h). If
either (w, h) are strings, the units they specify are used
for that dimension. Allows ``fig.set_size(12, 15, units='cm')``

forward : bool, default True
Forward to the window so the canvas size is updated;
e.g., you can resize the figure window from the shell

See Also
--------
matplotlib.Figure.set_size_inches

"""
if h is None:
figsize = self._parse_figsize(w)
w, h = figsize

conv = 1 # default units are inches.
if isinstance(units, str):
conv = cbook._print_unit(units, dpi=self._dpi)
if isinstance(conv, str):
raise ValueError('Could not convert units str {} to '
'inches'.format(conv))

if isinstance(w, str):
w = cbook._print_unit(w, dpi=self._dpi)
if isinstance(w, str):
raise ValueError('Could not convert str {} to '
'inches'.format(w))
else:
# convert using units arg
w = w * conv

if isinstance(h, str):
h = cbook._print_unit(h, dpi=self._dpi)
if isinstance(h, str):
raise ValueError('Could not convert str {} to '
'inches'.format(h))
else:
# convert using units arg
h = h * conv
# all done: set...
self.set_size_inches(w, h, forward=forward)

def set_size_inches(self, w, h=None, forward=True):
"""Set the figure size in inches.
"""Set the figure size.

Call signatures::

fig.set_size_inches(w, h) # OR
fig.set_size_inches((w, h))

optional kwarg *forward=True* will cause the canvas size to be
Optional kwarg *forward=True* will cause the canvas size to be
automatically updated; e.g., you can resize the figure window
from the shell

ACCEPTS: a (w, h) tuple with w, h in inches
ACCEPTS: a (w, h) tuple with w, h in inches.

See Also
--------
Expand All @@ -883,8 +957,10 @@ def set_size_inches(self, w, h=None, forward=True):

# the width and height have been passed in as a tuple to the first
# argument, so unpack them

if h is None:
w, h = w

if not all(np.isfinite(_) for _ in (w, h)):
raise ValueError('figure size must be finite not '
'({}, {})'.format(w, h))
Expand Down