Fix behaviour of Figure.clear() for SubplotParams#27183
Conversation
f76c23f to
adebaa3
Compare
|
@jklymak Would you be able to review this pr? Thanks |
|
Why add an additional flag to I am not against the current implementation, but am curious why a more minimal approach would not work? |
@tacaswell This is a continuation of #11086 and I followed the implementation there. Looking at the code right now: the new flag is used both in I am fine with leaving the PR as is, or removing the flag in both |
|
@tacaswell Gentle ping |
Co-authored-by: Kyle Sunden <git@ksunden.space>
| setattr(self, key, mpl.rcParams[f"figure.subplot.{key}"]) | ||
| self.update(left, bottom, right, top, wspace, hspace) | ||
|
|
||
| def _repr_pretty_(self, p: Any, cycle: bool) -> None: |
There was a problem hiding this comment.
Why not simply define __repr__?
There was a problem hiding this comment.
__repr__ ends up at many places (logging, inside other repr of containers) and not everybody likes these kind of changes. The _repr_pretty_ is mainly used in interactive environments (spyder, jypyter noteobook, etc.) and visible to humans.
I do not think this change belongs in this PR though, so I removed it. Let me know if you want to to open a separate PR for this.
There was a problem hiding this comment.
It's rare that we/users ever access this, so I don't really care. But I think if we do this, we can afford to do __repr__. We don't have precedence for _repr_pretty_ in the code base. Take or leave a separate PR.
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
…b into clf_subplots_adjust
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Co-authored-by: Jody Klymak <jklymak@gmail.com>
Co-authored-by: Jody Klymak <jklymak@gmail.com>
Co-authored-by: Jody Klymak <jklymak@gmail.com>
|
You have a stray end of file issue that fails CI. |
jklymak
left a comment
There was a problem hiding this comment.
Small suggestion to the what's new (at the very least the line was too long).
If someone merges this, it should be squash-merged.
Co-authored-by: Jody Klymak <jklymak@gmail.com>
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
PR summary
Figure.clear()to reset the properties ofFigure.subplotpars.matplotlib.gridspec.SubplotParamsrc_defaultparameter to theSubplotParams.updateFixes #11059. This PR is a continuation of #11086 by @fredrik-1
PR checklist