ENH: rect for constrained_layout by jklymak · Pull Request #22627 · matplotlib/matplotlib

jklymak · 2022-03-09T05:47:08Z

PR Summary

Closes #22623 adding a rectangle to constrained layout

import matplotlib.pyplot as plt
import numpy as np
import matplotlib as mpl

fig, ax = plt.subplots(layout='constrained')
fig.get_layout_engine().set(rect=[0.1, 0.1, 0.5, 0.5])
plt.show()

PR Checklist

Tests and Styling

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

New features are documented, with examples if plot related.
New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
Documentation is sphinx and numpydoc compliant (the docs should build without error).

lib/matplotlib/_constrained_layout.py

greglucas

Looks good.

greglucas · 2022-03-11T03:18:08Z

lib/matplotlib/_constrained_layout.py

       of 0.1 of the figure width between each column.
       If h/wspace < h/w_pad, then the pads are used instead.

+    rect : [l, b, w, h]


Just a minor suggestion for the inputs here.

Suggested change

rect : [l, b, w, h]

rect : tuple of 4 floats

This should be the type, you're already specifying what they are below.

Also, do you want to change all the default inputs to tuples instead of a list, rect=(0, 0, 1, 1) to avoid mutability concerns?

I'm fine to do this, but note that we don't do this elsewhere in the library when we specify rectangles?

You're right, it seems rects are specified in a lot of different ways!

rect : tuple[float, float, float, float], optional rect : sequence of float rect : tuple (left, bottom, right, top), default: (0, 0, 1, 1) rect : tuple of 4 floats, default: (0, 0, 1, 1) rect : [left, bottom, width, height] rect : (float, float, float, float)

I'm pretty indifferent on most of those styles, but I don't think the orientation belongs on this first line. My approval still stands regardless of which format of documentation you choose, consolidation of rect across matplotlib should be a follow-up issue.

I changed this, though this is a private module.

greglucas · 2022-03-11T03:21:10Z

lib/matplotlib/_layoutgrid.py

-            hc = [self.lefts[0] == 0,
-                  self.rights[-1] == 1,
+        if not isinstance(parent, LayoutGrid):
+            # specify a rectangle in figure co-ordinates


Suggested change

# specify a rectangle in figure co-ordinates

# specify a rectangle in figure coordinates

Fixed, though its a private module, and I like the British spelling better (at least I don't add an umlaut!) ;-)

oscargus · 2022-03-13T14:20:16Z

lib/matplotlib/_constrained_layout.py



-def make_layoutgrids(fig, layoutgrids):
+def make_layoutgrids(fig, layoutgrids, rect=[0, 0, 1, 1]):


Suggested change

def make_layoutgrids(fig, layoutgrids, rect=[0, 0, 1, 1]):

def make_layoutgrids(fig, layoutgrids, rect=(0, 0, 1, 1)):

I guess the immutability is important enough to guard here...

oscargus · 2022-03-13T14:21:45Z

lib/matplotlib/layout_engine.py

            :rc:`figure.constrained_layout.wspace`.
+        rect : tuple of 4 floats
+            Rectangle in figure coordinates to perform constrained layout in
+            [left, bottom, width, height], each from 0-1.


Suggested change

[left, bottom, width, height], each from 0-1.

(left, bottom, width, height), each from 0-1.

oscargus · 2022-03-13T14:22:04Z

lib/matplotlib/_constrained_layout.py


+    rect : tuple of 4 floats
+        Rectangle in figure coordinates to perform constrained layout in
+        [left, bottom, width, height], each from 0-1.


Suggested change

[left, bottom, width, height], each from 0-1.

(left, bottom, width, height), each from 0-1.

Should one possibly add default values here? (And in other locations.)

Btw, doesn't look like there is a renderer parameter (anymore?).

Yes, needed to rebase.

oscargus · 2022-03-13T14:23:11Z

lib/matplotlib/layout_engine.py

+        rect : [l, b, w, h]
+            Rectangle in figure coordinates to perform constrained layout in
+            [left, bottom, width, height], each from 0-1.


Suggested change

rect : [l, b, w, h]

Rectangle in figure coordinates to perform constrained layout in

[left, bottom, width, height], each from 0-1.

rect : tuple of 4 floats

Rectangle in figure coordinates to perform constrained layout in

(left, bottom, width, height), each from 0-1.

Fixed all these, also in TightLayoutEngine..

anntzer · 2022-05-27T10:00:46Z

@jklymak Is there any reason why you deleted pytest.ini and tests.py here?

jklymak · 2022-05-27T10:03:23Z

No, must have been a rebase gone awry? Sorry I didn't notice that.

anntzer · 2022-05-27T10:06:51Z

No worries, I'll restore them.

jklymak · 2022-05-27T10:07:01Z

Looks like we have a new pytest.ini.

jklymak · 2022-05-27T10:07:11Z

What does test.py do?

anntzer · 2022-05-27T10:11:24Z

It's been discussed a couple of times, e.g. at #20586. I think we should probably get rid of it at some point, but there still seems to be some users for now so that should be discussed separately.

jklymak added the topic: geometry manager LayoutEngine, Constrained layout, Tight layout label Mar 9, 2022

jklymak added this to the v3.6.0 milestone Mar 9, 2022

oscargus reviewed Mar 9, 2022

View reviewed changes

lib/matplotlib/_constrained_layout.py Outdated Show resolved Hide resolved

greglucas approved these changes Mar 11, 2022

View reviewed changes

jklymak force-pushed the enh-rect-for-CL branch 3 times, most recently from c39027a to acae2c4 Compare March 11, 2022 05:12

oscargus reviewed Mar 13, 2022

View reviewed changes

ENH: add rect parameter to constrained_layout

21263a5

jklymak force-pushed the enh-rect-for-CL branch from 08d0b6a to 21263a5 Compare April 8, 2022 09:26

jklymak requested a review from oscargus May 8, 2022 17:52

oscargus approved these changes May 8, 2022

View reviewed changes

oscargus merged commit 9537906 into matplotlib:main May 8, 2022

jklymak deleted the enh-rect-for-CL branch May 27, 2022 10:07

anntzer mentioned this pull request May 27, 2022

Restore accidentally removed pytest.ini and tests.py. #23153

Merged

6 tasks

	# specify a rectangle in figure co-ordinates
	# specify a rectangle in figure coordinates



		def make_layoutgrids(fig, layoutgrids):
		def make_layoutgrids(fig, layoutgrids, rect=[0, 0, 1, 1]):

	[left, bottom, width, height], each from 0-1.
	(left, bottom, width, height), each from 0-1.

Uh oh!

Conversation

jklymak commented Mar 9, 2022

PR Summary

PR Checklist

Uh oh!

Uh oh!

greglucas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anntzer commented May 27, 2022

Uh oh!

jklymak commented May 27, 2022

Uh oh!

anntzer commented May 27, 2022

Uh oh!

jklymak commented May 27, 2022

Uh oh!

jklymak commented May 27, 2022

Uh oh!

anntzer commented May 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants