Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
FIX: correctly handle generic font families in svg text-as-text mode#23638
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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). |
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. |
This seems to cause an 8x increase in memory usage and a 10x slow down in the test suite. |
lib/matplotlib/font_manager.py Outdated
| family, | ||
| ", ".join(self._expand_aliases(family)) | ||
| if family in font_family_aliases: | ||
| # deal with the sans/san-serif/sans serif issue |
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 and the following line (the start of the for loop) can be just
forfaminself._expand_aliases(family):iffaminfont_family_aliases:continue# is that even possible? ...
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 will go into another PR, this change seems to be causing a 10x memory increase and I do not understand why yet.
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. |
36725ed toaf2cc4fCompareI 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. |
| @@ -1151,10 +1151,56 @@ def _draw_text_as_text(self, gc, x, y, s, prop, angle, ismath, mtext=None): | |||
| weight = fm.weight_dict[prop.get_weight()] | |||
| if weight != 400: | |||
| font_parts.append(f'{weight}') | |||
| def unique_iter(inp): | |||
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.
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 likedict.fromkeys(s for fam in prop.get_family() for s in _format_font_name(fam)) (ordict.fromkeys(itertools.chain(*map(_format_font_name, prop.get_family()))) for itertools aficionados)
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.
oh, nice!
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 replacedunique_iter, but didn't do anything fancier.
af2cc4f to26345f8CompareUh oh!
There was an error while loading.Please reload this page.
26345f8 to3b9dc06CompareUh oh!
There was an error while loading.Please reload this page.
3b9dc06 to99aeb16CompareThis tests:1. all of the fonts from the generic lists Matplotlib maintains ends up in the svg2. the generic family is included and un-escaped3. the specific fonts are escaped4. 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 areclosesmatplotlib#22528closesmatplotlib#23492
If the cached function raises it will not be cached and we will continuouslypay for cache misses.
99aeb16 to6e99a52CompareI pushed some minor corrections to the comments directly. |
…lies in svg text-as-text mode
…638-on-v3.6.xBackport PR#23638 on branch v3.6.x (FIX: correctly handle generic font families in svg text-as-text mode)
PR Summary
This:
in the svg output
closes#22528
closes#23492
I still need to write a test for this, but interactively this gives things like
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).