MNT: print fontname in missing glyph warning by story645 · Pull Request #26989 · matplotlib/matplotlib · GitHub

PR summary

Modified the "missing glyph" warning to also print out the name of the font the glyph is missing from. This is mostly to help me debug why #26854 is failing on CI but working locally but there have been times where I've gotten this error and would have found it helpful to have the added context.

second commit adds a :filter-warning: option to the ..plot directive spun out into #27076

PR checklist

[n/a] "closes #0000" is in the body of the PR description to link the related issue
new and changed code is tested
[n/a] Plotting related features are demonstrated in an example
[n/a] New Features and API Changes are noted with a directive and release note
[n/a] Documentation complies with general and docstring guidelines

How does this interact with the fallback list? Presumable we would want to show all of the fonts that were considered?

How does this interact with the fallback list? Presumable we would want to show all of the fonts that were considered?

Currently fallback suppresses the missing font and if fallback doesn't work only prints the first font

eta: I think b/c it's marked as a flag once and load_char_with_fallaback is kinda recursive.

matplotlib/src/ft2font.cpp

Lines 514 to 518 in 21ce592

    
           bool was_found = load_char_with_fallback(ft_object_with_glyph, glyph_index, glyphs, 
        
                                                    char_to_font, glyph_to_font, codepoints[n], flags, 
        
                                                    charcode_error, glyph_error, false); 
        
           if (!was_found) { 
        
               ft_glyph_warn((FT_ULong)codepoints[n]);

: I think b/c it's marked as a flag once and load_char_with_fallaback is kinda recursive.

Which also means I can't think of a clean way to only print the warnings if the fallback fails w/o maintaining a separate stack of warnings & I think that rabbit hole is out of scope for this PR.

Added a :filter-warning: to the plot directive so docs can build cleanly, but I'm really unsure that raising the warnings is all that helpful to someone using fallback-presumably they're using it b/c they expect that not all the fonts will work.

When we exhaust the list of fallback fonts, do we have anyway to get back to the top of the list and walk it again collecting all of their names?

When we exhaust the list of fallback fonts, do we have anyway to get back to the top of the list and walk it again collecting all of their names?

Kinda maybe? if we exhaust the list, it means i==fallbacks.size() and if that works that's definitely a cleaner solution than the changes I've been making of passing around a cache object...but also currently pybind doesn't want to work for me so I can't build anything.

matplotlib/src/ft2font.cpp

Lines 672 to 680 in 6ba7d5f

    
           for (size_t i = 0; i < fallbacks.size(); ++i) { 
        
               bool was_found = fallbacks[i]->load_char_with_fallback( 
        
                   ft_object_with_glyph, final_glyph_index, parent_glyphs, parent_char_to_font, 
        
                   parent_glyph_to_font, charcode, flags, charcode_error, glyph_error, override); 
        
               if (was_found) { 
        
                   return true; 
        
               } 
        
           } 
        
           return false;

So I was wrong in that it'll miss the first glyph when there's an override.

Got this to work by adding an array to F2tFont to store the names of the fonts. Reason is because if you can't find anything, you need to print font + fallback_list

I think this can be done without adding any new state to the objects.

changed to parameter into function

passing cache through works.

one warning w/ all the fonts instead of n warnings w/ one font per @tacaswell's request - and I'm 99% sure that the set can be passed from c++ to python directly but since this is for a string I figure there's no harm in doing the strcat on the C++ side since passing the set over would I think involve copying the set to the python set object. I figure this can get cleaned up as part of the pybind port if it's important enough.

lists all fallback fonts if glyph missing in all fonts removed ft_get_char_index_or_warn added a test for otf fonts on pdf + some windows fonts to tests Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com> Co-authored-by: Thomas A Caswell <tcaswell@gmail.com>

one warning w/ all the fonts instead of n warnings w/ one font per @tacaswell's request

It was not a request, I only asked (in a private channel) why many instead of one!

Thank you @story645 !

story645 added topic: text/fonts Maintenance labels Oct 3, 2023

story645 changed the title print fontname in missing glyph warning MNT: print fontname in missing glyph warning Oct 3, 2023

story645 force-pushed the warning-fontname branch from a5d314f to c952d15 Compare October 4, 2023 04:28

story645 force-pushed the warning-fontname branch 2 times, most recently from 22e8b01 to e16b88d Compare October 4, 2023 18:56

story645 force-pushed the warning-fontname branch from c5648ce to b613bd7 Compare October 4, 2023 19:39

story645 force-pushed the warning-fontname branch from d0908d8 to dc2366a Compare October 4, 2023 23:10

story645 force-pushed the warning-fontname branch from dc2366a to 4aaf163 Compare October 12, 2023 21:23

story645 force-pushed the warning-fontname branch from 4aaf163 to 8ca754a Compare October 12, 2023 21:27

story645 requested a review from tacaswell October 12, 2023 21:28

story645 force-pushed the warning-fontname branch 3 times, most recently from c66bfd9 to f9274f3 Compare October 12, 2023 22:02

story645 mentioned this pull request Oct 12, 2023

DOC: plot-directive warning filter option #27076

Closed

5 tasks

story645 force-pushed the warning-fontname branch 3 times, most recently from df5d2ae to c719d89 Compare October 15, 2023 01:41

story645 mentioned this pull request Oct 15, 2023

[ENH] xkcd.mplstyle w/ quasi parsing of patheffects.{functions} #26854

Open

3 of 4 tasks

ksunden reviewed Oct 16, 2023

View reviewed changes

Comment thread src/ft2font.cpp Outdated

Comment thread src/ft2font.cpp Outdated

story645 force-pushed the warning-fontname branch from 16a3fb9 to 09584ff Compare October 16, 2023 20:43

ksunden reviewed Oct 16, 2023

View reviewed changes

Comment thread src/ft2font.cpp Outdated

story645 force-pushed the warning-fontname branch 4 times, most recently from b3dd35d to 1ff3b42 Compare October 17, 2023 15:23

ksunden reviewed Oct 17, 2023

View reviewed changes

Comment thread lib/matplotlib/tests/test_backend_pdf.py Outdated

story645 force-pushed the warning-fontname branch 2 times, most recently from 0a39542 to 296977b Compare October 17, 2023 19:56

tacaswell reviewed Oct 18, 2023

View reviewed changes

Comment thread src/ft2font.h Outdated

tacaswell previously requested changes Oct 18, 2023

View reviewed changes

story645 force-pushed the warning-fontname branch from 296977b to eee55ab Compare October 19, 2023 01:51

story645 requested a review from tacaswell October 19, 2023 20:39

story645 added this to the v3.8.1 milestone Oct 22, 2023

tacaswell reviewed Oct 26, 2023

View reviewed changes

Comment thread lib/matplotlib/tests/test_ft2font.py Outdated

tacaswell reviewed Oct 26, 2023

View reviewed changes

Comment thread src/ft2font.h Outdated

story645 force-pushed the warning-fontname branch from eee55ab to 249f4be Compare October 27, 2023 05:01

story645 force-pushed the warning-fontname branch 2 times, most recently from 22c8c97 to 6470890 Compare October 27, 2023 06:00

story645 force-pushed the warning-fontname branch from 6470890 to 4ece8ab Compare October 27, 2023 16:25

mend

15c44a5

ksunden modified the milestones: v3.8.1, v3.9.0 Oct 27, 2023

tacaswell approved these changes Oct 27, 2023

View reviewed changes

tacaswell merged commit dbebe96 into matplotlib:main Oct 27, 2023

story645 deleted the warning-fontname branch October 27, 2023 20:58

MNT: print fontname in missing glyph warning by story645 · Pull Request #26989 · matplotlib/matplotlib · GitHub

Repository navigation

MNT: print fontname in missing glyph warning - #26989

MNT: print fontname in missing glyph warning#26989tacaswell merged 2 commits intomatplotlib:mainmatplotlib/matplotlib:mainfrom story645:warning-fontnamestory645/matplotlib:warning-fontnameCopy head branch name to clipboard

Conversation

story645 commented Oct 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR summary

PR checklist

Uh oh!

tacaswell commented Oct 3, 2023

Uh oh!

story645 commented Oct 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

story645 commented Oct 4, 2023

Uh oh!

story645 commented Oct 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tacaswell commented Oct 4, 2023

Uh oh!

story645 commented Oct 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

story645 commented Oct 12, 2023

Uh oh!

story645 commented Oct 12, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tacaswell left a comment

Choose a reason for hiding this comment

Uh oh!

story645 commented Oct 19, 2023

Uh oh!

Uh oh!

Uh oh!

story645 commented Oct 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tacaswell commented Oct 27, 2023

Uh oh!

tacaswell commented Oct 27, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MNT: print fontname in missing glyph warning#26989
tacaswell merged 2 commits into
matplotlib:mainfrom
story645:warning-fontname

story645 commented Oct 3, 2023 •
edited
Loading

story645 commented Oct 4, 2023 •
edited
Loading

story645 commented Oct 4, 2023 •
edited
Loading

story645 commented Oct 4, 2023 •
edited
Loading

story645 commented Oct 27, 2023 •
edited
Loading