-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
DOC: remove constrained_layout kwarg from examples #25198
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
import matplotlib.lines as ml | ||
from matplotlib.ticker import Formatter | ||
|
||
# Load a numpy record array from yahoo csv data with fields date, open, high, | ||
# low, close, volume, adj_close from the mpl-data/sample_data directory. The | ||
# record array stores the date as an np.datetime64 with a day unit ('D') in | ||
# the date column (``r.date``). | ||
r = (cbook.get_sample_data('goog.npz', np_load=True)['price_data'] | ||
.view(np.recarray)) | ||
r = cbook.get_sample_data('goog.npz', np_load=True)['price_data'].view(np.recarray) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know we said we wouldn't re-flow, but this line was crying out for it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small nit. Could self merge if you take it or not so long as the doc build and flake8 pass.
r = r[:9] # get the first 9 days | ||
|
||
fig, (ax1, ax2) = plt.subplots(nrows=2, figsize=(6, 6), | ||
constrained_layout={'hspace': .15}) | ||
layout=ConstrainedLayoutEngine(hspace=0.15)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't mind this, but obscure imports are sometimes not nice. Maybe:
layout=ConstrainedLayoutEngine(hspace=0.15)) | |
layout='constrained') | |
fig.get_layout_engine().set(hspace=0.15) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel strongly either way, so done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I am not updated on the 3.7 status, I am not merging, but looks good!
8b0a2ff
to
8278163
Compare
8278163
to
e0dcac8
Compare
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
I’ll take a look at the backport after 3.7.0 is out. Thanks for the reviews! |
…rom examples Manual backport of matplotlib#25198 due to conflicts in examples/subplots_axes_and_figures/demo_constrained_layout.py This was because some of the changes here were right below the section separators which changed in main at matplotlib#25021.
Backport PR #25198 - DOC: remove constrained_layout kwarg from examples
PR Summary
The form
constrained_layout=True
is discouraged in the relevant docstrings since v3.5 (#19892), but is still used across the examples. This PR updates the examples with the preferred form. For the strings, I tried to choose single vs double quotes to be consistent with any strings on nearby lines. Also puts instances of the phrase "constrained layout" in italics, consistent with the decision for the constrained layout tutorial at #25144.The tutorials and user guide also have
constrained_layout=True
in places, but I think that can be a separate PR.PR Checklist
Documentation and Tests
pytest
passes)Release Notes
.. versionadded::
directive in the docstring and documented indoc/users/next_whats_new/
.. versionchanged::
directive in the docstring and documented indoc/api/next_api_changes/
next_whats_new/README.rst
ornext_api_changes/README.rst