Set figure width and height with set_size_inches #4677
Conversation
|
The use of I also wonder why the default of the "forward" kwarg is False; why would one want to use set_size_inches and not have the manager enlarge the window accordingly? Maybe it is just a question of when the action occurs. |
|
I am all for dropping kwargs when we can. My guess on that default would be back compatibility and/or not picking up That seems like a default we can change in 2.0. On Mon, Jul 13, 2015, 1:36 AM Eric Firing notifications@github.com wrote:
|
|
re: kwargs, remember that super-fied classes tends to prefer _args and On Mon, Jul 13, 2015 at 9:11 AM, Thomas A Caswell notifications@github.com
|
|
I think the way we should deal with The use case for the |
|
I also would think that having I'm happy to make the change to just having a single explicit argument to |
|
Ok, PR updated to remove |
| w, h = args[0] | ||
| else: | ||
| w, h = args | ||
| # the width and height have been passed in as a tuple to the first |
There was a problem hiding this comment.
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?)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 firstYeah, 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.
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
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.
MNT: Set figure width and height with set_size_inches
Fixes #4673
This modifies
set_figwidthandset_figheightso that they callset_size_inchesand pass along any additional kwargs (such asforward). It also adds some very basic tests for calling all three methods.