Improve warning for unsupported scripts. #16368
Conversation
| PyObject *text_helpers = NULL, *tmp = NULL; | ||
| if (!(text_helpers = PyImport_ImportModule("matplotlib._text_helpers")) || | ||
| !(tmp = PyObject_CallMethod(text_helpers, "warn_on_missing_glyph", "k", charcode))) { | ||
| goto exit; |
There was a problem hiding this comment.
I don't really get the goto exit. It just jumps out of the if block. But the exit: label is the next statement anyway.
There was a problem hiding this comment.
This is a normal pattern for C extension functions (have an exit block at the end which just has all the XDECREFs and jump to it if anything fails). See e.g. convert_open_args in ft2font_wrapper. Here the function is simple enough that you could not use this pattern; I prefer keeping it but can rewrite the function without it if you insist.
There was a problem hiding this comment.
I guess it's a little bit weird, because the actual exit is line 180; this is actually already the error handling section. However, the point here is not to go the line after the if, it's to take advantage of the lazy evaluation of ||, so I think maybe some of the confusion stems from that.
There was a problem hiding this comment.
This will be more useful if more lines get added below, which is perhaps not very likely in this particular function, but who knows. In any case, I think the flow of execution is quite clear.
|
rebased |
|
sure |
Trying e.g. to use an Indic script now gives
```
<string>:5: UserWarning: Glyph 3520 (\N{SINHALA LETTER VAYANNA}) missing from current font.
<string>:5: UserWarning: Matplotlib currently does not support Sinhala natively.
```
PR Summary
Improves the warning for cases like #16367.
Trying e.g. to use an Indic script now gives
We could even make the warning point to mplcairo... not that I particularly relish getting tons of bug reports about how to install it on obscure (i.e. nonlinux :-)) platforms.
The list of unicode blocks is at https://en.wikipedia.org/wiki/Unicode_block (I don't think it's available in the stdlib); I just hardcoded a few (Hebrew, Arabic, various Indic scripts) that I know are (almost) certainly not working due to shaping needs.
I moved the warning generation to Python because I don't particularly want to figure out how to call
encode("ascii", "namereplace")from C (well, it's just a big bunch of Py_CallFoo and Py_XDECREFs...), and anyways we're in a non-working case so the performance loss should be a big deal.PR Checklist