GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add machinery for png-only, single-font mathtext tests. by anntzer · Pull Request #19261 · matplotlib/matplotlib

Conversation

@anntzer

@anntzer anntzer commented Jan 8, 2021

Copy link
Copy Markdown
Contributor

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).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@anntzer anntzer added topic: testing topic: text/mathtext labels Jan 8, 2021
timhoffm
timhoffm reviewed Jan 9, 2021
View reviewed changes
Comment thread lib/matplotlib/tests/test_mathtext.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/matplotlib/tests/test_mathtext.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/matplotlib/tests/test_mathtext.py Outdated
horizontalalignment='center', verticalalignment='center')


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

@timhoffm timhoffm Jan 9, 2021

Copy link
Copy Markdown
Member

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 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).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@anntzer anntzer Jan 9, 2021

Copy link
Copy Markdown
Contributor Author

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 added another commit (actually, a first commit) to factor out test skipping for all of mathtext_rendering, mathtext_rendering_lightweight, and mathfont_rendering.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
anntzer added 3 commits January 9, 2021 13:45
@anntzer
Use simpler approach to skip previously-removed mathtext tests.
2971563 
Just pytest.skip()ping them in the baseline_images fixture will avoid
having to duplicate the logic in the upcoming png-only tests.
@anntzer
Add machinery for png-only, single-font mathtext tests.
d6687ce 
(For the sqrt test added below, each font adds ~2k to the size of the
baseline image file.)
@anntzer
Support \sqrt[not-a-number]{...}.
7f7fad9
@anntzer
anntzer force-pushed the mathtextlightweighttests branch from 86f97b2 to 7f7fad9 Compare January 9, 2021 12:54
timhoffm
timhoffm approved these changes Jan 9, 2021
View reviewed changes
QuLogic
QuLogic reviewed Jan 13, 2021
View reviewed changes
Comment thread lib/matplotlib/tests/test_mathtext.py
# default) and only png outputs, in order to minimize the size of baseline
# images.
lightweight_math_tests = [
r'$\sqrt[ab]{123}$', # github issue #8665

@QuLogic QuLogic Jan 13, 2021

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@anntzer anntzer Jan 13, 2021

Copy link
Copy Markdown
Contributor Author

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@QuLogic QuLogic mentioned this pull request Jan 16, 2021
Create tiny mathtext baseline images using svg with non-embedded fonts. #19201
Closed
7 tasks
QuLogic
QuLogic approved these changes Jan 16, 2021
View reviewed changes
@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
This was referenced Jan 16, 2021
Support non-integer bases in mathtext sqrt #19186
Closed
Noninteger Bases in mathtext sqrt #8665
Closed
@ednl

ednl commented Jan 31, 2021

Copy link
Copy Markdown

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@anntzer anntzer mentioned this pull request Feb 7, 2021
Fix over/under mathtext symbols #19314
Merged
5 of 7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@QuLogic QuLogic QuLogic approved these changes
@timhoffm timhoffm timhoffm approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic: testing topic: text/mathtext

Projects

None yet

Milestone

v3.4.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@anntzer @ednl @QuLogic @timhoffm
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.