Skip to content

Changes to figure.clf() and suplot_adjust - #11086

Closed
fredrik-1 wants to merge 1 commit into
matplotlib:masterfrom
fredrik-1:figure_clf_subplots_adjust_#11059
Closed

Changes to figure.clf() and suplot_adjust#11086
fredrik-1 wants to merge 1 commit into
matplotlib:masterfrom
fredrik-1:figure_clf_subplots_adjust_#11059

Conversation

@fredrik-1

@fredrik-1 fredrik-1 commented Apr 19, 2018

Copy link
Copy Markdown
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
jklymak
jklymak reviewed Apr 19, 2018
View reviewed changes

@jklymak jklymak left a comment

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.

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

Comment thread lib/matplotlib/figure.py Outdated
Set *keep_observers* to True if, for example,
a gui widget is tracking the axes in the figure.
Parameters
---------

@jklymak jklymak Apr 19, 2018

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.

Missed underline...

@jklymak jklymak added the API: changes Changes to the public API, typically requiring deprecation. label Apr 19, 2018
@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
anntzer
anntzer reviewed May 11, 2018
View reviewed changes
Comment thread lib/matplotlib/figure.py
"""
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.*`.

@anntzer anntzer May 11, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would leave [name]

@fredrik-1 fredrik-1 May 16, 2018

Copy link
Copy Markdown
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

anntzer commented May 11, 2018

Copy link
Copy Markdown
Contributor

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

@anntzer anntzer mentioned this pull request May 11, 2018
Closed
6 tasks
@fredrik-1

fredrik-1 commented May 16, 2018

Copy link
Copy Markdown
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

fredrik-1 commented May 18, 2018

Copy link
Copy Markdown
Contributor Author

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

@tacaswell tacaswell modified the milestones: v3.0, v3.1 Jul 7, 2018
@jklymak

jklymak commented Feb 7, 2019

Copy link
Copy Markdown
Member

@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
@ImportanceOfBeingErnest ImportanceOfBeingErnest mentioned this pull request Sep 8, 2019
Merged
6 tasks
@jklymak jklymak added the stale label Oct 1, 2019
@jklymak

jklymak commented Oct 14, 2019

Copy link
Copy Markdown
Member

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
@eendebakpt eendebakpt mentioned this pull request Oct 24, 2023
Merged
4 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anntzer anntzer anntzer left review comments
@jklymak jklymak jklymak left review comments

Assignees

No one assigned

Labels

API: changes Changes to the public API, typically requiring deprecation. status: needs rebase status: superseded

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@fredrik-1 @anntzer @jklymak @tacaswell @QuLogic @timhoffm