Warn user when mathtext font is used for ticks #20235
Conversation
|
I'm confused by this. I would think you'd only want the warning if there is a minus sign? Or are we claiming people can't put |
We're claiming that For ticks, we need to force axes to use mathtext, hence, the warning. |
|
Then should there also be a warning when the rcParam is validated? |
|
By the time we're validating rcParams, I don't think we can identify the use of mathtext (one could explicitly pass |
|
I guess I'm finding this all a little rickety. When folks want |
This more or less means (some part of) @aitikgupta's GSOC: currently, non-mathtext text cannot use multiple fonts on a single text object at all. |
|
We would also need to check font.{families}, for example, the repro code at #16995 (comment) before merging. |
|
Sorry, are you saying this is not ready for review? I'll move to draft, but feel free to move back |
|
LGTM, but do we want a test? |
|
Do we test |
|
Interesting, this breaks matplotlib/lib/matplotlib/tests/test_legend.py Lines 747 to 757 in d56e1b4 ^when the font is |
lib/matplotlib/ticker.py
Outdated
| mpl.font_manager.FontProperties( | ||
| mpl.rcParams["font.family"] | ||
| ) | ||
| ) == str(Path(mpl.get_data_path(), "fonts/ttf/cmr10.ttf")): |
There was a problem hiding this comment.
there's cbook._get_data_path too.
|
The warning arises because test_legend is using usetex, which uses different family names (here "Computer Modern", not "cmr10"), which can be found by texmanager but not by the normal font system. In practice, what this likely means here is that the call to |
|
Hmm, something weird is going on with warnings, spent some trying to get this work: with warnings.catch_warnings():
warnings.filterwarnings("ignore")
_log.warn("This is not ignored.") # <---
ufont = mpl.font_manager.findfont(mpl.font_manager.FontProperties(mpl.rcParams["font.family"]))
# ^Neither does it suppress the `findfont` warning
if ufont == str(cbook._get_data_path("fonts/ttf/cmr10.ttf")):
_api.warn_external(
"cmr10 font should...."
)Moreover, if we use the context manager, the warnings are no longer "just once". with warnings.catch_warnings():
warnings.filterwarnings("ignore")
# Do Nothing
ufont = mpl.font_manager.findfont(mpl.font_manager.FontProperties(mpl.rcParams["font.family"]))
if ufont == str(cbook._get_data_path("fonts/ttf/cmr10.ttf")):
_api.warn_external(
"cmr10 font should..."
)^Output contains a lot of "cmr10 font should..." (since we call multiple instances of Previously, without using the context manager: ufont = mpl.font_manager.findfont(mpl.font_manager.FontProperties(mpl.rcParams["font.family"]))
if ufont == str(cbook._get_data_path("fonts/ttf/cmr10.ttf")):
_api.warn_external(
"cmr10 font should..."
)^Output contains only one "cmr10 font should..." (even when multiple instances of |
|
I haven't fully unraveled your points, but the fact that warnings.catch_warnings() doesn't catch log.warn() is expected (logging and warnings are two different systems). (But I did miss that initially.) (The font_manager API is turning out to be not really optimal for this use case, but that's another story...) |
|
Interesting, I started off on the fundamentally wrong track! |
lib/matplotlib/ticker.py
Outdated
| except ValueError: | ||
| ufont = None | ||
|
|
||
| if ufont is not None and ufont == str( |
There was a problem hiding this comment.
you don't need the None check here
There was a problem hiding this comment.
I was under the impression that we shouldn't compare values if one of them is None...
I'll update 👍🏼
|
done! |
PR Summary
Following up the patch mentioned in #18397 (review), w.r.t. @anntzer's original comment #18397 (comment)
When using the "cmr10" font (or possibly more mathtext-fonts) for ticks, one should set
rcParams["axes.formatter.use_mathtext"] = Trueto trigger the machinery implemented by #18397Possible improvements:
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).