FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

ENH: compressed layout by jklymak · Pull Request #22289 · matplotlib/matplotlib · GitHub

Repository navigation

ENH: compressed layout - #22289

Merged
timhoffm merged 1 commit into
matplotlib:mainfrom
jklymak:enh-compress-layout
Jun 10, 2022
Merged

timhoffm merged 1 commit into
matplotlib:mainfrom
jklymak:enh-compress-layout

Conversation

jklymak commented Jan 22, 2022

Copy link
Copy Markdown
Member

PR Summary

Replaces #20016

We often want a grid of fixed-aspect ratio axes, but constrained_layout and tigt_layout leave large blank spaces between the axes because they only work with the original axes positions, not the aspect-modified versions.

Here I proposed an extension to constrianed_layout that works for simple grids of axes...

Old:

New:

Code

import matplotlib.pyplot as plt
import numpy as np

nrows = 2
ncols = 3
for compress in ['constrained', 'compressed']:
    print('compress?', compress)
    fig, axs = plt.subplots( nrows, ncols, figsize=(4,4),
                            layout=compress,
                            sharex=True, sharey=True)
    fig.patch.set_facecolor("0.9")
    #fig.set_constrained_layout_pads(compress=True)

    for i in range(nrows):
        for j in range(ncols):
            ax = axs[i, j]
            ax.set_aspect(1)
            pc = ax.pcolormesh(np.random.randn(30, 30))
    axs[0, 1].set_xlabel('Boooo')
    fig.colorbar(pc, ax=axs)

Failures...

This isn't perfect, and that is why it is not in "constrained_layout" Folks should be able to turn this off... This example isn't terrible, but its not very good either, though

fig, axs = plt.subplot_mosaic([['A', 'B', 'B'], ['C', 'D', 'E']], layout='compressed',
                            sharex=True, sharey=True)
for k in axs:
    axs[k].imshow(X)

Or:

fig, axs = plt.subplot_mosaic([['A', 'B', 'B'], ['C', 'B', 'B']], layout='compressed',
                            sharex=True, sharey=True, figsize=(3, 7))

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).

jklymak added the topic: geometry manager LayoutEngine, Constrained layout, Tight layout label Jan 22, 2022
jklymak added this to the v3.6.0 milestone Jan 22, 2022
jklymak marked this pull request as draft January 22, 2022 19:54
jklymak marked this pull request as ready for review January 24, 2022 08:35
Comment thread lib/matplotlib/layout_engine.py Outdated
hspace=mpl.rcParams['figure.constrained_layout.hspace'])
# set anything that was passed in (None will be ignored):
self.set(w_pad=w_pad, h_pad=h_pad, wspace=wspace, hspace=hspace)
# see CompressedLayoutEngine below...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This comment seems wrong now?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ooops, yes, I decided against needing a subclass here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Still needs fixing?

Comment thread lib/matplotlib/figure.py Outdated
Comment thread lib/matplotlib/_constrained_layout.py Outdated
Comment thread lib/matplotlib/_constrained_layout.py Outdated
Comment thread lib/matplotlib/_constrained_layout.py Outdated
jklymak force-pushed the enh-compress-layout branch 2 times, most recently from dcca890 to 8175f87 Compare March 24, 2022 14:00
Comment thread lib/matplotlib/_constrained_layout.py Outdated
Comment thread lib/matplotlib/_constrained_layout.py Outdated
Comment thread lib/matplotlib/figure.py Outdated
Comment thread lib/matplotlib/layout_engine.py Outdated
jklymak force-pushed the enh-compress-layout branch from e445e28 to 777b8ef Compare March 30, 2022 08:45
Comment thread lib/matplotlib/_constrained_layout.py Outdated
jklymak force-pushed the enh-compress-layout branch 2 times, most recently from ead8ac7 to e303fbb Compare May 16, 2022 08:22
Comment thread lib/matplotlib/layout_engine.py Outdated
hspace=mpl.rcParams['figure.constrained_layout.hspace'])
# set anything that was passed in (None will be ignored):
self.set(w_pad=w_pad, h_pad=h_pad, wspace=wspace, hspace=hspace)
# see CompressedLayoutEngine below...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Still needs fixing?

Comment thread lib/matplotlib/_constrained_layout.py Outdated
Comment thread lib/matplotlib/_constrained_layout.py Outdated
Comment on lines +474 to +484
fig, axs = plt.subplots(2, 2, figsize=(5, 3),
sharex=True, sharey=True, layout='compressed')
for ax in axs.flat:
ax.imshow(arr)
fig.suptitle("fixed-aspect plots, layout='compressed'")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This seems to look the same as the constrained version?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Agreed, https://output.circle-artifacts.com/output/job/4eee1b1a-1268-4881-8d21-61000c011c8e/artifacts/0/doc/build/html/tutorials/intermediate/constrainedlayout_guide.html#grids-of-fixed-aspect-ratio-axes-compressed-layout

but I have no idea what is happening here. This example works fine manually, just using Agg, etc. I'm flummoxed why the sphinx version doesn't work...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Fixed in newest version - there was some weird interactions between the rcParam and subsequent calls. maybe worth chasing down, but I could not reproduce without sphinx-gallery, so I guess sphinx-galery is using some weird rcParam contexts?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think the context remains the same through a single file.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yes, but even if I change the rcParam in a normal script, and then specify the kwarg, compress works fine. But in sphinx- gallery it has this weird problem.

jklymak force-pushed the enh-compress-layout branch from 5380aa1 to dbcb46c Compare May 28, 2022 11:44
jklymak marked this pull request as draft May 28, 2022 17:56
jklymak force-pushed the enh-compress-layout branch from f14b4bb to 1adfa9d Compare May 31, 2022 12:27
jklymak marked this pull request as ready for review May 31, 2022 12:45
jklymak force-pushed the enh-compress-layout branch from 1adfa9d to 421c99e Compare May 31, 2022 12:59
Comment on lines +474 to +484
fig, axs = plt.subplots(2, 2, figsize=(5, 3),
sharex=True, sharey=True, layout='compressed')
for ax in axs.flat:
ax.imshow(arr)
fig.suptitle("fixed-aspect plots, layout='compressed'")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think the context remains the same through a single file.

Comment thread lib/matplotlib/_constrained_layout.py Outdated
A value of 0.2 for a three-column layout would have a space
of 0.1 of the figure width between each column.
If h/wspace < h/w_pad, then the pads are used instead.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

For consistency with above, this empty line should be here (and one added after the rect description) or all above also removed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sure, though this is private, so never rendered in the docs ;-)

jklymak force-pushed the enh-compress-layout branch from 421c99e to 1984d55 Compare June 1, 2022 06:53
Adds compress=True to compressed layout engine.  Works for compact
axes grids.

jklymak commented Jun 10, 2022

Copy link
Copy Markdown
Member Author

I've marked merge with single review. This is pretty stand alone, so it shouldn't break anyone. However, happy to get a second review. I'll merge by next Friday if it doesn't attract one.

timhoffm merged commit b396481 into matplotlib:main Jun 10, 2022

jklymak commented Jun 10, 2022

Copy link
Copy Markdown
Member Author

Thanks everyone for your work on this!

Copy link
Copy Markdown
Member

Thanks everyone for your work on this!

Thanks for creating this! 😄

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: geometry manager LayoutEngine, Constrained layout, Tight layout

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL