[with findfont diff] Parsing all families in font_manager#20496
[with findfont diff] Parsing all families in font_manager#20496aitikgupta wants to merge 15 commits intomatplotlib:mainfrom
Conversation
|
EDIT: This is solved now The tests now fail only due to an unrelated issue, and seems to be affecting all recent PRs. FAILED lib/matplotlib/tests/test_image.py::test_huge_range_log[png] - ValueError: Invalid vmin or vmax |
aitikgupta
changed the title
aitikgupta
changed the title
jklymak
left a comment
jklymak
left a comment
There was a problem hiding this comment.
findfont is public API, so can we just change its return type? At the very least this requires an API change note?
| # The original font is Calibri, if that is not installed, we fall back | ||
| # to Carlito, which is metrically equivalent. | ||
| if 'Calibri' in matplotlib.font_manager.findfont('Calibri:bold'): | ||
| if 'Calibri' in matplotlib.font_manager.findfont('Calibri:bold').keys(): |
There was a problem hiding this comment.
ah that's right, I was probably searching for findfont usages within mpl and happen to change this eagerly.
Because this is a public API, the return type change is quite huge, which would break things at a lot of places. (including users who don't want their less-relevant part of their work getting messed up, which is also a bit hard to traceback) I've proposed a 'without findfont diff' parsing of all families, which would only change internals of mpl and won't affect end users. see: #20549 I think this PR should still be open for a while until we've got a good look at the other PR's possibilities. |
|
Please reference other relevant PRs in a PR before asking folks to review. I had no idea there were two, and we probably should decide on an approach before reviewing. |
I did? I see a On the other hand, the logic still needs to be reviewed (which is mostly similar in this and the other PR), which is also why I didn't convert it into a draft before.. |
|
Folks aren't going to click through every link on a PR. A quick comment here that this was likely superseded by the other PR would have been helpful. |
|
I am going to close this PR because the core of it got merged in #20740 and we have decided to keep the API on |
3 participants
PR Summary
This is the beginning of migrating from Matplotlib's "Font-First" approach to a "Text-First" approach.
The very first step is to parse all families in font manager. Previously, we only parsed families until we find a font file, and the rest of the backends are accustomed to just a single font file (which needs changing)
I'm hoping this breaks many tests, so as to know exactly what to fix at other places.
findfont, from simple 'str' to 'OrderedDict'font.familyI've added a bunch of To-Dos (future PRs) and some fixes at some places.
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).