FIX: correctly handle generic font families in svg text-as-text mode by tacaswell · Pull Request #23638 · matplotlib/matplotlib

tacaswell · 2022-08-16T15:45:56Z

PR Summary

This:

expands the generic fonts families to the user configured specific fonts
in the svg output
ensures that each font family only appears once per text element in the svg output
ensures that generic families are unquoted and specific families are

I still need to write a test for this, but interactively this gives things like

 <text style="font: 10px 'DejaVu Sans', 'WenQuanYi Zen Hei', 'DejaVu Sans', 'Bitstream Vera Sans', 'Computer Modern Sans Serif', 'Lucida Grande', 'Verdana', 'Geneva', 'Lucid', 'Arial', 'Helvetica', 'Avant Garde', sans-serif; text-anchor: start" x="236.16" y="174.528" transform="rotate(-0 236.16 174.528)">There are 几个汉字 in between!</text>

PR Checklist

Tests and Styling

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

tacaswell · 2022-08-17T02:40:44Z

This might require some changes to sort out if CI has the fonts (or just just the "wrong" kind of font to exercise the code).

tacaswell · 2022-08-17T04:58:24Z

I appear to have introduced a memory leak as the runners are getting OOM'd and running the tests with a single worker is using ~2G of ram.

tacaswell · 2022-08-17T14:23:42Z

This seems to cause an 8x increase in memory usage and a 10x slow down in the test suite.

anntzer · 2022-08-18T16:01:01Z

lib/matplotlib/font_manager.py

-                        family,
-                        ", ".join(self._expand_aliases(family))
+            if family in font_family_aliases:
+                # deal with the sans/san-serif/sans serif issue


This and the following line (the start of the for loop) can be just

for fam in self._expand_aliases(family): if fam in font_family_aliases: continue # is that even possible? ...

This will go into another PR, this change seems to be causing a 10x memory increase and I do not understand why yet.

tacaswell · 2022-08-19T19:42:31Z

This is not ready yet and I do not want to hold 3.6 on it. It is possible that the changes will be small enough to backport to a 3.6.x, but I am not optimistic.

tacaswell · 2022-09-01T02:00:11Z

I think this may now be small enough to backport to v3.6.x but I do not think we should block 3.6.0 over it.

anntzer · 2022-09-01T13:55:28Z

lib/matplotlib/backends/backend_svg.py

            if weight != 400:
                font_parts.append(f'{weight}')
+
+            def unique_iter(inp):


That's actually [*dict.fromkeys(inp)] (or even without the conversion to list which is not needed in this case, iterating over the keys is fine); this is so short that it could even be inlined with a comment, something like dict.fromkeys(s for fam in prop.get_family() for s in _format_font_name(fam)) (or dict.fromkeys(itertools.chain(*map(_format_font_name, prop.get_family()))) for itertools aficionados)

I replaced unique_iter, but didn't do anything fancier.

lib/matplotlib/tests/test_backend_svg.py

This tests: 1. all of the fonts from the generic lists Matplotlib maintains ends up in the svg 2. the generic family is included and un-escaped 3. the specific fonts are escaped 4. glyph fallback will happen in fonts found via generic family names

This: - expands the generic fonts families to the user configured specific fonts in the svg output - ensures that each font family only appears once per text element in the svg output - ensures that generic families are unquoted and specific families are closes matplotlib#22528 closes matplotlib#23492

If the cached function raises it will not be cached and we will continuously pay for cache misses.

QuLogic · 2022-09-30T04:53:16Z

I pushed some minor corrections to the comments directly.

tacaswell · 2022-09-30T22:27:37Z

I'm going to merge this on @QuLogic 's review via working on it and @anntzer 's review.

…lies in svg text-as-text mode

…638-on-v3.6.x Backport PR #23638 on branch v3.6.x (FIX: correctly handle generic font families in svg text-as-text mode)

tacaswell added backend: svg status: needs tests labels Aug 16, 2022

tacaswell added this to the v3.6.0 milestone Aug 16, 2022

tacaswell mentioned this pull request Aug 16, 2022

fixed font prop in text elements in svg figures #22541

Closed

tacaswell force-pushed the svg_generic_fonts branch from b31ed3f to 41a1777 Compare August 17, 2022 02:40

tacaswell marked this pull request as ready for review August 17, 2022 02:40

tacaswell removed the status: needs tests label Aug 17, 2022

tacaswell marked this pull request as draft August 17, 2022 14:23

anntzer reviewed Aug 18, 2022

View reviewed changes

tacaswell modified the milestones: v3.6.0, v3.7.0 Aug 19, 2022

tacaswell force-pushed the svg_generic_fonts branch 2 times, most recently from 36725ed to af2cc4f Compare September 1, 2022 01:51

tacaswell modified the milestones: v3.7.0, v3.6.1 Sep 1, 2022

tacaswell marked this pull request as ready for review September 1, 2022 02:07

anntzer reviewed Sep 1, 2022

View reviewed changes

QuLogic force-pushed the svg_generic_fonts branch from af2cc4f to 26345f8 Compare September 29, 2022 04:09

anntzer reviewed Sep 29, 2022

View reviewed changes

lib/matplotlib/tests/test_backend_svg.py Outdated Show resolved Hide resolved

QuLogic force-pushed the svg_generic_fonts branch from 26345f8 to 3b9dc06 Compare September 29, 2022 05:58

oscargus reviewed Sep 29, 2022

View reviewed changes

lib/matplotlib/tests/test_backend_svg.py Outdated Show resolved Hide resolved

QuLogic force-pushed the svg_generic_fonts branch from 3b9dc06 to 99aeb16 Compare September 29, 2022 20:17

tacaswell added 3 commits September 30, 2022 00:51

FIX: do not raise in lru_cached function

6e99a52

If the cached function raises it will not be cached and we will continuously pay for cache misses.

QuLogic force-pushed the svg_generic_fonts branch from 99aeb16 to 6e99a52 Compare September 30, 2022 04:52

anntzer approved these changes Sep 30, 2022

View reviewed changes

tacaswell merged commit 8f6616c into matplotlib:main Sep 30, 2022

tacaswell deleted the svg_generic_fonts branch September 30, 2022 22:27

meeseeksmachine mentioned this pull request Sep 30, 2022

Backport PR #23638 on branch v3.6.x (FIX: correctly handle generic font families in svg text-as-text mode) #24059

Merged

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 30, 2022

Backport PR matplotlib#23638: FIX: correctly handle generic font fami…

bf16cb6

…lies in svg text-as-text mode

oscargus added a commit that referenced this pull request Oct 1, 2022

Merge pull request #24059 from meeseeksmachine/auto-backport-of-pr-23…

147b5d4

…638-on-v3.6.x Backport PR #23638 on branch v3.6.x (FIX: correctly handle generic font families in svg text-as-text mode)

ksunden mentioned this pull request Feb 20, 2023

merge up 3.7.0 #25270

Merged

6 tasks

Uh oh!

Conversation

tacaswell commented Aug 16, 2022

PR Summary

PR Checklist

Uh oh!

tacaswell commented Aug 17, 2022

Uh oh!

tacaswell commented Aug 17, 2022

Uh oh!

tacaswell commented Aug 17, 2022

Uh oh!

anntzer Aug 18, 2022

Choose a reason for hiding this comment

Uh oh!

tacaswell Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

tacaswell commented Aug 19, 2022

Uh oh!

tacaswell commented Sep 1, 2022

Uh oh!

anntzer Sep 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tacaswell Sep 1, 2022

Choose a reason for hiding this comment

Uh oh!

QuLogic Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

QuLogic commented Sep 30, 2022

Uh oh!

tacaswell commented Sep 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

anntzer Sep 1, 2022 •

edited

Loading