Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Merged
merged 1 commit into from
Sep 23, 2020

Conversation

tacaswell
Copy link
Member

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

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • [N/A ] New features are documented, with examples if plot related.
  • [N/A] Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and pydocstyle<4 and run flake8 --docstring-convention=all).
  • [N/A] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [N/A] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

@tacaswell tacaswell added this to the v3.4.0 milestone Sep 7, 2020
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.

Looks, good, though I don't understand why you added an import 😉

lib/matplotlib/tests/test_axes.py Show resolved Hide resolved
Comment on lines 6522 to 6523
fig = Figure()
ax = fig.gca()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fig = Figure()
ax = fig.gca()
fig, ax = plt.subplots()

@tacaswell
Copy link
Member Author

I was intentionally writing the tests to not invoke the pyplot machinery to see how it feels.

@jklymak
Copy link
Member

jklymak commented Sep 8, 2020

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 fig.add_subplots() rather than gca() if we want to develop good grammar for this?

@timhoffm
Copy link
Member

I'm ok with the extra import, but agree, that fig.add_subplots() would be better than gca(), the latter being too pyploty.

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?

@tacaswell tacaswell force-pushed the fix_no_realign_ylabel branch 2 times, most recently from 708c0f3 to 95d2158 Compare September 23, 2020 15:18
lib/matplotlib/tests/test_axes.py Outdated Show resolved Hide resolved
@tacaswell tacaswell force-pushed the fix_no_realign_ylabel branch from 95d2158 to ad7007b Compare September 23, 2020 15:35
@tacaswell
Copy link
Member Author

Only took 4 rebases...

@jklymak jklymak merged commit 2486ba2 into matplotlib:master Sep 23, 2020
@tacaswell tacaswell deleted the fix_no_realign_ylabel branch September 23, 2020 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

yaxis.set_label_position("right") resets "horizontalalignment"
5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.