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

Group figure.subplot.* rc to a single rcParam. by anntzer · Pull Request #11231 · matplotlib/matplotlib · GitHub

Repository navigation

Group figure.subplot.* rc to a single rcParam. - #11231

Closed
anntzer wants to merge 1 commit into
matplotlib:mainfrom
anntzer:single-subplot-adjust
Closed

anntzer wants to merge 1 commit into
matplotlib:mainfrom
anntzer:single-subplot-adjust

Conversation

anntzer commented May 11, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

The idea is to make it easier to restore a figure's subplotparams to the
rc-provided defaults. Specifically, this can now be done with

fig.subplots_adjust(**rcParams["figure.subplot"])

or

fig.subplots_adjust(**dict(rcParams["figure.subplot"], left=...))

if some values need to be overridden.

Also make Figure.clf() restore the subplotsparams to these rc-provided
defaults (that is the original motivation for the change).

Note that this PR runs into a limitation of the rcParams API: it would
be nicer if the validators took the rcParams as parameters (e.g.
implicitly by being methods of the RcParams class), so that they can
actually depend on another key in the same instance.

Also, the tests now restore original rcParams using dict.update to
avoid revalidating them and running into the warnings.
(rc_context.__exit__ does the same.)

Alternative to #11086.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

Copy link
Copy Markdown
Member

This PR seems to restrict the subplot params into the the range [0,1]. This not useful.
All you have to check for is that left is smaller than right and bottom is smaller than top.
In addition you may check for left==right, but that may need to be allowed too, and it may be a fall-back in tight_layout.

I don't like figure.subplot, because it sound so general... like "Hell yes, I want a subplot, so let's set it to True". But I suppose it's the natural name, given the current params.

anntzer commented May 11, 2018

Copy link
Copy Markdown
Contributor Author

I just copied the name from figure.subplot.foo, don't really like it either, happy to change it for something better.
The 0 < ... < 1 restriction comes from the rcParams validator, will drop it, but note (as a side point) the benefit of having a single validation point for the rcParams...

anntzer force-pushed the single-subplot-adjust branch from f04d697 to ebb16de Compare May 11, 2018 15:05

Copy link
Copy Markdown
Member

Uh, so maybe that validator should change. I agree that it doesn't make sense to be able to set values larger 1 only via the api but not via rc.

Resons to keep it as it is:

  • You may simply want to have >100% space between subplots (e.g. for small plots with large captions).
  • When using the bbox_inches="tight" saving option, you may use some negative margin to get a desired output.
  • When combining subplots with 3d axes you might want to fiddle around with the subplot_params to get a 2d and 3d plot nicely aligned.
  • When creating a wide figure with an image and a colorbar you may use left < 0 to center the image. When creating a subplot grid with colorbars you may use right > 1 to reduce space to the right of the colorbar.
  • When creating a wedged polar plot, you may need out of range params to get the plot tight into the figure (Wedged polar plot's position/scaling leaves too much white space. #10264)

The latter are mainly workarounds, but as long as such problems do not have a good solution (and there will always such special cases I figure) it would be a pitty to artificially limit the available options to solve them quickly.

anntzer force-pushed the single-subplot-adjust branch 2 times, most recently from 5955f3a to 80b1a8e Compare May 11, 2018 16:09

anntzer commented May 11, 2018

Copy link
Copy Markdown
Contributor Author

Oh I see, the validators for hspace and wspace were just completely wrong too... killed them as well.

anntzer force-pushed the single-subplot-adjust branch 3 times, most recently from 6398d16 to 4e16522 Compare May 11, 2018 22:19
The idea is to make it easier to restore a figure's subplotparams to the
rc-provided defaults.  Specifically, this can now be done with

    fig.subplots_adjust(**rcParams["figure.subplot"])

or

    fig.subplots_adjust(**dict(rcParams["figure.subplot"], left=...))

if some values need to be overridden.

Also make Figure.clf() restore the subplotsparams to these rc-provided
defaults (that is the original motivation for the change).

Note that this PR runs into a limitation of the rcParams API: it would
be nicer if the validators took the rcParams as parameters (e.g.
implicitly by being methods of the RcParams class), so that they can
actually depend on another key in the same instance.

github-actions Bot commented May 6, 2023

Copy link
Copy Markdown

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.

github-actions Bot added the status: inactive Marked by the “Stale” Github Action label May 6, 2023
anntzer mentioned this pull request May 9, 2023
5 tasks

anntzer commented May 9, 2023

Copy link
Copy Markdown
Contributor Author

Just for tracking purposes, the surrounding machinery has mostly all been pr'd separately; the only question remaining is whether we want to group the rcparams into a single dict or not. (I still think this would be a good idea -- probably easiest would be to just start a new pr from scratch.)

github-actions Bot removed the status: inactive Marked by the “Stale” Github Action label May 10, 2023

Copy link
Copy Markdown
Member

the only question remaining is whether we want to group the rcparams into a single dict or not.

I think we should not do this right now. While a single dict would make some things easier, other things get hader. You'll have to write rcParams["figure.subplot"]["wspace"] instead of rcParams["figure.subplot.wspace"], which is more cumbersome and an API change. The dict-base rcParams is not suited to make the transition easy and it also lacks features to support all use cases well.

I suggest to defer this to the redesign of the config system #24585. In particular. we can build a proper hierarchial interface there, which supports both rcParams["figure.subplot.wspace"] and config["figure.subplot"]. With a bit of careful design, the latter could be a dict or dict-like.

anntzer closed this May 10, 2023
anntzer deleted the single-subplot-adjust branch May 10, 2023 12:35
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL