Skip to content

Remove ticks and titles from tight bbox tests. #12790

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
Nov 11, 2018

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Nov 10, 2018

PR Summary

These text elements may shift with different versions of FreeType, making the tight bbox results unstable when building against external newer versions (such as this Fedora build against FreeType 2.9.1.)

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • [N/A] New features are documented, with examples if plot related
  • [N/A] Documentation is sphinx and numpydoc compliant
  • [N/A] Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • [N/A] Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

These text elements may shift with different versions of FreeType,
making the tight bbox results unstable when building against external
newer versions.
@QuLogic QuLogic added this to the v3.0.3 milestone Nov 10, 2018
@QuLogic QuLogic requested a review from jklymak November 10, 2018 09:35
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.

Its prob. fine for both these tests to not have text. OTOH, I don't see how you are passing many tests if you aren't using the testing version of free-type....

@timhoffm timhoffm merged commit 1ffaf41 into matplotlib:master Nov 11, 2018
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Nov 11, 2018
@jklymak
Copy link
Member

jklymak commented Nov 11, 2018

Um, ok I guess I should have blocked. Why was this change helpful?

@QuLogic QuLogic deleted the tightbbox-text branch November 11, 2018 23:32
@QuLogic
Copy link
Member Author

QuLogic commented Nov 11, 2018

For exactly the reason stated in the summary?

QuLogic added a commit that referenced this pull request Nov 11, 2018
…790-on-v3.0.x

Backport PR #12790 on branch v3.0.x (Remove ticks and titles from tight bbox tests.)
@jklymak
Copy link
Member

jklymak commented Nov 11, 2018

But how were any other tests passing?

The reason I ask is that the tightbbox machinery is supposed to care about text. So removing text, while not a big deal in this case, seems a strange thing to do.

@QuLogic
Copy link
Member Author

QuLogic commented Nov 12, 2018

I maintain a set of images generated with 2.9.1 for those tests. That's why I asked about constrained layout issues on gitter from time to time.

@jklymak
Copy link
Member

jklymak commented Nov 12, 2018

Ok thanks. I’ll keep that in mind for future tests. Obviously it’s nice not to have to make image tests for everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants