Skip to content

Changes to figure.clf() and suplot_adjust #11086

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

Conversation

fredrik-1
Copy link
Contributor

I wrote some code to solve #11059

  • figure.clf() now sets the subplots parameters to their default values. This is a behavior that I have missed before and I think that it is the correct thing to do.
  • A new keyword rc_default is added to subplots_adjust to make it simpler to get back to default values when tuning the subplot parameters
  • The SubplotParams object has got a __repr__ function with the important attributes such that it is easier to see and print what the subplot parameters are
  • The SubplotParams object has got a get function to make it easier to get its parameters
  • The code in SubplotParams.update has been written more compact
  • A few tests have been written to test the old and new behavior
  • The docstrings has been changed such that they are more complete and (mostly at least) follows the new docstring directive

I think that these changes make it easier to work with and understand the subplot parameters without changing the interface to much.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • Documentation is sphinx and numpydoc compliant

@fredrik-1 fredrik-1 force-pushed the figure_clf_subplots_adjust_#11059 branch from d2289c9 to 54d5e19 Compare April 19, 2018 08:10
@jklymak jklymak changed the title Changes to figure.clf() and suplot_adjust #11059 Changes to figure.clf() and suplot_adjust Apr 19, 2018
Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

Thanks a lot, I think this looks good. But the new behaviour is a breaking change, so it needs an API change note (matplotlib/doc/api/next_api_changes/). You also have some PEP8 errors (see the CI that fails).

Set *keep_observers* to True if, for example,
a gui widget is tracking the axes in the figure.
Parameters
---------
Copy link
Member

Choose a reason for hiding this comment

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

Missed underline...

@jklymak jklymak added this to the v3.0 milestone Apr 19, 2018
@fredrik-1 fredrik-1 force-pushed the figure_clf_subplots_adjust_#11059 branch 4 times, most recently from 2a96fc9 to 1afff75 Compare April 25, 2018 11:30
@fredrik-1 fredrik-1 force-pushed the figure_clf_subplots_adjust_#11059 branch from 1afff75 to 541f151 Compare May 11, 2018 09:20
@@ -173,77 +173,81 @@ def __init__(self, left=None, bottom=None, right=None, top=None,
wspace=None, hspace=None):
"""
All dimensions are fractions of the figure width or height.
Defaults are given by :rc:`figure.subplot.[name]`.
Defaults are given by :rc:`figure.subplot.*`.
Copy link
Contributor

Choose a reason for hiding this comment

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

would leave [name]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't really like any of them but I don't know a better one. I think that figure.subplot.[name] make it easier to understand what to write.

@anntzer
Copy link
Contributor

anntzer commented May 11, 2018

I think a simpler API would be to have rcParams["figure.subplot"] be a dict (xref the python-syntax matplotlibrc MEP...) and just let the user do

plt.subplots_adjust(**{**rcParams["figure.subplot"], "left": ..., (whatever they want to override)})

@fredrik-1
Copy link
Contributor Author

A dict would be good but I don't think that that API are very good.

@fredrik-1 fredrik-1 force-pushed the figure_clf_subplots_adjust_#11059 branch from 541f151 to de2f714 Compare May 16, 2018 20:31
@fredrik-1
Copy link
Contributor Author

I made a small change so that the test passed again.

@jklymak
Copy link
Member

jklymak commented Feb 7, 2019

@fredrik-1 sorry this fell off our radar - be sure to ping us if you pick it up again and you want it reviewed.

@jklymak jklymak modified the milestones: v3.1.0, v3.2.0 Feb 7, 2019
@timhoffm timhoffm modified the milestones: v3.2.0, v3.3.0 Aug 16, 2019
@jklymak jklymak added the stale label Oct 1, 2019
@jklymak
Copy link
Member

jklymak commented Oct 14, 2019

I'm going to close as abandoned, but feel free to re-open if wanted. Thanks for the help @fredrik-1

@jklymak jklymak closed this Oct 14, 2019
@QuLogic QuLogic modified the milestones: v3.3.0, unassigned Jul 9, 2020
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.

6 participants