Skip to content

Add machinery for png-only, single-font mathtext tests. #19261

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 3 commits into from
Jan 16, 2021

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jan 8, 2021

Alternate for #19201. png turns out to be heavier than svg-without-font embedding, but if we stick to a single font it's cheaper that testing all 5 fonts in svg. (For the sqrt test added below, each font adds ~2k to the size of the baseline image file.)

PR Summary

PR Checklist

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

horizontalalignment='center', verticalalignment='center')


font_tests = list(filter(lambda x: x[1] is not None, enumerate(font_tests)))
Copy link
Member

Choose a reason for hiding this comment

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

I think this filtering should stay right below the definition of font_tests, because it's only a helper to sanetize the list (namely, to skip deleted tests).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added another commit (actually, a first commit) to factor out test skipping for all of mathtext_rendering, mathtext_rendering_lightweight, and mathfont_rendering.

Just pytest.skip()ping them in the baseline_images fixture will avoid
having to duplicate the logic in the upcoming png-only tests.
(For the sqrt test added below, each font adds ~2k to the size of the
baseline image file.)
@anntzer anntzer force-pushed the mathtextlightweighttests branch from 86f97b2 to 7f7fad9 Compare January 9, 2021 12:54
@@ -114,6 +114,7 @@
# default) and only png outputs, in order to minimize the size of baseline
# images.
lightweight_math_tests = [
r'$\sqrt[ab]{123}$', # github issue #8665
Copy link
Member

Choose a reason for hiding this comment

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

What's up with the ab alignment? It doesn't appear to follow the baseline to me, but align to top instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This only appears with text.hinting = "none" (which is used by tests, but is not the default otherwise). I suspect this is just one of the vertical alignments problems mentioned in #5414.
Also manually testing with a large text size (e.g. 48) allows verifying that there is no top-alignment.

@QuLogic QuLogic closed this Jan 16, 2021
@QuLogic QuLogic reopened this Jan 16, 2021
@QuLogic QuLogic merged commit 07d4530 into matplotlib:master Jan 16, 2021
@QuLogic QuLogic added this to the v3.4.0 milestone Jan 16, 2021
@anntzer anntzer deleted the mathtextlightweighttests branch January 16, 2021 20:51
@ednl
Copy link

ednl commented Jan 31, 2021

Thanks for doing this. Can't wait for it to be released, mainly for "noninteger bases in mathtext sqrt" which I ran in to recently.

@anntzer anntzer mentioned this pull request Feb 7, 2021
7 tasks
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