-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
FIX: do not reset ylabel ha when changing position #18430
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
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.
Looks, good, though I don't understand why you added an import 😉
lib/matplotlib/tests/test_axes.py
Outdated
fig = Figure() | ||
ax = fig.gca() |
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.
fig = Figure() | |
ax = fig.gca() | |
fig, ax = plt.subplots() |
I was intentionally writing the tests to not invoke the pyplot machinery to see how it feels. |
It feels like you had to do an extra import for no good reason. 😉 I'm not blocking, but I'd use |
I'm ok with the extra import, but agree, that On a general note: What is the motivation to leave out pyplot? Is there a perfromance benefit, or is it the desire to reduce the dependency on pyplot. Is there a plan to move larger parts of the tests away from pyplot? |
708c0f3
to
95d2158
Compare
95d2158
to
ad7007b
Compare
Only took 4 rebases... |
PR Summary
closes #18427
This line came in via b2a80a7 / #1589 . It looks like everything still passes without it (!!) so I think that means either we never needed it or the work done since then on the colorbar code has made it redundant.
PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
andpydocstyle<4
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).